From ca6a2a524832e051b64a1545a91220f9dc9887cb Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Wed, 26 Oct 2016 14:13:05 +0200 Subject: [PATCH 1/3] Fix warning with ICC --- Eigen/src/Core/CoreEvaluators.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Eigen/src/Core/CoreEvaluators.h b/Eigen/src/Core/CoreEvaluators.h index 00c079bd8..1d14af652 100644 --- a/Eigen/src/Core/CoreEvaluators.h +++ b/Eigen/src/Core/CoreEvaluators.h @@ -1302,7 +1302,7 @@ struct evaluator > } protected: - const ArgTypeNested m_arg; + typename internal::add_const_on_value_type::type m_arg; const MemberOp m_functor; }; From 97feea9d39ccaf298082cd537e85c311bf354010 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Wed, 26 Oct 2016 15:53:13 +0200 Subject: [PATCH 2/3] add a generic EIGEN_HAS_CXX11 --- Eigen/src/Core/util/Macros.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Eigen/src/Core/util/Macros.h b/Eigen/src/Core/util/Macros.h index ce715716c..00aeb858d 100644 --- a/Eigen/src/Core/util/Macros.h +++ b/Eigen/src/Core/util/Macros.h @@ -356,6 +356,13 @@ #define EIGEN_MAX_CPP_VER 99 #endif +#if EIGEN_MAX_CPP_VER>=11 && defined(__cplusplus) && (__cplusplus >= 201103L) +#define EIGEN_HAS_CXX11 1 +#else +#define EIGEN_HAS_CXX11 0 +#endif + + // Do we support r-value references? #ifndef EIGEN_HAS_RVALUE_REFERENCES #if EIGEN_MAX_CPP_VER>=11 && \ From 3ecb343dc3f0be6c60654cec581d0f3145553238 Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Wed, 26 Oct 2016 22:50:41 +0200 Subject: [PATCH 3/3] Fix regression in X = (X*X.transpose())/s with X rectangular by deferring resizing of the destination after the creation of the evaluator of the source expression. --- Eigen/src/Core/AssignEvaluator.h | 36 ++++++++-------- Eigen/src/Core/DiagonalMatrix.h | 5 +++ Eigen/src/Core/ProductEvaluators.h | 12 ++++++ Eigen/src/Core/Solve.h | 16 ++++++- Eigen/src/Core/Transpose.h | 5 +++ Eigen/src/Core/TriangularMatrix.h | 18 +++++--- Eigen/src/Geometry/Homogeneous.h | 10 +++++ .../IterativeLinearSolvers/SolveWithGuess.h | 6 ++- Eigen/src/LU/InverseImpl.h | 6 ++- Eigen/src/SparseCore/SparseAssign.h | 17 +++++++- Eigen/src/SparseCore/SparseProduct.h | 5 +++ test/product_extra.cpp | 43 +++++++++++++++++++ 12 files changed, 151 insertions(+), 28 deletions(-) diff --git a/Eigen/src/Core/AssignEvaluator.h b/Eigen/src/Core/AssignEvaluator.h index abad8c790..ffe1dd0ca 100644 --- a/Eigen/src/Core/AssignEvaluator.h +++ b/Eigen/src/Core/AssignEvaluator.h @@ -697,15 +697,21 @@ protected: ***************************************************************************/ template -EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(const DstXprType& dst, const SrcXprType& src, const Functor &func) +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(DstXprType& dst, const SrcXprType& src, const Functor &func) { - eigen_assert(dst.rows() == src.rows() && dst.cols() == src.cols()); - typedef evaluator DstEvaluatorType; typedef evaluator SrcEvaluatorType; - DstEvaluatorType dstEvaluator(dst); SrcEvaluatorType srcEvaluator(src); + + // NOTE To properly handle A = (A*A.transpose())/s with A rectangular, + // we need to resize the destination after the source evaluator has been created. + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + + DstEvaluatorType dstEvaluator(dst); typedef generic_dense_assignment_kernel Kernel; Kernel kernel(dstEvaluator, srcEvaluator, func, dst.const_cast_derived()); @@ -714,7 +720,7 @@ EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(const DstX } template -EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(const DstXprType& dst, const SrcXprType& src) +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_dense_assignment_loop(DstXprType& dst, const SrcXprType& src) { call_dense_assignment_loop(dst, src, internal::assign_op()); } @@ -796,11 +802,6 @@ void call_assignment_no_alias(Dst& dst, const Src& src, const Func& func) ) && int(Dst::SizeAtCompileTime) != 1 }; - Index dstRows = NeedToTranspose ? src.cols() : src.rows(); - Index dstCols = NeedToTranspose ? src.rows() : src.cols(); - if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) - dst.resize(dstRows, dstCols); - typedef typename internal::conditional, Dst>::type ActualDstTypeCleaned; typedef typename internal::conditional, Dst&>::type ActualDstType; ActualDstType actualDst(dst); @@ -823,15 +824,11 @@ template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE void call_assignment_no_alias_no_transpose(Dst& dst, const Src& src, const Func& func) { - Index dstRows = src.rows(); - Index dstCols = src.cols(); - if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) - dst.resize(dstRows, dstCols); - // TODO check whether this is the right place to perform these checks: EIGEN_STATIC_ASSERT_LVALUE(Dst) EIGEN_STATIC_ASSERT_SAME_MATRIX_SIZE(Dst,Src) - + EIGEN_CHECK_BINARY_COMPATIBILIY(Func,typename Dst::Scalar,typename Src::Scalar); + Assignment::run(dst, src, func); } template @@ -853,8 +850,6 @@ struct Assignment EIGEN_DEVICE_FUNC static EIGEN_STRONG_INLINE void run(DstXprType &dst, const SrcXprType &src, const Functor &func) { - eigen_assert(dst.rows() == src.rows() && dst.cols() == src.cols()); - #ifndef EIGEN_NO_DEBUG internal::check_for_aliasing(dst, src); #endif @@ -873,6 +868,11 @@ struct Assignment EIGEN_DEVICE_FUNC static EIGEN_STRONG_INLINE void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &/*func*/) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + eigen_assert(dst.rows() == src.rows() && dst.cols() == src.cols()); src.evalTo(dst); } diff --git a/Eigen/src/Core/DiagonalMatrix.h b/Eigen/src/Core/DiagonalMatrix.h index c682c6d7f..ecfdce8ef 100644 --- a/Eigen/src/Core/DiagonalMatrix.h +++ b/Eigen/src/Core/DiagonalMatrix.h @@ -320,6 +320,11 @@ struct Assignment { static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &/*func*/) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + dst.setZero(); dst.diagonal() = src.diagonal(); } diff --git a/Eigen/src/Core/ProductEvaluators.h b/Eigen/src/Core/ProductEvaluators.h index dfd76990d..c9e2e1a07 100644 --- a/Eigen/src/Core/ProductEvaluators.h +++ b/Eigen/src/Core/ProductEvaluators.h @@ -140,6 +140,10 @@ struct Assignment, internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); // FIXME shall we handle nested_eval here? generic_product_impl::evalTo(dst, src.lhs(), src.rhs()); } @@ -154,6 +158,10 @@ struct Assignment, internal::add_assign_op< static EIGEN_STRONG_INLINE void run(DstXprType &dst, const SrcXprType &src, const internal::add_assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); // FIXME shall we handle nested_eval here? generic_product_impl::addTo(dst, src.lhs(), src.rhs()); } @@ -168,6 +176,10 @@ struct Assignment, internal::sub_assign_op< static EIGEN_STRONG_INLINE void run(DstXprType &dst, const SrcXprType &src, const internal::sub_assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); // FIXME shall we handle nested_eval here? generic_product_impl::subTo(dst, src.lhs(), src.rhs()); } diff --git a/Eigen/src/Core/Solve.h b/Eigen/src/Core/Solve.h index 8fc69c4b8..960a58597 100644 --- a/Eigen/src/Core/Solve.h +++ b/Eigen/src/Core/Solve.h @@ -139,7 +139,11 @@ struct Assignment, internal::assign_op SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { - // FIXME shall we resize dst here? + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + src.dec()._solve_impl(src.rhs(), dst); } }; @@ -151,6 +155,11 @@ struct Assignment,RhsType>, internal: typedef Solve,RhsType> SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + src.dec().nestedExpression().template _solve_impl_transposed(src.rhs(), dst); } }; @@ -163,6 +172,11 @@ struct Assignment, const Transpose >,RhsType> SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + src.dec().nestedExpression().nestedExpression().template _solve_impl_transposed(src.rhs(), dst); } }; diff --git a/Eigen/src/Core/Transpose.h b/Eigen/src/Core/Transpose.h index bc232526a..79b767bcc 100644 --- a/Eigen/src/Core/Transpose.h +++ b/Eigen/src/Core/Transpose.h @@ -78,6 +78,11 @@ template class Transpose typename internal::remove_reference::type& nestedExpression() { return m_matrix; } + /** \internal */ + void resize(Index nrows, Index ncols) { + m_matrix.resize(ncols,nrows); + } + protected: typename internal::ref_selector::non_const_type m_matrix; }; diff --git a/Eigen/src/Core/TriangularMatrix.h b/Eigen/src/Core/TriangularMatrix.h index 71f5d4f29..641c20417 100644 --- a/Eigen/src/Core/TriangularMatrix.h +++ b/Eigen/src/Core/TriangularMatrix.h @@ -775,15 +775,18 @@ public: template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE -void call_triangular_assignment_loop(const DstXprType& dst, const SrcXprType& src, const Functor &func) +void call_triangular_assignment_loop(DstXprType& dst, const SrcXprType& src, const Functor &func) { - eigen_assert(dst.rows() == src.rows() && dst.cols() == src.cols()); - typedef evaluator DstEvaluatorType; typedef evaluator SrcEvaluatorType; - DstEvaluatorType dstEvaluator(dst); SrcEvaluatorType srcEvaluator(src); + + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + DstEvaluatorType dstEvaluator(dst); typedef triangular_dense_assignment_kernel< Mode&(Lower|Upper),Mode&(UnitDiag|ZeroDiag|SelfAdjoint),SetOpposite, DstEvaluatorType,SrcEvaluatorType,Functor> Kernel; @@ -800,7 +803,7 @@ void call_triangular_assignment_loop(const DstXprType& dst, const SrcXprType& sr template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE -void call_triangular_assignment_loop(const DstXprType& dst, const SrcXprType& src) +void call_triangular_assignment_loop(DstXprType& dst, const SrcXprType& src) { call_triangular_assignment_loop(dst, src, internal::assign_op()); } @@ -936,6 +939,11 @@ struct Assignment, internal::assign_ typedef Product SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + dst.setZero(); dst._assignProduct(src, 1); } diff --git a/Eigen/src/Geometry/Homogeneous.h b/Eigen/src/Geometry/Homogeneous.h index 804e5da73..b4d7946e3 100644 --- a/Eigen/src/Geometry/Homogeneous.h +++ b/Eigen/src/Geometry/Homogeneous.h @@ -334,6 +334,11 @@ struct Assignment, internal::assign_op typedef Homogeneous SrcXprType; EIGEN_DEVICE_FUNC static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + dst.template topRows(src.nestedExpression().rows()) = src.nestedExpression(); dst.row(dst.rows()-1).setOnes(); } @@ -346,6 +351,11 @@ struct Assignment, internal::assign_ typedef Homogeneous SrcXprType; EIGEN_DEVICE_FUNC static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + dst.template leftCols(src.nestedExpression().cols()) = src.nestedExpression(); dst.col(dst.cols()-1).setOnes(); } diff --git a/Eigen/src/IterativeLinearSolvers/SolveWithGuess.h b/Eigen/src/IterativeLinearSolvers/SolveWithGuess.h index 0498db396..0ace45177 100644 --- a/Eigen/src/IterativeLinearSolvers/SolveWithGuess.h +++ b/Eigen/src/IterativeLinearSolvers/SolveWithGuess.h @@ -98,7 +98,11 @@ struct Assignment, interna typedef SolveWithGuess SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { - // FIXME shall we resize dst here? + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + dst = src.guess(); src.dec()._solve_with_guess_impl(src.rhs(), dst/*, src.guess()*/); } diff --git a/Eigen/src/LU/InverseImpl.h b/Eigen/src/LU/InverseImpl.h index 3134632e1..018f99b58 100644 --- a/Eigen/src/LU/InverseImpl.h +++ b/Eigen/src/LU/InverseImpl.h @@ -292,7 +292,11 @@ struct Assignment, internal::assign_op SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { - // FIXME shall we resize dst here? + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + const int Size = EIGEN_PLAIN_ENUM_MIN(XprType::ColsAtCompileTime,DstXprType::ColsAtCompileTime); EIGEN_ONLY_USED_FOR_DEBUG(Size); eigen_assert(( (Size<=1) || (Size>4) || (extract_data(src.nestedExpression())!=extract_data(dst))) diff --git a/Eigen/src/SparseCore/SparseAssign.h b/Eigen/src/SparseCore/SparseAssign.h index fa5386599..83776645b 100644 --- a/Eigen/src/SparseCore/SparseAssign.h +++ b/Eigen/src/SparseCore/SparseAssign.h @@ -139,13 +139,16 @@ struct Assignment { static void run(DstXprType &dst, const SrcXprType &src, const Functor &func) { - eigen_assert(dst.rows() == src.rows() && dst.cols() == src.cols()); - if(internal::is_same >::value) dst.setZero(); internal::evaluator srcEval(src); + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); internal::evaluator dstEval(dst); + const Index outerEvaluationSize = (internal::evaluator::Flags&RowMajorBit) ? src.rows() : src.cols(); for (Index j=0; j::InnerIterator i(srcEval,j); i; ++i) @@ -161,6 +164,11 @@ struct Assignment, internal::assign_op SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + src.dec()._solve_impl(src.rhs(), dst); } }; @@ -179,6 +187,11 @@ struct Assignment template static void run(SparseMatrix &dst, const SrcXprType &src, const internal::assign_op &/*func*/) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + Index size = src.diagonal().size(); dst.makeCompressed(); dst.resizeNonZeros(size); diff --git a/Eigen/src/SparseCore/SparseProduct.h b/Eigen/src/SparseCore/SparseProduct.h index 7a5ad0635..4cbf68781 100644 --- a/Eigen/src/SparseCore/SparseProduct.h +++ b/Eigen/src/SparseCore/SparseProduct.h @@ -104,6 +104,11 @@ struct Assignment, internal::assig typedef Product SrcXprType; static void run(DstXprType &dst, const SrcXprType &src, const internal::assign_op &) { + Index dstRows = src.rows(); + Index dstCols = src.cols(); + if((dst.rows()!=dstRows) || (dst.cols()!=dstCols)) + dst.resize(dstRows, dstCols); + generic_product_impl::evalTo(dst,src.lhs(),src.rhs()); } }; diff --git a/test/product_extra.cpp b/test/product_extra.cpp index e4990ac8c..03d5c3657 100644 --- a/test/product_extra.cpp +++ b/test/product_extra.cpp @@ -256,7 +256,49 @@ Index compute_block_size() return ret; } +template +void aliasing_with_resize() +{ + Index m = internal::random(10,50); + Index n = internal::random(10,50); + MatrixXd A, B, C(m,n), D(m,m); + VectorXd a, b, c(n); + C.setRandom(); + D.setRandom(); + c.setRandom(); + double s = internal::random(1,10); + A = C; + B = A * A.transpose(); + A = A * A.transpose(); + VERIFY_IS_APPROX(A,B); + + A = C; + B = (A * A.transpose())/s; + A = (A * A.transpose())/s; + VERIFY_IS_APPROX(A,B); + + A = C; + B = (A * A.transpose()) + D; + A = (A * A.transpose()) + D; + VERIFY_IS_APPROX(A,B); + + A = C; + B = D + (A * A.transpose()); + A = D + (A * A.transpose()); + VERIFY_IS_APPROX(A,B); + + A = C; + B = s * (A * A.transpose()); + A = s * (A * A.transpose()); + VERIFY_IS_APPROX(A,B); + + A = C; + a = c; + b = (A * a)/s; + a = (A * a)/s; + VERIFY_IS_APPROX(a,b); +} template void bug_1308() @@ -318,5 +360,6 @@ void test_product_extra() CALL_SUBTEST_7( compute_block_size() ); CALL_SUBTEST_7( compute_block_size() ); CALL_SUBTEST_7( compute_block_size >() ); + CALL_SUBTEST_8( aliasing_with_resize() ); }