search

robotology / osqp-eigen

Simple Eigen-C++ wrapper for OSQP library
https://robotology.github.io/osqp-eigen/
BSD 3-Clause "New" or "Revised" License
396 stars 118 forks source link

Correctly iterate over row major sparse matrices #54

Closed mpowelson closed 4 years ago

mpowelson commented 4 years ago

Changes createOsqpSparseMatrix to iterate to eigenSparseMatrix.outerSize() instead of cols().

S-Dafarra commented 4 years ago

Hi @mpowelson. Thanks a lot for the PR! Out of sheer curiosity, how did you manage to find this? Do you have some piece of code that is failing without this change?

mpowelson commented 4 years ago

We are using osqp-eigen in trajopt_ros and our matrices are row major. I don't remember the symptoms, but something was going wrong and when I debugged it, I saw the matrix was the wrong size.

S-Dafarra commented 4 years ago

Thanks for the explanation! I tried to investigate a little more the issue. Actually, when developing we mainly focused on "Compressed Sparse-Column" (CSC) as this is what osqp is expecting (https://github.com/oxfordcontrol/osqp/blob/master/include/types.h#L21-L29l). I guess that by passing a "Compressed Sparse-Row" (CSR) the net effect is that the matrix may result transposed.

On the Eigen side, it is possible to pass from CSR to CSC with a simple assignment. Probably it is safer to save the input matrix into a CSC to avoid this kind of problem. I tried to implement this in https://github.com/robotology/osqp-eigen/pull/57. @mpowelson can you try to check that out to see if it solves your problem?

mpowelson commented 4 years ago

Yes, it seems like that solves it. I'll close this.