This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen
Bug 650 - noalias += Row Major Sparse Matrix multiplication
Summary: noalias += Row Major Sparse Matrix multiplication
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: Sparse (show other bugs)
Version: 3.3 (current stable)
Hardware: All All
: High Compilation Problem
Assignee: Nobody
URL:
Whiteboard:
Keywords: test-needed
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2013-08-19 14:54 UTC by matthieu.nesme
Modified: 2019-12-04 12:35 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.
Comment 7 Nobody 2019-12-04 12:35:26 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/650.

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