From 218c37beb4dbc701938da71acf0bd3914f34f89e Mon Sep 17 00:00:00 2001 From: Gael Guennebaud Date: Wed, 31 Aug 2016 15:45:25 +0200 Subject: [PATCH] bug #1286: automatically detect the available prototypes of functors passed to CwiseNullaryExpr such that functors have only to implement the operators that matters among: operator()() operator()(i) operator()(i,j) Linear access is also automatically detected based on the availability of operator()(i,j). --- Eigen/src/Core/CoreEvaluators.h | 61 +++++++++++++++++++++-- Eigen/src/Core/CwiseNullaryOp.h | 27 +--------- Eigen/src/Core/Random.h | 3 +- Eigen/src/Core/functors/NullaryFunctors.h | 37 ++++---------- Eigen/src/Core/util/Meta.h | 43 ++++++++++++++++ Eigen/src/Core/util/XprHelper.h | 10 ---- test/nullary.cpp | 34 +++++++++++++ 7 files changed, 145 insertions(+), 70 deletions(-) diff --git a/Eigen/src/Core/CoreEvaluators.h b/Eigen/src/Core/CoreEvaluators.h index 7ba92963c..01678c6c2 100644 --- a/Eigen/src/Core/CoreEvaluators.h +++ b/Eigen/src/Core/CoreEvaluators.h @@ -337,6 +337,56 @@ protected: // Like Matrix and Array, this is not really a unary expression, so we directly specialize evaluator. // Likewise, there is not need to more sophisticated dispatching here. +template::value, + bool has_unary = has_unary_operator::value, + bool has_binary = has_binary_operator::value> +struct nullary_wrapper +{ + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar operator()(const NullaryOp& op, Index i, Index j) const { return op(i,j); } + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar operator()(const NullaryOp& op, Index i) const { return op(i); } + + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE T packetOp(const NullaryOp& op, Index i, Index j) const { return op.template packetOp(i,j); } + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE T packetOp(const NullaryOp& op, Index i) const { return op.template packetOp(i); } +}; + +template +struct nullary_wrapper +{ + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar operator()(const NullaryOp& op, ...) const { return op(); } + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE T packetOp(const NullaryOp& op, ...) const { return op.template packetOp(); } +}; + +template +struct nullary_wrapper +{ + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar operator()(const NullaryOp& op, Index i, Index j=0) const { return op(i,j); } + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE T packetOp(const NullaryOp& op, Index i, Index j=0) const { return op.template packetOp(i,j); } +}; + +// We need the following specialization for vector-only functors assigned to a runtime vector, +// for instance, using linspace and assigning a RowVectorXd to a MatrixXd or even a row of a MatrixXd. +// In this case, i==0 and j is used for the actual iteration. +template +struct nullary_wrapper + : nullary_wrapper // to get the identity wrapper +{ + typedef nullary_wrapper base; + using base::operator(); + using base::packetOp; + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Scalar operator()(const NullaryOp& op, Index i, Index j) const { + eigen_assert(i==0 || j==0); + return op(i+j); + } + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE T packetOp(const NullaryOp& op, Index i, Index j) const { + eigen_assert(i==0 || j==0); + return op.template packetOp(i+j); + } +}; + +template +struct nullary_wrapper {}; + template struct evaluator > : evaluator_base > @@ -356,7 +406,7 @@ struct evaluator > }; EIGEN_DEVICE_FUNC explicit evaluator(const XprType& n) - : m_functor(n.functor()) + : m_functor(n.functor()), m_wrapper() { EIGEN_INTERNAL_CHECK_COST_VALUE(CoeffReadCost); } @@ -366,31 +416,32 @@ struct evaluator > EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE CoeffReturnType coeff(Index row, Index col) const { - return m_functor(row, col); + return m_wrapper(m_functor, row, col); } EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE CoeffReturnType coeff(Index index) const { - return m_functor(index); + return m_wrapper(m_functor,index); } template EIGEN_STRONG_INLINE PacketType packet(Index row, Index col) const { - return m_functor.template packetOp(row, col); + return m_wrapper.template packetOp(m_functor,row, col); } template EIGEN_STRONG_INLINE PacketType packet(Index index) const { - return m_functor.template packetOp(index); + return m_wrapper.template packetOp(m_functor,index); } protected: const NullaryOp m_functor; + const internal::nullary_wrapper m_wrapper; }; // -------------------- CwiseUnaryOp -------------------- diff --git a/Eigen/src/Core/CwiseNullaryOp.h b/Eigen/src/Core/CwiseNullaryOp.h index bc3b3f240..ad6b1923d 100644 --- a/Eigen/src/Core/CwiseNullaryOp.h +++ b/Eigen/src/Core/CwiseNullaryOp.h @@ -20,7 +20,8 @@ struct traits > : traits::Flags & RowMajorBit }; }; -} + +} // namespace internal /** \class CwiseNullaryOp * \ingroup Core_Module @@ -70,30 +71,6 @@ class CwiseNullaryOp : public internal::dense_xpr_base< CwiseNullaryOp - EIGEN_STRONG_INLINE PacketScalar packet(Index rowId, Index colId) const - { - return m_functor.packetOp(rowId, colId); - } - - EIGEN_DEVICE_FUNC - EIGEN_STRONG_INLINE const Scalar coeff(Index index) const - { - return m_functor(index); - } - - template - EIGEN_STRONG_INLINE PacketScalar packet(Index index) const - { - return m_functor.packetOp(index); - } - /** \returns the functor representing the nullary operation */ EIGEN_DEVICE_FUNC const NullaryOp& functor() const { return m_functor; } diff --git a/Eigen/src/Core/Random.h b/Eigen/src/Core/Random.h index 02038e9e3..6faf789c7 100644 --- a/Eigen/src/Core/Random.h +++ b/Eigen/src/Core/Random.h @@ -16,8 +16,7 @@ namespace internal { template struct scalar_random_op { EIGEN_EMPTY_STRUCT_CTOR(scalar_random_op) - template - inline const Scalar operator() (Index, Index = 0) const { return random(); } + inline const Scalar operator() () const { return random(); } }; template diff --git a/Eigen/src/Core/functors/NullaryFunctors.h b/Eigen/src/Core/functors/NullaryFunctors.h index eaa582f23..692242f7d 100644 --- a/Eigen/src/Core/functors/NullaryFunctors.h +++ b/Eigen/src/Core/functors/NullaryFunctors.h @@ -18,10 +18,9 @@ template struct scalar_constant_op { EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE scalar_constant_op(const scalar_constant_op& other) : m_other(other.m_other) { } EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE scalar_constant_op(const Scalar& other) : m_other(other) { } - template - EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Scalar operator() (Index, Index = 0) const { return m_other; } - template - EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const PacketType packetOp(Index, Index = 0) const { return internal::pset1(m_other); } + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Scalar operator() () const { return m_other; } + template + EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const PacketType packetOp() const { return internal::pset1(m_other); } const Scalar m_other; }; template @@ -146,27 +145,9 @@ template struct linspa template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Scalar operator() (Index i) const { return impl(i); } - // We need this function when assigning e.g. a RowVectorXd to a MatrixXd since - // there row==0 and col is used for the actual iteration. - template - EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Scalar operator() (Index row, Index col) const - { - eigen_assert(col==0 || row==0); - return impl(col + row); - } - - template + template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Packet packetOp(Index i) const { return impl.packetOp(i); } - // We need this function when assigning e.g. a RowVectorXd to a MatrixXd since - // there row==0 and col is used for the actual iteration. - template - EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE const Packet packetOp(Index row, Index col) const - { - eigen_assert(col==0 || row==0); - return impl.packetOp(col + row); - } - // This proxy object handles the actual required temporaries, the different // implementations (random vs. sequential access) as well as the // correct piping to size 2/4 packet operations. @@ -175,11 +156,11 @@ template struct linspa const linspaced_op_impl::IsInteger?true:RandomAccess),NumTraits::IsInteger> impl; }; -// all functors allow linear access, except scalar_identity_op. So we fix here a quick meta -// to indicate whether a functor allows linear access, just always answering 'yes' except for -// scalar_identity_op. -template struct functor_has_linear_access { enum { ret = 1 }; }; -template struct functor_has_linear_access > { enum { ret = 0 }; }; +// Linear access is automatically determined from the operator() prototypes available for the given functor. +// If it exposes an operator()(i,j), then we assume the i and j coefficients are required independently +// and linear access is not possible. In all other cases, linear access is enabled. +// Users should not have to deal with this struture. +template struct functor_has_linear_access { enum { ret = !has_binary_operator::value }; }; } // end namespace internal diff --git a/Eigen/src/Core/util/Meta.h b/Eigen/src/Core/util/Meta.h index b9beb6eb7..e69c78db2 100755 --- a/Eigen/src/Core/util/Meta.h +++ b/Eigen/src/Core/util/Meta.h @@ -22,6 +22,16 @@ namespace Eigen { +typedef EIGEN_DEFAULT_DENSE_INDEX_TYPE DenseIndex; + +/** + * \brief The Index type as used for the API. + * \details To change this, \c \#define the preprocessor symbol \c EIGEN_DEFAULT_DENSE_INDEX_TYPE. + * \sa \blank \ref TopicPreprocessorDirectives, StorageIndex. + */ + +typedef EIGEN_DEFAULT_DENSE_INDEX_TYPE Index; + namespace internal { /** \internal @@ -371,6 +381,39 @@ struct has_ReturnType enum { value = sizeof(testFunctor(0)) == sizeof(yes) }; }; +template struct any_int {}; +template const T& return_ref(); + +struct meta_yes { char data[1]; }; +struct meta_no { char data[2]; }; + +template +struct has_nullary_operator +{ + template static meta_yes testFunctor(C const *,any_int< sizeof(return_ref()()) > * = 0); + static meta_no testFunctor(...); + + enum { value = sizeof(testFunctor(static_cast(0))) == sizeof(meta_yes) }; +}; + +template +struct has_unary_operator +{ + template static meta_yes testFunctor(C const *,any_int< sizeof(return_ref()(Index(0))) > * = 0); + static meta_no testFunctor(...); + + enum { value = sizeof(testFunctor(static_cast(0))) == sizeof(meta_yes) }; +}; + +template +struct has_binary_operator +{ + template static meta_yes testFunctor(C const *,any_int< sizeof(return_ref()(Index(0),Index(0))) > * = 0); + static meta_no testFunctor(...); + + enum { value = sizeof(testFunctor(static_cast(0))) == sizeof(meta_yes) }; +}; + /** \internal In short, it computes int(sqrt(\a Y)) with \a Y an integer. * Usage example: \code meta_sqrt<1023>::ret \endcode */ diff --git a/Eigen/src/Core/util/XprHelper.h b/Eigen/src/Core/util/XprHelper.h index a98ba6e86..7e43f4fea 100644 --- a/Eigen/src/Core/util/XprHelper.h +++ b/Eigen/src/Core/util/XprHelper.h @@ -24,16 +24,6 @@ namespace Eigen { -typedef EIGEN_DEFAULT_DENSE_INDEX_TYPE DenseIndex; - -/** - * \brief The Index type as used for the API. - * \details To change this, \c \#define the preprocessor symbol \c EIGEN_DEFAULT_DENSE_INDEX_TYPE. - * \sa \blank \ref TopicPreprocessorDirectives, StorageIndex. - */ - -typedef EIGEN_DEFAULT_DENSE_INDEX_TYPE Index; - namespace internal { template diff --git a/test/nullary.cpp b/test/nullary.cpp index cb87695ee..ad306c1e9 100644 --- a/test/nullary.cpp +++ b/test/nullary.cpp @@ -104,13 +104,29 @@ void testVectorType(const VectorType& base) template void testMatrixType(const MatrixType& m) { + using std::abs; const Index rows = m.rows(); const Index cols = m.cols(); + typedef typename MatrixType::Scalar Scalar; + typedef typename MatrixType::RealScalar RealScalar; + + Scalar s1; + do { + s1 = internal::random(); + } while(abs(s1)::IsInteger)); MatrixType A; A.setIdentity(rows, cols); VERIFY(equalsIdentity(A)); VERIFY(equalsIdentity(MatrixType::Identity(rows, cols))); + + + A = MatrixType::Constant(rows,cols,s1); + Index i = internal::random(0,rows-1); + Index j = internal::random(0,cols-1); + VERIFY_IS_APPROX( MatrixType::Constant(rows,cols,s1)(i,j), s1 ); + VERIFY_IS_APPROX( MatrixType::Constant(rows,cols,s1).coeff(i,j), s1 ); + VERIFY_IS_APPROX( A(i,j), s1 ); } void test_nullary() @@ -137,4 +153,22 @@ void test_nullary() // Assignment of a RowVectorXd to a MatrixXd (regression test for bug #79). VERIFY( (MatrixXd(RowVectorXd::LinSpaced(3, 0, 1)) - RowVector3d(0, 0.5, 1)).norm() < std::numeric_limits::epsilon() ); #endif + +#ifdef EIGEN_TEST_PART_10 + // check some internal logic + VERIFY(( internal::has_nullary_operator >::value )); + VERIFY(( !internal::has_unary_operator >::value )); + VERIFY(( !internal::has_binary_operator >::value )); + VERIFY(( internal::functor_has_linear_access >::ret )); + + VERIFY(( !internal::has_nullary_operator >::value )); + VERIFY(( !internal::has_unary_operator >::value )); + VERIFY(( internal::has_binary_operator >::value )); + VERIFY(( !internal::functor_has_linear_access >::ret )); + + VERIFY(( !internal::has_nullary_operator >::value )); + VERIFY(( internal::has_unary_operator >::value )); + VERIFY(( !internal::has_binary_operator >::value )); + VERIFY(( internal::functor_has_linear_access >::ret )); +#endif }