From 61e58cf602849d4d71361fb7eaa25cb8b5a1196d Mon Sep 17 00:00:00 2001 From: Benoit Jacob Date: Sat, 5 Apr 2008 14:15:02 +0000 Subject: [PATCH] fixes as discussed with Gael on IRC. Mainly, in Fuzzy.h, and Dot.h, use ei_xpr_copy to evaluate args when needed. Had to introduce an ugly trick with ei_unref as when the XprCopy type is a reference one can't directly access member typedefs such as Scalar. --- Eigen/src/Core/Dot.h | 27 ++++++++++++++++----------- Eigen/src/Core/ForwardDeclarations.h | 5 ++++- Eigen/src/Core/Fuzzy.h | 15 ++++++++++----- Eigen/src/Core/MatrixBase.h | 2 +- Eigen/src/Core/OperatorEquals.h | 8 ++------ Eigen/src/Core/Product.h | 4 +--- Eigen/src/Core/Util.h | 4 +--- 7 files changed, 35 insertions(+), 30 deletions(-) diff --git a/Eigen/src/Core/Dot.h b/Eigen/src/Core/Dot.h index 248fbca79..ba45d5192 100644 --- a/Eigen/src/Core/Dot.h +++ b/Eigen/src/Core/Dot.h @@ -72,22 +72,24 @@ template typename ei_traits::Scalar MatrixBase::dot(const MatrixBase& other) const { + typename Derived::XprCopy xprCopy(derived()); + typename OtherDerived::XprCopy otherXprCopy(other.derived()); + ei_assert(IsVectorAtCompileTime && OtherDerived::IsVectorAtCompileTime - && size() == other.size()); + && xprCopy.size() == otherXprCopy.size()); Scalar res; - if(EIGEN_UNROLLED_LOOPS - && SizeAtCompileTime != Dynamic - && SizeAtCompileTime <= EIGEN_UNROLLING_LIMIT) + if(SizeAtCompileTime <= EIGEN_UNROLLING_LIMIT) ei_dot_unroller > - ::run(derived(), other, res); + typename ei_unref::type, + typename ei_unref::type> + ::run(xprCopy, otherXprCopy, res); else { - res = coeff(0) * ei_conj(other.coeff(0)); + res = xprCopy.coeff(0) * ei_conj(otherXprCopy.coeff(0)); for(int i = 1; i < size(); i++) - res += coeff(i)* ei_conj(other.coeff(i)); + res += xprCopy.coeff(i)* ei_conj(otherXprCopy.coeff(i)); } return res; } @@ -140,7 +142,9 @@ template bool MatrixBase::isOrtho (const MatrixBase& other, RealScalar prec) const { - return ei_abs2(dot(other)) <= prec * prec * norm2() * other.norm2(); + typename Derived::XprCopy xprCopy(derived()); + typename OtherDerived::XprCopy otherXprCopy(other.derived()); + return ei_abs2(xprCopy.dot(otherXprCopy)) <= prec * prec * xprCopy.norm2() * otherXprCopy.norm2(); } /** \returns true if *this is approximately an unitary matrix, @@ -157,12 +161,13 @@ bool MatrixBase::isOrtho template bool MatrixBase::isOrtho(RealScalar prec) const { + typename Derived::XprCopy xprCopy(derived()); for(int i = 0; i < cols(); i++) { - if(!ei_isApprox(col(i).norm2(), static_cast(1), prec)) + if(!ei_isApprox(xprCopy.col(i).norm2(), static_cast(1), prec)) return false; for(int j = 0; j < i; j++) - if(!ei_isMuchSmallerThan(col(i).dot(col(j)), static_cast(1), prec)) + if(!ei_isMuchSmallerThan(xprCopy.col(i).dot(xprCopy.col(j)), static_cast(1), prec)) return false; } return true; diff --git a/Eigen/src/Core/ForwardDeclarations.h b/Eigen/src/Core/ForwardDeclarations.h index bf82a0c0a..36519c7da 100644 --- a/Eigen/src/Core/ForwardDeclarations.h +++ b/Eigen/src/Core/ForwardDeclarations.h @@ -80,6 +80,9 @@ template struct ei_eval ei_traits::MaxColsAtCompileTime> type; }; +template struct ei_unref { typedef T type; }; +template struct ei_unref { typedef T type; }; + template struct ei_xpr_copy { typedef typename ei_meta_if< ei_traits::Flags & EvalBeforeNestingBit, @@ -95,7 +98,7 @@ template struct ei_eval_if_needed_before_nesting { // FIXME should we consider the additional store as well as the creation cost of the temporary ? enum { eval = T::Flags & EvalBeforeNestingBit - || n * NumTraits::Scalar>::ReadCost < (n-1) * T::CoeffReadCost }; + || (n+1) * NumTraits::Scalar>::ReadCost < (n-1) * T::CoeffReadCost }; typedef typename ei_meta_if::type, T>::ret XprType; typedef typename ei_meta_if::type, typename T::XprCopy>::ret CopyType; }; diff --git a/Eigen/src/Core/Fuzzy.h b/Eigen/src/Core/Fuzzy.h index 4ebe9d2a7..5dd0265ba 100644 --- a/Eigen/src/Core/Fuzzy.h +++ b/Eigen/src/Core/Fuzzy.h @@ -44,7 +44,7 @@ template template bool MatrixBase::isApprox( - const OtherDerived& other, + const MatrixBase& other, typename NumTraits::Real prec ) const { @@ -55,9 +55,11 @@ bool MatrixBase::isApprox( } else { + typename Derived::XprCopy xprCopy(derived()); + typename OtherDerived::XprCopy otherXprCopy(other.derived()); for(int i = 0; i < cols(); i++) - if((col(i) - other.col(i)).norm2() - > std::min(col(i).norm2(), other.col(i).norm2()) * prec * prec) + if((xprCopy.col(i) - otherXprCopy.col(i)).norm2() + > std::min(xprCopy.col(i).norm2(), otherXprCopy.col(i).norm2()) * prec * prec) return false; return true; } @@ -85,8 +87,9 @@ bool MatrixBase::isMuchSmallerThan( } else { + typename Derived::XprCopy xprCopy(*this); for(int i = 0; i < cols(); i++) - if(col(i).norm2() > ei_abs2(other * prec)) + if(xprCopy.col(i).norm2() > ei_abs2(other * prec)) return false; return true; } @@ -116,8 +119,10 @@ bool MatrixBase::isMuchSmallerThan( } else { + typename Derived::XprCopy xprCopy(*this); + typename OtherDerived::XprCopy otherXprCopy(other); for(int i = 0; i < cols(); i++) - if(col(i).norm2() > other.col(i).norm2() * prec * prec) + if(xprCopy.col(i).norm2() > otherXprCopy.col(i).norm2() * prec * prec) return false; return true; } diff --git a/Eigen/src/Core/MatrixBase.h b/Eigen/src/Core/MatrixBase.h index 7720433d0..2bc54701d 100644 --- a/Eigen/src/Core/MatrixBase.h +++ b/Eigen/src/Core/MatrixBase.h @@ -340,7 +340,7 @@ template class MatrixBase /// \name Comparison and diagnostic //@{ template - bool isApprox(const OtherDerived& other, + bool isApprox(const MatrixBase& other, RealScalar prec = precision()) const; bool isMuchSmallerThan(const RealScalar& other, RealScalar prec = precision()) const; diff --git a/Eigen/src/Core/OperatorEquals.h b/Eigen/src/Core/OperatorEquals.h index 0bed89b7e..c93a9329f 100644 --- a/Eigen/src/Core/OperatorEquals.h +++ b/Eigen/src/Core/OperatorEquals.h @@ -106,9 +106,7 @@ Derived& MatrixBase // copying a vector expression into a vector { ei_assert(size() == other.size()); - if(EIGEN_UNROLLED_LOOPS - && SizeAtCompileTime != Dynamic - && SizeAtCompileTime <= EIGEN_UNROLLING_LIMIT) + if(SizeAtCompileTime <= EIGEN_UNROLLING_LIMIT) ei_vector_operator_equals_unroller else // copying a matrix expression into a matrix { ei_assert(rows() == other.rows() && cols() == other.cols()); - if(EIGEN_UNROLLED_LOOPS - && SizeAtCompileTime != Dynamic - && SizeAtCompileTime <= EIGEN_UNROLLING_LIMIT) + if(SizeAtCompileTime <= EIGEN_UNROLLING_LIMIT) { ei_matrix_operator_equals_unroller class Product : ei_no_assignm const Scalar _coeff(int row, int col) const { Scalar res; - if(EIGEN_UNROLLED_LOOPS - && Lhs::ColsAtCompileTime != Dynamic - && Lhs::ColsAtCompileTime <= EIGEN_UNROLLING_LIMIT) + if(Lhs::ColsAtCompileTime <= EIGEN_UNROLLING_LIMIT) ei_product_unroller