Re: [eigen] unpleasant surprise mit math functions

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

The patch is pushed:

https://bitbucket.org/eigen/eigen/changeset/ee70f8f54114/
changeset:   ee70f8f54114
user:        ggael
date:        2012-11-06 15:25:50
summary:     Fix bug 314:
- remove most of the metaprogramming kung fu in MathFunctions.h (only keep
functions that differs from the std)
- remove the overloads for array expression that were in the std namespace
affected #:  88 files


Still have to deal with conj, real, and imag which differ from the std
variants (they are only defined for complex).


cheers,
gael


On Tue, Oct 9, 2012 at 2:03 PM, Gael Guennebaud
<gael.guennebaud@xxxxxxxxx> wrote:
> Hi,
>
> I started to write a fix that removes all this meta-programming kung
> fu. You can see it there (it is not finalized yet, that's just a
> preview):
>
> http://eigen.tuxfamily.org/bz/show_bug.cgi?id=314
>
> This patch also includes two related changes:
>
> 1 - the global math functions for ArrayBase<> were incorrectly defined
> on the Eigen::internal namespace, while the intention was to have them
> in the Eigen namespace (since ArrayBase<> has a base class in the
> internal namespace this worked as expected though).
>
> 2 - it also removes the overloads of the standard math functions
> defined in the std namespace for Eigen::ArrayBase<> objects: indeed,
> they leads to ambiguous calls when someone do, for instance:
>
> template<typename T> void foo(const T& t)
> {
>   using std::sin;
>   sin(t);
> }
>
> and T is an ArrayBase<> object.
>
> cheers,
> gael
>
> On Thu, Sep 27, 2012 at 11:26 PM, Gael Guennebaud
> <gael.guennebaud@xxxxxxxxx> wrote:
>> On Thu, Sep 27, 2012 at 12:01 PM, Christoph Hertzberg
>> <chtz@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>>> On 27.09.2012 11:28, Hauke Heibel wrote:
>>>>
>>>> In summary, I would be glad if it were possible to have the option to
>>>> call sin/cos and co on Arrays exactly as I can do it in Matlab.
>>>
>>>
>>> I don't think anybody (at least not me) argued for removing
>>> sin(ArrayBase<...>);
>>> There are currently two places (see below) where Eigen defines
>>> Eigen::internal::sin and one where it defines std::sin. I would suggest only
>>> removing the internal::sin of MathFunctions.h whose purpose only seems to be
>>> to prevent calling sin(int), however it leads to infinite recursion for
>>> Matrix-expressions (and maybe actually someone likes to abuse cos(/* int */
>>> x) for (x==0) *g*)
>>
>> yes, the plan was only to cleanup the MathFunctions.h file. For the
>> record, here are two related bug reports:
>>
>> http://eigen.tuxfamily.org/bz/show_bug.cgi?id=314
>> http://eigen.tuxfamily.org/bz/show_bug.cgi?id=508
>>
>> gael
>>
>>> It might be even wise to define the other Eigen::internal::sin as
>>> Eigen::sin, that would make both Eigen::sin(array) and sin(array) work.
>>> I'm not very comfortable with allowing std::sin(array), but I'm ok with that
>>> for not breaking the API.
>>> In most applications you would be fine with
>>> {using std::sin; sin(x); /* x can be scalar or Eigen::ArrayBase<> */ }
>>>
>>>
>>> GlobalFunction.h:
>>>>
>>>> namespace std {
>>>> // Line 42:
>>>> template<typename Derived> \
>>>>   inline const Eigen::CwiseUnaryOp<Eigen::internal::scalar_sin_op<typename
>>>> Derived::Scalar>, const Derived> \
>>>>   sin(const Eigen::ArrayBase<Derived>& x) { \
>>>>     return x.derived(); \
>>>>   }
>>>> // EOL 42
>>>> }
>>>>
>>>>
>>>> namespace Eigen { namespace internal {
>>>> // Line 88:
>>>> template<typename Derived> \
>>>>   struct sin_retval<ArrayBase<Derived> > \
>>>>   { \
>>>>     typedef const
>>>> Eigen::CwiseUnaryOp<Eigen::internal::scalar_sin_op<typename
>>>> Derived::Scalar>, const Derived> type; \
>>>>   }; \
>>>>   template<typename Derived> \
>>>>   struct sin_impl<ArrayBase<Derived> > \
>>>>   { \
>>>>     static inline typename sin_retval<ArrayBase<Derived> >::type run(const
>>>> Eigen::ArrayBase<Derived>& x) \
>>>>     { \
>>>>       return x.derived(); \
>>>>     } \
>>>>   };
>>>> // EOL 88
>>>> }}
>>>
>>>
>>> MathFunctions.h  ONLY REMOVE THESE!
>>>>
>>>> namespace Eigen { namespace internal {
>>>> // Line 476:
>>>> template<typename Scalar, bool IsInteger> struct sin_default_impl {
>>>> \
>>>>     static inline Scalar run(const Scalar& x) { using std::sin; return
>>>> sin(x); }  \
>>>>   };
>>>> \
>>>>   template<typename Scalar> struct sin_default_impl<Scalar, true> {
>>>> \
>>>>     static inline Scalar run(const Scalar&) {
>>>> \
>>>>       if
>>>> (Eigen::internal::static_assertion<bool(!NumTraits<Scalar>::IsInteger)>::THIS_FUNCTION_IS_NOT_FOR_INTEGER_NUMERIC_TYPES)
>>>> {}                                       \
>>>>       return Scalar(0);
>>>> \
>>>>     }
>>>> \
>>>>   };
>>>> \
>>>>   template<typename Scalar> struct sin_impl
>>>> \
>>>>     : sin_default_impl<Scalar, NumTraits<Scalar>::IsInteger>
>>>> \
>>>>   {};
>>>> \
>>>>   template<typename Scalar> struct sin_retval { typedef Scalar type; };
>>>> \
>>>>   template<typename Scalar>
>>>> \
>>>>   inline typename sin_retval<typename
>>>> global_math_functions_filtering_base<Scalar>::type>::type sin(const Scalar&
>>>> x) {                \
>>>>     return sin_impl<typename
>>>> global_math_functions_filtering_base<Scalar>::type>::run(x);
>>>> \
>>>>   }
>>>> // EOL 476
>>>> }}
>>>
>>>
>>>
>>>
>>> Christoph
>>>
>>> --
>>> ----------------------------------------------
>>> Dipl.-Inf. Christoph Hertzberg
>>> Cartesium 0.049
>>> Universität Bremen
>>> Enrique-Schmidt-Straße 5
>>> 28359 Bremen
>>>
>>> Tel: +49 (421) 218-64252
>>> ----------------------------------------------
>>>
>>>