New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 718 - Inner Iterator Incorrect for Row Major Sparse Matrices
Summary: Inner Iterator Incorrect for Row Major Sparse Matrices
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Sparse (show other bugs)
Version: 3.2
Hardware: All All
: Normal major
Assignee: Nobody
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2013-12-19 03:22 UTC by PCAdkins
Modified: 2014-10-20 14:08 UTC (History)
2 users (show)



Attachments

Description PCAdkins 2013-12-19 03:22:39 UTC
In the following code, I declare and initialize a 5x5 sparse matrix and set the (0,0) value to 1; matrix A is row major, matrix B is column major.  I run over the outer index and through the inner iterator in each case identically.  However, when I run the code - with matrix B, I get a "1" printed out - as expected; with matrix A I get an absurd number.  

I encountered this while trying to implement a mean calculation for a sparse matrix.  Here's a bit of sample code to demonstrate the problem as discussed above.  Yes, I know some of the header stuff is unnecessary; I just nabbed it from another demo I was working on:

#include <iostream>
#include <Eigen/Dense>
#include <Eigen/Sparse>

using namespace Eigen;
using namespace std;
int main()
{
 SparseMatrix<float,RowMajor> A(5,5);
 
 A.insert(0,0)=1;

 for (int k = 0; k<A.outerSize(); ++k)
 for (SparseMatrix<float>::InnerIterator it(A,k); it; ++it)
     std::cout<<it.value()<<std::endl; 

 SparseMatrix<float,ColMajor> B(5,5);

 B.insert(0,0)=1;

 for (int k = 0; k<B.outerSize(); ++k)
 for (SparseMatrix<float>::InnerIterator it(B,k); it; ++it)
     std::cout<<it.value()<<std::endl;
}
Comment 1 Christoph Hertzberg 2013-12-19 11:24:54 UTC
(In reply to comment #0)
>  SparseMatrix<float,RowMajor> A(5,5);
> 
>  A.insert(0,0)=1;
> 
>  for (int k = 0; k<A.outerSize(); ++k)
>  for (SparseMatrix<float>::InnerIterator it(A,k); it; ++it)
>      std::cout<<it.value()<<std::endl; 

You must use SparseMatrix<float, ColMajor>::InnerIterator.

I guess what happens is that A is converted to a temporary ColMajor matrix which gets invalid before it.value() is called. 
We should catch that somehow, maybe by adding a constructor for invalid types which results in a static assertion.

I assume things like the following won't work properly, either:

  typedef SparseMatrix<float> Sparse;
  Sparse A, B;                      // initialize somehow
  Sparse::InnerIterator(A+B, 0);    // Iterator to a temporary
Comment 2 Gael Guennebaud 2014-10-20 14:08:00 UTC
This has been fixed a few weeks ago for SparseMatrix:
https://bitbucket.org/eigen/eigen/commits/afd3e37726ef

and for SparseVector:
https://bitbucket.org/eigen/eigen/commits/4a1386d13502/

Note You need to log in before you can comment on or make changes to this bug.