New user self-registration is disabled due to spam. Please email eigen-core-team @ lists.tuxfamily.org if you need an account.
Before reporting a bug, please make sure that your Eigen version is up-to-date!
Bug 619 - On Win CE 7, std::abs uses abs instead of fabs
Summary: On Win CE 7, std::abs uses abs instead of fabs
Status: RESOLVED FIXED
Alias: None
Product: Eigen
Classification: Unclassified
Component: General (show other bugs)
Version: unspecified
Hardware: Other Windows
: Normal Compilation Problem
Assignee: Nobody
URL:
Whiteboard:
Keywords:
: 723 943 (view as bug list)
Depends on:
Blocks: 3.3
  Show dependency treegraph
 
Reported: 2013-06-21 15:14 UTC by Christian Schluchter
Modified: 2015-01-30 18:36 UTC (History)
6 users (show)



Attachments

Description Christian Schluchter 2013-06-21 15:14:18 UTC
On WinEC7, std::abs is mapped to abs instead of fabs and returns an integer instead of a floating point number. So any floating point number will be reduced to the next lower int.

std::abs is used e.g. in MathFunctions.h, where the error can be reproduced e.g. by runnning array_1 test:

template<typename Scalar>
struct abs_impl
{
  typedef typename NumTraits<Scalar>::Real RealScalar;
  static inline RealScalar run(const Scalar& x)
  {
    using std::abs;
    RealScalar test = abs(x); //x is some float e.g. 0.12345, test is then 0
    return abs(x);
  }
};
Comment 1 Gael Guennebaud 2013-06-21 21:56:33 UTC
What if you explicitly call std::abs:

RealScalar test = std::abs(x);

If that works then it's a bug of the compiler, otherwise it's a bug of the associated STL.
Comment 2 Christian Schluchter 2013-06-24 13:50:57 UTC
Calling std::abs(x) still results in an integer, so the problem is definitely on the side of STL, namely std::abs is only implemented for int on WinCE. I'm not sure whether this can be seen as a bug or just a deviating implementation on WinCE.

Would it be possible to add implementations for abs(double) etc. in MathFunctions.h to become independent of the obviously insufficient STL implementations? 

Something along the lines of the attached template specializations would probably do the job. I agree it's rather ugly to define platform specific functions, but I don't see a solution to support WinCE with using std::abs.

template<typename Scalar>
inline EIGEN_MATHFUNC_RETVAL(abs, Scalar) abs(const Scalar& x)
{
  return EIGEN_MATHFUNC_IMPL(abs, Scalar)::run(x);
}

#ifdef _WIN32_WCE
template<>
inline double abs<double>(const double& x)
{
  return fabs(x);
}

template<>
inline float abs<float>(const float& x)
{
  return fabsf(x);
}

template<>
inline int abs<int>(const int& x)
{
  return abs(x);
}
#endif
Comment 3 Christoph Hertzberg 2013-06-24 14:27:54 UTC
(In reply to comment #2)
> Calling std::abs(x) still results in an integer, so the problem is definitely
> on the side of STL, namely std::abs is only implemented for int on WinCE. I'm
> not sure whether this can be seen as a bug or just a deviating implementation
> on WinCE.

One last hope: Did you include <cmath>? If that fixes it, we might have just forgotten to include that somewhere.

> Would it be possible to add implementations for abs(double) etc. in
> MathFunctions.h to become independent of the obviously insufficient STL
> implementations? 

Generally, that would be possible. The question is to what extent we are responsible for fixing broken libraries ...
What you can do locally is to make specializations of std::abs yourself (similar to what you suggested below) and include that before including any Eigen-headers. (Specializing std-functions in contrast to overloading is usually not considered bad practice)

Is it possible to file a bug-report at your libraries support site?

> template<>
> inline int abs<int>(const int& x)
> {
>   return abs(x);
> }

Wouldn't that result in infinite recursion? Or is there a reason why the non-template abs will be called instead?
Comment 4 Gael Guennebaud 2013-06-24 14:41:08 UTC
hm, this is very strange. According to the c++ standard, there must be overloads for float, double and long double and I cannot find any information about this possible issue with wince. Are you sure you don't have weird defines that could explain this?
Comment 5 Christian Schluchter 2013-06-24 15:12:51 UTC
Sorry to destroy this last hope, but cmath is included.

You are right about not being responsible about broken libraries. In this case, however, I'm not sure whether it can be called broken or not. According to the answer of http://stackoverflow.com/questions/13460750/on-the-stdabs-function, the implementation of std::abs(double) etc. is guaranteed as of C++11 to be present, but not earlier and probably WinCE (in VisualStudio 2008) just complies to an older standard. Unfortunately, I wasn't able to find a document that confirms the standard for WinCE in the MSDN mess. So I guess if you want to support WinCE, it would probably still be Eigen's responsibilty.

You are right about the infinite recursion, thanks, I didn't notice that.
Comment 6 Christoph Hertzberg 2013-06-24 15:41:17 UTC
That quoted SO answer does not say anything explicit about C++98.
I have not read the actual standard, but e.g.
http://www.cplusplus.com/reference/cmath/abs/
says <cmath> must have specializations for float, double and long double.
And for C++11 it must additionally have specializations for arbitrary integral types (which was the actual question on SO).

However, we do claim to support MSVC 2008 and newer. And as we did not say that this excludes WinCE, we should either make these compatibility hacks or warn that there are problems and provide work-arounds. And I'm somewhat afraid that more problems will arise ...
Comment 7 Christian Schluchter 2013-06-26 14:15:45 UTC
@Gael:
I'm very sure, but how could I guarantee.

@Christoph:
You're right about the quote, I misread the sentence about double, but I couldn't find something else that would tell whether double should be supported or not.

Anyway, I can only back Christoph's conclusion as MS certainly won't provide a new implementation and thank you guys for your help so far.
Comment 8 Christoph Hertzberg 2014-09-07 17:04:22 UTC
*** Bug 723 has been marked as a duplicate of this bug. ***
Comment 9 Christoph Hertzberg 2014-09-07 17:07:53 UTC
The general question is to what extent we want to support incomplete C++-libraries. And then of course, how we can detect this.
Comment 10 Gael Guennebaud 2014-09-07 23:03:47 UTC
Since we have two reports on that issue, let's workaround it:

https://bitbucket.org/eigen/eigen/commits/3212115e8ab5/
Changeset:   3212115e8ab5
User:        ggael
Date:        2014-09-07 23:02:30
Summary:     Bug 619: workaround MSVC 2008 implementing std::abs for int only on WINCE

Please confirm that's doing the job.
Comment 11 Gael Guennebaud 2015-01-30 18:36:05 UTC
*** Bug 943 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.