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

Ownership of bounds, gradient, Hessian, and constraint matrix #16

Open
joaospinto opened this issue Jan 18, 2019 · 3 comments
Open

Ownership of bounds, gradient, Hessian, and constraint matrix #16

joaospinto opened this issue Jan 18, 2019 · 3 comments

Comments

@joaospinto
Copy link

joaospinto commented Jan 18, 2019

When writing a class that encapsulates the use of this library in my codebase, I noticed that I had to store all matrices/vectors as private variables, otherwise some of the pointers would become stale (your library does not seem to be taking ownership of those matrices/vectors). Is this intended? Took me a while to figure this out.

Also, the way you pass Eigen objects around seems a bit unorthodox. You may want to read this.

@joaospinto joaospinto changed the title Support for runtime-determined matrix sizes Ownership of bounds, gradient, Hessian, and constraint matrix Jan 18, 2019
@GiulioRomualdi
Copy link
Member

Hi @joaospinto thanks for using osqp-eigen

When writing a class that encapsulates the use of this library in my codebase, I noticed that I had to store all matrices/vectors as private variables, otherwise some of the pointers would become stale (your library does not seem to be taking ownership of those matrices/vectors). Is this intended?

The library wants to be a simple wrapper to osqp and so we try to reduce the memory allocation (matrices and vectors). However in the initialization phase (e.g. here) the Hessian matrix is saved inside the data structure of osqp.

if(!OsqpEigen::SparseMatrixHelper::createOsqpSparseMatrix(hessianMatrix, m_data->P)){

and here the memory is allocated and the Hessian matrix is copied

osqpSparseMatrix = csc_spalloc(rows, cols, numberOfNonZeroCoeff, 1, 0);
int innerOsqpPosition = 0;
for(int k = 0; k < cols; k++) {
if (eigenSparseMatrix.isCompressed()) {
osqpSparseMatrix->p[k] = static_cast<c_int>(outerIndexPtr[k]);
} else {
if (k == 0) {
osqpSparseMatrix->p[k] = 0;
} else {
osqpSparseMatrix->p[k] = osqpSparseMatrix->p[k-1] + innerNonZerosPtr[k-1];
}
}
for (typename Eigen::SparseMatrix<T>::InnerIterator it(eigenSparseMatrix,k); it; ++it) {
osqpSparseMatrix->i[innerOsqpPosition] = static_cast<c_int>(it.row());
osqpSparseMatrix->x[innerOsqpPosition] = static_cast<c_float>(it.value());
innerOsqpPosition++;
}
}
osqpSparseMatrix->p[static_cast<int>(cols)] = static_cast<c_int>(innerOsqpPosition);

This is required by osqp itself. Indeed osqp wants a pointer to a csc struct when the first optimization problem is created.

On the other hand, when you set the gradient no memory allocation is performed and a pointer to the vector is passed to the osqp library

template<int n>
bool OsqpEigen::Data::setGradient(Eigen::Matrix<c_float, n, 1>& gradient)
{
if(gradient.rows() != m_data->n){
std::cerr << "[OsqpEigen::Data::setGradient] The size of the gradient must be equal to the number of the variables."
<< std::endl;
return false;
}
m_isGradientSet = true;
m_data->q = gradient.data();
return true;
}

So in simple terms:

  • you do not need to save the matrices as private variables (dynamic memory allocation is performed by the csc_spalloc function)
  • you need to save the vectors as private variables because no memory allocation is performed.

On the light of this, it would be nice to have a simple example where I can try to replicate your problem.

@joaospinto
Copy link
Author

@GiulioRomualdi That makes sense. I actually only had issues with the bound vectors; I somehow assumed that the same issue would exist with the matrices, so I made sure they would not go out of scope as well, but had not double checked. Thanks for clarifying! Also, thanks for writing this library, you probably saved me a couple of days of work.

@GiulioRomualdi
Copy link
Member

cc @MiladShafiee

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

No branches or pull requests

2 participants