New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 650 - noalias += Row Major Sparse Matrix multiplication
noalias += Row Major Sparse Matrix multiplication
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Sparse
3.3 (current stable)
All All
: High Compilation Problem
Assigned To: Nobody
: test-needed
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2013-08-19 14:54 UTC by matthieu.nesme
Modified: 2015-06-09 16:42 UTC (History)
3 users (show)



Attachments
unit test (2.67 KB, text/x-c++src)
2015-04-26 15:38 UTC, Jan van Dijk
no flags Details

Description matthieu.nesme 2013-08-19 14:54:06 UTC
Hello.

Sometimes using .noalias() += does not perform a += but only a =.
I do not know if it is the only case, but I found that adding the multiplication of a SparseRowMajor matrix with a vector is bugging:

v.noalias() += sparseRowMajorMatrix * w;

The multiplication with a col major matrix seems to correctly perform the addition.



typedef Eigen::Matrix< double, Eigen::Dynamic, 1 > Vec;
typedef Eigen::SparseMatrix<double,Eigen::RowMajor> SparseRowMatrix;
typedef Eigen::SparseMatrix<double,Eigen::ColMajor> SparseColMatrix;

Vec test(3); test.fill(3.0);
Vec result(3);
SparseRowMatrix MSparseRow(3,3); MSparseRow.setIdentity();
SparseColMatrix MSparseCol(3,3); MSparseCol.setIdentity();

result.setOnes();
result.noalias() += test; // OK
std::cerr<<result<<std::endl<<std::endl; // 4 4 4

result.setOnes();
result.noalias() += MSparseRow * test; // KO
std::cerr<<result<<std::endl<<std::endl; // 3 3 3

result.setOnes();
result.noalias() += MSparseCol * test; // OK
std::cerr<<result<<std::endl<<std::endl; // 4 4 4
Comment 1 Jan van Dijk 2015-04-25 18:11:22 UTC
I just tried to reproduce this by compiling the following
completed test case,


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

int main()
{       
  ... insert OP's code above here
}

I verified that the code gives wrong results up to revision 6829. Since revision 6830, this code no longer compiles. The second and third += calls in the OP's test code are now invalid because a member 'addTo' is missing, see the compiler output below.

jan@linux-pwd6:~/src/gum-cvs/ideas/jan/eigen> g++ --version
g++ (SUSE Linux) 4.8.3 20140627 [gcc-4_8-branch revision 212064]
...
jan@linux-pwd6:~/src/gum-cvs/ideas/jan/eigen> g++ -I /home/jan/local/eigen/include/eigen3 650.cpp
In file included from /home/jan/local/eigen/include/eigen3/Eigen/Core:384:0,
                 from 650.cpp:1:
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/ProductEvaluators.h: In instantiation of ‘static void Eigen::internal::Assignment<DstXprType, Eigen::Product<Lhs, Rhs, 0>, Eigen::internal::add_assign_op<Scalar>, Eigen::internal::Dense2Dense, Scalar>::run(DstXprType&, const SrcXprType&, const Eigen::internal::add_assign_op<Scalar>&) [with DstXprType = Eigen::Matrix<double, -1, 1>; Lhs = Eigen::SparseMatrix<double, 1>; Rhs = Eigen::Matrix<double, -1, 1>; Scalar = double; Eigen::internal::Assignment<DstXprType, Eigen::Product<Lhs, Rhs, 0>, Eigen::internal::add_assign_op<Scalar>, Eigen::internal::Dense2Dense, Scalar>::SrcXprType = Eigen::Product<Eigen::SparseMatrix<double, 1>, Eigen::Matrix<double, -1, 1>, 0>]’:
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/AssignEvaluator.h:752:70:   required from ‘void Eigen::internal::call_assignment_no_alias(Dst&, const Src&, const Func&) [with Dst = Eigen::Matrix<double, -1, 1>; Src = Eigen::Product<Eigen::SparseMatrix<double, 1>, Eigen::Matrix<double, -1, 1>, 0>; Func = Eigen::internal::add_assign_op<double>]’
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/NoAlias.h:50:96:   required from ‘ExpressionType& Eigen::NoAlias<ExpressionType, StorageBase>::operator+=(const StorageBase<OtherDerived>&) [with OtherDerived = Eigen::Product<Eigen::SparseMatrix<double, 1>, Eigen::Matrix<double, -1, 1>, 0>; ExpressionType = Eigen::Matrix<double, -1, 1>; StorageBase = Eigen::MatrixBase]’
650.cpp:21:19:   required from here
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/ProductEvaluators.h:150:68: error: ‘addTo’ is not a member of ‘Eigen::internal::generic_product_impl<Eigen::SparseMatrix<double, 1>, Eigen::Matrix<double, -1, 1>, Eigen::SparseShape, Eigen::DenseShape, 6>’
     generic_product_impl<Lhs, Rhs>::addTo(dst, src.lhs(), src.rhs());
                                                                    ^
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/ProductEvaluators.h: In instantiation of ‘static void Eigen::internal::Assignment<DstXprType, Eigen::Product<Lhs, Rhs, 0>, Eigen::internal::add_assign_op<Scalar>, Eigen::internal::Dense2Dense, Scalar>::run(DstXprType&, const SrcXprType&, const Eigen::internal::add_assign_op<Scalar>&) [with DstXprType = Eigen::Matrix<double, -1, 1>; Lhs = Eigen::SparseMatrix<double, 0>; Rhs = Eigen::Matrix<double, -1, 1>; Scalar = double; Eigen::internal::Assignment<DstXprType, Eigen::Product<Lhs, Rhs, 0>, Eigen::internal::add_assign_op<Scalar>, Eigen::internal::Dense2Dense, Scalar>::SrcXprType = Eigen::Product<Eigen::SparseMatrix<double, 0>, Eigen::Matrix<double, -1, 1>, 0>]’:
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/AssignEvaluator.h:752:70:   required from ‘void Eigen::internal::call_assignment_no_alias(Dst&, const Src&, const Func&) [with Dst = Eigen::Matrix<double, -1, 1>; Src = Eigen::Product<Eigen::SparseMatrix<double, 0>, Eigen::Matrix<double, -1, 1>, 0>; Func = Eigen::internal::add_assign_op<double>]’
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/NoAlias.h:50:96:   required from ‘ExpressionType& Eigen::NoAlias<ExpressionType, StorageBase>::operator+=(const StorageBase<OtherDerived>&) [with OtherDerived = Eigen::Product<Eigen::SparseMatrix<double, 0>, Eigen::Matrix<double, -1, 1>, 0>; ExpressionType = Eigen::Matrix<double, -1, 1>; StorageBase = Eigen::MatrixBase]’
650.cpp:25:19:   required from here
/home/jan/local/eigen/include/eigen3/Eigen/src/Core/ProductEvaluators.h:150:68: error: ‘addTo’ is not a member of ‘Eigen::internal::generic_product_impl<Eigen::SparseMatrix<double, 0>, Eigen::Matrix<double, -1, 1>, Eigen::SparseShape, Eigen::DenseShape, 6>’
Comment 2 Christoph Hertzberg 2015-04-25 18:25:08 UTC
(In reply to Jan van Dijk from comment #1)
> I just tried to reproduce this by compiling the following
> completed test case,

I can confirm this as well. I guess that's a task for Gael ...


> #include "Eigen/Core"
> #include "Eigen/Sparse"

NB: It's sufficient to include <Eigen/SparseCore> (which itself
includes <Eigen/Core>), if you want to avoid including all sparse modules.
Comment 3 Jan van Dijk 2015-04-26 15:38:10 UTC
Created attachment 565 [details]
unit test

I noticed the 'test-needed' keyword in the bug entry, so here we go:

I attached a small program that demonstrates the various problems that are outlined in this bug report. It uses 1x1 matrices instead of matrices and vectors, so all possible m1*m2 can be tested, as opposed to just m*v.

It is a stand-alone program, but written such that it can easily be changed into an Eigen-style unit test. See the comments in the top of the file for more info.
Comment 4 Gael Guennebaud 2015-04-26 20:48:16 UTC
> I guess that's a task for Gael ...

yes, I'll have a look.

Thank you Jan for the self-contained test case.
Comment 5 Gael Guennebaud 2015-06-09 16:04:37 UTC
Fixed for 3.2: https://bitbucket.org/eigen/eigen/commits/d35bd1174aa3/
Comment 6 Gael Guennebaud 2015-06-09 16:40:29 UTC
Fixed for the general case in the devel branch: https://bitbucket.org/eigen/eigen/commits/f011fb12755f/

The remaining compilation issues concern situations for which .noalias() is pointless, like res.noalias() += m2.

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