From 841c8986f8de0b4984b234b1bde49b9418dba7a6 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Wed, 24 Feb 2021 17:49:20 -0800 Subject: [PATCH 1/7] Make it possible to specify NaN propagation strategy for maxCoeff/minCoeff reductions. --- Eigen/src/Core/DenseBase.h | 5 +++++ Eigen/src/Core/Redux.h | 30 ++++++++++++++++++++++++++++++ test/array_cwise.cpp | 14 ++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index 767a8e274..c83a3fcc6 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -452,6 +452,11 @@ template class DenseBase EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + template EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff(IndexType* row, IndexType* col) const; template EIGEN_DEVICE_FUNC diff --git a/Eigen/src/Core/Redux.h b/Eigen/src/Core/Redux.h index 2eef5abc5..4e5affc43 100644 --- a/Eigen/src/Core/Redux.h +++ b/Eigen/src/Core/Redux.h @@ -429,6 +429,21 @@ DenseBase::minCoeff() const return derived().redux(Eigen::internal::scalar_min_op()); } +/** \returns the minimum of all coefficients of \c *this. + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is minimum of elements that are not NaN + * \warning the matrix must be not empty, otherwise an assertion is triggered. + */ +template +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar +DenseBase::minCoeff() const +{ + return derived().redux(Eigen::internal::scalar_min_op()); +} + /** \returns the maximum of all coefficients of \c *this. * \warning the matrix must be not empty, otherwise an assertion is triggered. * \warning the result is undefined if \c *this contains NaN. @@ -440,6 +455,21 @@ DenseBase::maxCoeff() const return derived().redux(Eigen::internal::scalar_max_op()); } +/** \returns the maximum of all coefficients of \c *this. + * 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. + */ +template +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar +DenseBase::maxCoeff() const +{ + return derived().redux(Eigen::internal::scalar_max_op()); +} + /** \returns the sum of all coefficients of \c *this * * If \c *this is empty, then the value 0 is returned. diff --git a/test/array_cwise.cpp b/test/array_cwise.cpp index 7f7e44f89..92abf6968 100644 --- a/test/array_cwise.cpp +++ b/test/array_cwise.cpp @@ -610,6 +610,20 @@ template void min_max(const ArrayType& m) VERIFY_IS_APPROX(ArrayType::Constant(rows,cols, maxM1), (m1.max)( maxM1)); VERIFY_IS_APPROX(m1, (m1.max)( minM1)); + + // min/max with various NaN propagation options. + if (m1.size() > 1 && !NumTraits::IsInteger) { + m1(0,0) = std::numeric_limits::quiet_NaN(); + maxM1 = m1.template maxCoeff(); + minM1 = m1.template minCoeff(); + VERIFY((numext::isnan)(maxM1)); + VERIFY((numext::isnan)(minM1)); + + maxM1 = m1.template maxCoeff(); + minM1 = m1.template minCoeff(); + VERIFY(!(numext::isnan)(maxM1)); + VERIFY(!(numext::isnan)(minM1)); + } } EIGEN_DECLARE_TEST(array_cwise) From 0065f9d3225def945333ca33da0b020a95ce8c36 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Wed, 24 Feb 2021 17:49:20 -0800 Subject: [PATCH 2/7] Make it possible to specify NaN propagation strategy for maxCoeff/minCoeff reductions. --- Eigen/src/Core/DenseBase.h | 5 +++++ Eigen/src/Core/Redux.h | 30 ++++++++++++++++++++++++++++++ test/array_cwise.cpp | 14 ++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index 767a8e274..c83a3fcc6 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -452,6 +452,11 @@ template class DenseBase EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + template EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff(IndexType* row, IndexType* col) const; template EIGEN_DEVICE_FUNC diff --git a/Eigen/src/Core/Redux.h b/Eigen/src/Core/Redux.h index 2eef5abc5..4e5affc43 100644 --- a/Eigen/src/Core/Redux.h +++ b/Eigen/src/Core/Redux.h @@ -429,6 +429,21 @@ DenseBase::minCoeff() const return derived().redux(Eigen::internal::scalar_min_op()); } +/** \returns the minimum of all coefficients of \c *this. + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is minimum of elements that are not NaN + * \warning the matrix must be not empty, otherwise an assertion is triggered. + */ +template +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar +DenseBase::minCoeff() const +{ + return derived().redux(Eigen::internal::scalar_min_op()); +} + /** \returns the maximum of all coefficients of \c *this. * \warning the matrix must be not empty, otherwise an assertion is triggered. * \warning the result is undefined if \c *this contains NaN. @@ -440,6 +455,21 @@ DenseBase::maxCoeff() const return derived().redux(Eigen::internal::scalar_max_op()); } +/** \returns the maximum of all coefficients of \c *this. + * 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. + */ +template +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar +DenseBase::maxCoeff() const +{ + return derived().redux(Eigen::internal::scalar_max_op()); +} + /** \returns the sum of all coefficients of \c *this * * If \c *this is empty, then the value 0 is returned. diff --git a/test/array_cwise.cpp b/test/array_cwise.cpp index 7f7e44f89..92abf6968 100644 --- a/test/array_cwise.cpp +++ b/test/array_cwise.cpp @@ -610,6 +610,20 @@ template void min_max(const ArrayType& m) VERIFY_IS_APPROX(ArrayType::Constant(rows,cols, maxM1), (m1.max)( maxM1)); VERIFY_IS_APPROX(m1, (m1.max)( minM1)); + + // min/max with various NaN propagation options. + if (m1.size() > 1 && !NumTraits::IsInteger) { + m1(0,0) = std::numeric_limits::quiet_NaN(); + maxM1 = m1.template maxCoeff(); + minM1 = m1.template minCoeff(); + VERIFY((numext::isnan)(maxM1)); + VERIFY((numext::isnan)(minM1)); + + maxM1 = m1.template maxCoeff(); + minM1 = m1.template minCoeff(); + VERIFY(!(numext::isnan)(maxM1)); + VERIFY(!(numext::isnan)(minM1)); + } } EIGEN_DECLARE_TEST(array_cwise) From 4cb0592af706df224f6fed40bcf0bf56326a66f4 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Wed, 24 Feb 2021 17:59:36 -0800 Subject: [PATCH 3/7] Fix indentation. --- Eigen/src/Core/DenseBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index c83a3fcc6..94dddd27d 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -453,7 +453,7 @@ template class DenseBase EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; template - EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; template EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; From 5297b7162a0630e2e5b1459fa665c9f3b1eb532a Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Wed, 24 Feb 2021 17:49:20 -0800 Subject: [PATCH 4/7] Make it possible to specify NaN propagation strategy for maxCoeff/minCoeff reductions. --- Eigen/src/Core/DenseBase.h | 5 +++++ Eigen/src/Core/Redux.h | 30 ++++++++++++++++++++++++++++++ test/array_cwise.cpp | 14 ++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index 767a8e274..c83a3fcc6 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -452,6 +452,11 @@ template class DenseBase EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; + template + EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + template EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff(IndexType* row, IndexType* col) const; template EIGEN_DEVICE_FUNC diff --git a/Eigen/src/Core/Redux.h b/Eigen/src/Core/Redux.h index 2eef5abc5..4e5affc43 100644 --- a/Eigen/src/Core/Redux.h +++ b/Eigen/src/Core/Redux.h @@ -429,6 +429,21 @@ DenseBase::minCoeff() const return derived().redux(Eigen::internal::scalar_min_op()); } +/** \returns the minimum of all coefficients of \c *this. + * In case \c *this contains NaN, NaNPropagation determines the behavior: + * NaNPropagation == PropagateFast : undefined + * NaNPropagation == PropagateNaN : result is NaN + * NaNPropagation == PropagateNumbers : result is minimum of elements that are not NaN + * \warning the matrix must be not empty, otherwise an assertion is triggered. + */ +template +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar +DenseBase::minCoeff() const +{ + return derived().redux(Eigen::internal::scalar_min_op()); +} + /** \returns the maximum of all coefficients of \c *this. * \warning the matrix must be not empty, otherwise an assertion is triggered. * \warning the result is undefined if \c *this contains NaN. @@ -440,6 +455,21 @@ DenseBase::maxCoeff() const return derived().redux(Eigen::internal::scalar_max_op()); } +/** \returns the maximum of all coefficients of \c *this. + * 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. + */ +template +template +EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar +DenseBase::maxCoeff() const +{ + return derived().redux(Eigen::internal::scalar_max_op()); +} + /** \returns the sum of all coefficients of \c *this * * If \c *this is empty, then the value 0 is returned. diff --git a/test/array_cwise.cpp b/test/array_cwise.cpp index 7f7e44f89..92abf6968 100644 --- a/test/array_cwise.cpp +++ b/test/array_cwise.cpp @@ -610,6 +610,20 @@ template void min_max(const ArrayType& m) VERIFY_IS_APPROX(ArrayType::Constant(rows,cols, maxM1), (m1.max)( maxM1)); VERIFY_IS_APPROX(m1, (m1.max)( minM1)); + + // min/max with various NaN propagation options. + if (m1.size() > 1 && !NumTraits::IsInteger) { + m1(0,0) = std::numeric_limits::quiet_NaN(); + maxM1 = m1.template maxCoeff(); + minM1 = m1.template minCoeff(); + VERIFY((numext::isnan)(maxM1)); + VERIFY((numext::isnan)(minM1)); + + maxM1 = m1.template maxCoeff(); + minM1 = m1.template minCoeff(); + VERIFY(!(numext::isnan)(maxM1)); + VERIFY(!(numext::isnan)(minM1)); + } } EIGEN_DECLARE_TEST(array_cwise) From 51eba8c3e2567adbce53dfc9e62bddf294220fcc Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Wed, 24 Feb 2021 17:59:36 -0800 Subject: [PATCH 5/7] Fix indentation. --- Eigen/src/Core/DenseBase.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index c83a3fcc6..94dddd27d 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -453,7 +453,7 @@ template class DenseBase EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; template - EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; template EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; From fb5b59641aef640f154d2c3fc863885f68fc6034 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Fri, 26 Feb 2021 09:07:00 -0800 Subject: [PATCH 6/7] Defer default for minCoeff/maxCoeff to templated variant. --- Eigen/src/Core/DenseBase.h | 16 ++++++++++++---- Eigen/src/Core/Redux.h | 22 ---------------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index 94dddd27d..eeb1a2e8f 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -449,13 +449,21 @@ template class DenseBase EIGEN_DEVICE_FUNC Scalar prod() const; + template EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; + template EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; - template - EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const; - template - EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const; + // By default, the fastest version with undefined NaN propagation semantics is + // used. + EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff() const { + return minCoeff(); + } + EIGEN_DEVICE_FUNC typename internal::traits::Scalar maxCoeff() const { + return maxCoeff(); + } + + template EIGEN_DEVICE_FUNC typename internal::traits::Scalar minCoeff(IndexType* row, IndexType* col) const; diff --git a/Eigen/src/Core/Redux.h b/Eigen/src/Core/Redux.h index 4e5affc43..30598f415 100644 --- a/Eigen/src/Core/Redux.h +++ b/Eigen/src/Core/Redux.h @@ -418,17 +418,6 @@ DenseBase::redux(const Func& func) const return internal::redux_impl::run(thisEval, func, derived()); } -/** \returns the minimum of all coefficients of \c *this. - * \warning the matrix must be not empty, otherwise an assertion is triggered. - * \warning the result is undefined if \c *this contains NaN. - */ -template -EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar -DenseBase::minCoeff() const -{ - return derived().redux(Eigen::internal::scalar_min_op()); -} - /** \returns the minimum of all coefficients of \c *this. * In case \c *this contains NaN, NaNPropagation determines the behavior: * NaNPropagation == PropagateFast : undefined @@ -444,17 +433,6 @@ DenseBase::minCoeff() const return derived().redux(Eigen::internal::scalar_min_op()); } -/** \returns the maximum of all coefficients of \c *this. - * \warning the matrix must be not empty, otherwise an assertion is triggered. - * \warning the result is undefined if \c *this contains NaN. - */ -template -EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE typename internal::traits::Scalar -DenseBase::maxCoeff() const -{ - return derived().redux(Eigen::internal::scalar_max_op()); -} - /** \returns the maximum of all coefficients of \c *this. * In case \c *this contains NaN, NaNPropagation determines the behavior: * NaNPropagation == PropagateFast : undefined From 5e7d4c33d637866ba4c8eef46e4826ace1bf6402 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen Date: Fri, 26 Feb 2021 09:08:45 -0800 Subject: [PATCH 7/7] Add TODO. --- Eigen/src/Core/DenseBase.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Eigen/src/Core/DenseBase.h b/Eigen/src/Core/DenseBase.h index eeb1a2e8f..f28c47877 100644 --- a/Eigen/src/Core/DenseBase.h +++ b/Eigen/src/Core/DenseBase.h @@ -456,6 +456,8 @@ template class DenseBase // By default, the fastest version with undefined NaN propagation semantics is // 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 { return minCoeff(); }