Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider the possibility to store the gradient and the bounds vectors inside the library #17

Open
GiulioRomualdi opened this issue Feb 4, 2019 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@GiulioRomualdi
Copy link
Member

As pointed by @joaospinto in #16, it is not clear how the ownership of the optimization elements (matrices and vector) are handled.

As already explained in #16 (comment), osqp itself stores the hessian and the constraint matrix into its data structure. While, on the other hand, the vectors are not saved inside the osqp library and so the user has to guarantee the existence of the objects (e.g. save as a member of the class). I think that this asymmetry may cause confusion and may be a source of error.

What do you think to save the gradient and the bounds inside the osqp-eigen?

cc @traversaro and @S-Dafarra

@GiulioRomualdi GiulioRomualdi added the enhancement New feature or request label Feb 4, 2019
@S-Dafarra
Copy link
Collaborator

I think that this asymmetry may cause confusion and may be a source of error.

I think that the problem is related to the fact that in https://github.com/robotology/osqp-eigen/blob/master/include/OsqpEigen/Data.hpp#L94 and in https://github.com/robotology/osqp-eigen/blob/master/include/OsqpEigen/Data.hpp#L110 it is required a non-const reference. This is a source of confusion given that they are setter methods, where you don't expect the input to be modified, i.e. this (https://github.com/robotology/osqp-eigen/blob/master/include/OsqpEigen/Data.tpp#L107) is not expected. If the pointer to the data is needed, I would suggest to use Eigen::Ref (https://eigen.tuxfamily.org/dox/classEigen_1_1Ref.html) as an input parameter. In this way it is clear that the user still has to guarantee the existence and the lifetime of the Ref. Clearly, this should be specified also in the documentation. Another possibility is to use shared pointers, but I believe this would pollute the interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants