Hello,
I'm using the sparse matrix code and have the following issue.

I was following the documentation example for how to fill a sparse matrix in 'coherence order', which looks like this:

Code: Select all

SparseMatrix<float> mat(1000,1000);
mat.reserve(estimated_number_of_non_zero);  // optional
for(int j=0; j<1000; ++j)
{
  mat.startVec(j);                          // optional for a DynamicSparseMatrix
  for each i interacting with j             // with increasing i
      mat.insertBack(i,j) = foo(i,j);
}
mat.finalize();                             // optional for a DynamicSparseMatrix


When I tried this in my program, upon the second call to insertBack(), I hit the first assert().
The interface to insertBack goes as:

Code: Select all

inline Scalar& insertBack(int outer, int inner);


which is different from that of insert:

Code: Select all

EIGEN_DONT_INLINE Scalar& insert(int row, int col)
    {
      const int outer = IsRowMajor ? row : col;
      const int inner = IsRowMajor ? col : row;

      ...
    }


Note that insertBack() uses (outer, inner) while insert() uses (row, col). I think this is the point of confusion. Because of this discrepancy, the pseudocode in the documentation for filling in 'coherence order' looks totally fine for the normal convention of matrix addressing, but for a column-major matrix, the arguments passed are swapped!

Is the issue the documentation or the interface?

Cheers,
Leo

thanks for noticing that. I think it is both! Finally it looks dangerous to have insert and insertBack using different conventions for the arguments, so I'm going to make them consistent with (row,col), and add a insertBackByOiterInner as we now have in the dense module.

Gael,
Thanks for taking the time to work on this! However, there is now a bug in that insertBack() calls insertBackByInnerOuter() which looks like a typo for insertBackByOuterInner().

Cheers
Leo

oops thanks a lot !

have to improve the unit tests !