New user self-registration is currently disabled. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Bug 879 - Matrix product with triangularView does not compile
Matrix product with triangularView does not compile
Status: CONFIRMED
Product: Eigen
Classification: Unclassified
Component: Core - matrix products
unspecified
x86 - 64-bit Linux
: Normal Optimization
Assigned To: Nobody
:
Depends on:
Blocks: 3.4
  Show dependency treegraph
 
Reported: 2014-09-22 15:33 UTC by Sebastian Birk
Modified: 2015-09-03 10:10 UTC (History)
3 users (show)



Attachments

Description Sebastian Birk 2014-09-22 15:33:41 UTC
Hi,

the following minimal example does not compile (neither with clang nor with gcc) with the current tip (rev 6427) of the Eigen development branch:

  typedef Eigen::Matrix<std::complex<double>,Dynamic,Dynamic,RowMajor>  MatDense;
  MatDense A, B, C;

  A.triangularView<Eigen::Upper>() = B*C.triangularView<Eigen::Upper>();

I tracked the hg revision down to be 6359 that introduces the compilation error. Up to revision 6358 the above code compiled.

Clang prints several errors:

Eigen/src/Core/util/BlasUtil.h:162:52: error: no member named 'IsVectorAtCompileTime' in 'Eigen::TriangularView<Eigen::Matrix<std::complex<double>, -1, -1, 1, -1,
      -1>, 2>' && (   bool(XprType::IsVectorAtCompileTime)

Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h:250:65: error: no viable conversion from 'const Eigen::TriangularView<Eigen::Matrix<std::complex<double>,
      -1, -1, 1, -1, -1>, 2>' to 'typename internal::add_const_on_value_type<ActualRhs>::type' (aka 'const int')
    typename internal::add_const_on_value_type<ActualRhs>::type actualRhs = RhsBlasTraits::extract(prod.rhs());

Eigen/src/Core/products/GeneralMatrixMatrixTriangular.h:256:29: error: type '_ActualRhs' (aka 'int') cannot be used prior to '::' because it has no members
      typename Rhs::Scalar, _ActualRhs::Flags&RowMajorBit ? RowMajor : ColMajor, RhsBlasTraits::NeedToConjugate


Cheers,
Sebastian
Comment 1 Gael Guennebaud 2014-09-22 17:14:39 UTC
Actually, I did not known that 

A.triangularView<Eigen::Upper>() = B*C.triangularView<Eigen::Upper>();

compiled previously as this combination is not supported. It occurs that previously this expression was (wrongly) interpreted as:

A.triangularView<Eigen::Upper>() = B*C;

and therefore, you get the right results only if the strictly lower part of C is set to zero. As a quick fix, I'll make this expression compile as:

A.triangularView<Eigen::Upper>() = (B*C.triangularView<Eigen::Upper>()).eval();

in both the 3.2 and default branch.

Of course, it is planed to fully support all combination in a near future.
Comment 2 Gael Guennebaud 2014-09-22 17:36:58 UTC
Workarounds:

https://bitbucket.org/eigen/eigen/commits/0477645da8b6/
Changeset:   0477645da8b6
User:        ggael
Date:        2014-09-22 15:34:17+00:00
Summary:     Bug 879: fix compilation of tri1=mat*tri2 by copying tri2 into a full temporary.

https://bitbucket.org/eigen/eigen/commits/7a3f434088a7/
Changeset:   7a3f434088a7
Branch:      3.2
User:        ggael
Date:        2014-09-22 15:20:42+00:00
Summary:     Bug 879: tri1 = mat * tri2 was compiling and running incorrectly if tri2 was not numerically triangular. Workaround the issue by evaluating mat*tri2 into a temporary.

I do not close this bug because:
1) there is no proper unit tests. (required for 3.3)
2) we should fully support such combinations to get the missing second x2 speedup.
Comment 3 Gael Guennebaud 2015-06-16 20:12:25 UTC
Added a unit test: https://bitbucket.org/eigen/eigen/commits/b3cbade96607/

Good enough for 3.3

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