New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 1359 - Regression from 3.2.9 or working as intended?
Regression from 3.2.9 or working as intended?
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Sparse
3.3 (current stable)
All All
: Normal Compilation Problem
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-08 17:19 UTC by Pierre G.
Modified: 2016-12-14 16:10 UTC (History)
3 users (show)



Attachments

Description Pierre G. 2016-12-08 17:19:24 UTC
Hi,

The following code compiles in Eigen 3.2.9 but fails to compile starting 3.2.10 and in the current release (3.3.1)

#include "include/eigen3/Eigen/Sparse"
#include <iostream>

int main()
{
  Eigen::SparseMatrix<double> sm(5,5);
  std::vector< Eigen::Triplet<double> > tpl;
  tpl.push_back(Eigen::Triplet<double>(0, 0, 1));
  tpl.push_back(Eigen::Triplet<double>(0, 1, 2));
  sm.setFromTriplets(tpl.begin(), tpl.end());
  sm.row(0) *= 2;
  std::cout << sm << std::endl;
  return 0;
}

The error I get is:
include/eigen3/Eigen/src/SparseCore/SparseCwiseUnaryOp.h:127:42: error: no type named ‘InnerIterator’ in ‘class Eigen::Block<Eigen::SparseMatrix<double>, 1, -1, false>’
     for (typename Derived::InnerIterator i(derived(),j); i; ++i)
                                          ^
include/eigen3/Eigen/src/SparseCore/SparseCwiseUnaryOp.h:127:42: error: no type named ‘InnerIterator’ in ‘class Eigen::Block<Eigen::SparseMatrix<double>, 1, -1, false>’

More generally, any function that tries to access InnerIterator on a Block access to a SparseMatrix fails similarly (and perhaps logically). (note: I don't have any other example from within Eigen itself)

The sparse module documentation would indicate this is working as intended since accessing a row of a ColMajor sparse matrix should be read-only. However, it used to work until 3.2.10 (I believe https://bitbucket.org/eigen/eigen/commits/0730079b2 introduced the breaking change but the error is different from 3.3 as it relates to the absence of RefValue in the new sparse common iterator class introduced by this commit).

Note that it's possible to work around the bug by doing what the *= operator does without the block in the way: (this work-around is also broken in 3.2.10)

for(int k = 0; k < sm.outerSize(); ++k)
{
  for(Eigen::SparseMatrix<double>::InnerIterator it(sm, k); it; ++it)
  { 
    if(it.row() == 0) { it.valueRef() *= 2; } 
  } 
}
Comment 1 Gael Guennebaud 2016-12-14 15:14:01 UTC
You can workaround with:

  sm.middleRows(0,1) *= 2.

The thing is that since sm is column major, sm.row(0) is not writeable. The fact it worked in 3.2.9 is due to a side effect of a bug fixed by the commit you pointed out. For instance, in 3.2.9, SparseVector<double> vec = sm.row(0) does not work.

Nonetheless, in 3.3, operator*= should be fixed to use an evaluator to get an InnerIterator, but that's not fully helping because the iterator we get is read-only, and make it potentially read-write is not straightforward (lot of meta programming and addition specializations...). Moreover, be aware that the cost of this operation is O(sm.cols()+sm.nonZeros()) because you have to iterate over all columns, and for each column search for the respective coefficient if it exists. So this should only be done for rapid prototyping.
Comment 2 Gael Guennebaud 2016-12-14 16:10:03 UTC
Finally, I found an easy way to fix this:

https://bitbucket.org/eigen/eigen/commits/b2fecf173f2a/
Summary:     Bug 1359: fix sparse /=scalar and *=scalar implementation.
InnerIterators must be obtained from an evaluator.

https://bitbucket.org/eigen/eigen/commits/f6a4d21c07d6/
Summary:     Bug 1359: fix compilation of col_major_sparse.row() *= scalar
(used to work in 3.2.9 though the expression is not really writable) 

3.3 backports:
https://bitbucket.org/eigen/eigen/commits/c7c0ab99398e/
https://bitbucket.org/eigen/eigen/commits/1b1c0abf3f25/



For 3.2, if you really care, feel free to propose a patch based on https://bitbucket.org/eigen/eigen/commits/f6a4d21c07d6/

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