Re: [eigen] unpleasant surprise mit math functions

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

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
>> ----------------------------------------------
>>
>>