From 2e83cbbba98da3a3f645133352a996b3f6daaed0 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Tue, 16 Mar 2021 17:02:50 +0000 Subject: Add NaN propagation options to minCoeff/maxCoeff visitors. --- Eigen/src/Core/DenseBase.h | 38 ++++++++++++--- Eigen/src/Core/Visitor.h | 118 ++++++++++++++++++++++++++++++++++++--------- test/visitor.cpp | 66 +++++++++++++++++++++++-- 3 files changed, 190 insertions(+), 32 deletions(-) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index 20cc4821a..7193e60aa 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -459,22 +459,48 @@ template class DenseBase // used. // TODO(rmlarsen): Replace with default template argument when we move to // c++11 or beyond. - EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const { + EIGEN_DEVICE_FUNC inline typename internal::traits::Scalar minCoeff() const { return minCoeff(); } - EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const { + EIGEN_DEVICE_FUNC inline typename internal::traits::Scalar maxCoeff() const { return maxCoeff(); } - template EIGEN_DEVICE_FUNC + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff(IndexType* row, IndexType* col) const; - template EIGEN_DEVICE_FUNC + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff(IndexType* row, IndexType* col) const; - template EIGEN_DEVICE_FUNC + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff(IndexType* index) const; - template EIGEN_DEVICE_FUNC + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff(IndexType* index) const; + // TODO(rmlarsen): Replace these methods with a default template argument. + template + EIGEN_DEVICE_FUNC inline + typename internal::traits::Scalar minCoeff(IndexType* row, IndexType* col) const { + return minCoeff(row, col); + } + template + EIGEN_DEVICE_FUNC inline + typename internal::traits::Scalar maxCoeff(IndexType* row, IndexType* col) const { + return maxCoeff(row, col); + } + template + EIGEN_DEVICE_FUNC inline + typename internal::traits::Scalar minCoeff(IndexType* index) const { + return minCoeff(index); + } + template + EIGEN_DEVICE_FUNC inline + typename internal::traits::Scalar maxCoeff(IndexType* index) const { + return maxCoeff(index); + } + template EIGEN_DEVICE_FUNC Scalar redux(const BinaryOp& func) const; diff --git a/Eigen/src/Core/Visitor.h b/Eigen/src/Core/Visitor.h index 67a69c54f..e6ba8a098 100644 --- a/Eigen/src/Core/Visitor.h +++ b/Eigen/src/Core/Visitor.h @@ -157,7 +157,7 @@ struct coeff_visitor * * \sa DenseBase::minCoeff(Index*, Index*) */ -template +template struct min_coeff_visitor : coeff_visitor { typedef typename Derived::Scalar Scalar; @@ -173,8 +173,40 @@ struct min_coeff_visitor : coeff_visitor } }; -template -struct functor_traits > { +template +struct min_coeff_visitor : coeff_visitor +{ + typedef typename Derived::Scalar Scalar; + EIGEN_DEVICE_FUNC + void operator() (const Scalar& value, Index i, Index j) + { + if((numext::isnan)(this->res) || (!(numext::isnan)(value) && value < this->res)) + { + this->res = value; + this->row = i; + this->col = j; + } + } +}; + +template +struct min_coeff_visitor : coeff_visitor +{ + typedef typename Derived::Scalar Scalar; + EIGEN_DEVICE_FUNC + void operator() (const Scalar& value, Index i, Index j) + { + if((numext::isnan)(value) || value < this->res) + { + this->res = value; + this->row = i; + this->col = j; + } + } +}; + +template + struct functor_traits > { enum { Cost = NumTraits::AddCost }; @@ -185,7 +217,7 @@ struct functor_traits > { * * \sa DenseBase::maxCoeff(Index*, Index*) */ -template +template struct max_coeff_visitor : coeff_visitor { typedef typename Derived::Scalar Scalar; @@ -201,8 +233,40 @@ struct max_coeff_visitor : coeff_visitor } }; -template -struct functor_traits > { +template +struct max_coeff_visitor : coeff_visitor +{ + typedef typename Derived::Scalar Scalar; + EIGEN_DEVICE_FUNC + void operator() (const Scalar& value, Index i, Index j) + { + if((numext::isnan)(this->res) || (!(numext::isnan)(value) && value > this->res)) + { + this->res = value; + this->row = i; + this->col = j; + } + } +}; + +template +struct max_coeff_visitor : coeff_visitor +{ + typedef typename Derived::Scalar Scalar; + EIGEN_DEVICE_FUNC + void operator() (const Scalar& value, Index i, Index j) + { + if((numext::isnan)(value) || value > this->res) + { + this->res = value; + this->row = i; + this->col = j; + } + } +}; + +template +struct functor_traits > { enum { Cost = NumTraits::AddCost }; @@ -212,22 +276,24 @@ struct functor_traits > { /** \fn DenseBase::minCoeff(IndexType* rowId, IndexType* colId) const * \returns the minimum of all coefficients of *this and puts in *row and *col its location. - * + * + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is maximum of elements that are not NaN * \warning the matrix must be not empty, otherwise an assertion is triggered. - * - * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::minCoeff(Index*), DenseBase::maxCoeff(Index*,Index*), DenseBase::visit(), DenseBase::minCoeff() */ template -template +template EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::minCoeff(IndexType* rowId, IndexType* colId) const { eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); - internal::min_coeff_visitor minVisitor; + internal::min_coeff_visitor minVisitor; this->visit(minVisitor); *rowId = minVisitor.row; if (colId) *colId = minVisitor.col; @@ -236,14 +302,16 @@ DenseBase::minCoeff(IndexType* rowId, IndexType* colId) const /** \returns the minimum of all coefficients of *this and puts in *index its location. * + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is maximum of elements that are not NaN * \warning the matrix must be not empty, otherwise an assertion is triggered. - * - * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::minCoeff(IndexType*,IndexType*), DenseBase::maxCoeff(IndexType*,IndexType*), DenseBase::visit(), DenseBase::minCoeff() */ template -template +template EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::minCoeff(IndexType* index) const @@ -251,7 +319,7 @@ DenseBase::minCoeff(IndexType* index) const eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived) - internal::min_coeff_visitor minVisitor; + internal::min_coeff_visitor minVisitor; this->visit(minVisitor); *index = IndexType((RowsAtCompileTime==1) ? minVisitor.col : minVisitor.row); return minVisitor.res; @@ -260,21 +328,23 @@ DenseBase::minCoeff(IndexType* index) const /** \fn DenseBase::maxCoeff(IndexType* rowId, IndexType* colId) const * \returns the maximum of all coefficients of *this and puts in *row and *col its location. * + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is maximum of elements that are not NaN * \warning the matrix must be not empty, otherwise an assertion is triggered. - * - * \warning the result is undefined if \c *this contains NaN. * * \sa DenseBase::minCoeff(IndexType*,IndexType*), DenseBase::visit(), DenseBase::maxCoeff() */ template -template +template EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::maxCoeff(IndexType* rowPtr, IndexType* colPtr) const { eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); - internal::max_coeff_visitor maxVisitor; + internal::max_coeff_visitor maxVisitor; this->visit(maxVisitor); *rowPtr = maxVisitor.row; if (colPtr) *colPtr = maxVisitor.col; @@ -283,14 +353,16 @@ DenseBase::maxCoeff(IndexType* rowPtr, IndexType* colPtr) const /** \returns the maximum of all coefficients of *this and puts in *index its location. * + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is maximum of elements that are not NaN * \warning the matrix must be not empty, otherwise an assertion is triggered. * - * \warning the result is undefined if \c *this contains NaN. - * * \sa DenseBase::maxCoeff(IndexType*,IndexType*), DenseBase::minCoeff(IndexType*,IndexType*), DenseBase::visitor(), DenseBase::maxCoeff() */ template -template +template EIGEN_DEVICE_FUNC typename internal::traits::Scalar DenseBase::maxCoeff(IndexType* index) const @@ -298,7 +370,7 @@ DenseBase::maxCoeff(IndexType* index) const eigen_assert(this->rows()>0 && this->cols()>0 && "you are using an empty matrix"); EIGEN_STATIC_ASSERT_VECTOR_ONLY(Derived) - internal::max_coeff_visitor maxVisitor; + internal::max_coeff_visitor maxVisitor; this->visit(maxVisitor); *index = (RowsAtCompileTime==1) ? maxVisitor.col : maxVisitor.row; return maxVisitor.res; diff --git a/test/visitor.cpp b/test/visitor.cpp index de938fc95..20fb2c3ed 100644 --- a/test/visitor.cpp +++ b/test/visitor.cpp @@ -56,9 +56,44 @@ template void matrixVisitor(const MatrixType& p) VERIFY_IS_APPROX(maxc, m.maxCoeff()); eigen_maxc = (m.adjoint()*m).maxCoeff(&eigen_maxrow,&eigen_maxcol); - eigen_maxc = (m.adjoint()*m).eval().maxCoeff(&maxrow,&maxcol); - VERIFY(maxrow == eigen_maxrow); - VERIFY(maxcol == eigen_maxcol); + Index maxrow2=0,maxcol2=0; + eigen_maxc = (m.adjoint()*m).eval().maxCoeff(&maxrow2,&maxcol2); + VERIFY(maxrow2 == eigen_maxrow); + VERIFY(maxcol2 == eigen_maxcol); + + if (!NumTraits::IsInteger && m.size() > 2) { + // Test NaN propagation by replacing an element with NaN. + bool stop = false; + for (Index j = 0; j < cols && !stop; ++j) { + for (Index i = 0; i < rows && !stop; ++i) { + if (!(j == mincol && i == minrow) && + !(j == maxcol && i == maxrow)) { + m(i,j) = NumTraits::quiet_NaN(); + stop = true; + break; + } + } + } + + eigen_minc = m.template minCoeff(&eigen_minrow, &eigen_mincol); + eigen_maxc = m.template maxCoeff(&eigen_maxrow, &eigen_maxcol); + VERIFY(minrow == eigen_minrow); + VERIFY(maxrow == eigen_maxrow); + VERIFY(mincol == eigen_mincol); + VERIFY(maxcol == eigen_maxcol); + VERIFY_IS_APPROX(minc, eigen_minc); + VERIFY_IS_APPROX(maxc, eigen_maxc); + VERIFY_IS_APPROX(minc, m.template minCoeff()); + VERIFY_IS_APPROX(maxc, m.template maxCoeff()); + + eigen_minc = m.template minCoeff(&eigen_minrow, &eigen_mincol); + eigen_maxc = m.template maxCoeff(&eigen_maxrow, &eigen_maxcol); + VERIFY(minrow != eigen_minrow || mincol != eigen_mincol); + VERIFY(maxrow != eigen_maxrow || maxcol != eigen_maxcol); + VERIFY((numext::isnan)(eigen_minc)); + VERIFY((numext::isnan)(eigen_maxc)); + } + } template void vectorVisitor(const VectorType& w) @@ -111,6 +146,31 @@ template void vectorVisitor(const VectorType& w) v2.maxCoeff(&eigen_maxidx); VERIFY(eigen_minidx == (std::min)(idx0,idx1)); VERIFY(eigen_maxidx == (std::min)(idx0,idx2)); + + if (!NumTraits::IsInteger && size > 2) { + // Test NaN propagation by replacing an element with NaN. + for (Index i = 0; i < size; ++i) { + if (i != minidx && i != maxidx) { + v(i) = NumTraits::quiet_NaN(); + break; + } + } + eigen_minc = v.template minCoeff(&eigen_minidx); + eigen_maxc = v.template maxCoeff(&eigen_maxidx); + VERIFY(minidx == eigen_minidx); + VERIFY(maxidx == eigen_maxidx); + VERIFY_IS_APPROX(minc, eigen_minc); + VERIFY_IS_APPROX(maxc, eigen_maxc); + VERIFY_IS_APPROX(minc, v.template minCoeff()); + VERIFY_IS_APPROX(maxc, v.template maxCoeff()); + + eigen_minc = v.template minCoeff(&eigen_minidx); + eigen_maxc = v.template maxCoeff(&eigen_maxidx); + VERIFY(minidx != eigen_minidx); + VERIFY(maxidx != eigen_maxidx); + VERIFY((numext::isnan)(eigen_minc)); + VERIFY((numext::isnan)(eigen_maxc)); + } } EIGEN_DECLARE_TEST(visitor) -- cgit v1.2.3