Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Approx 0 comparison fails #1079

Closed
doubleday opened this issue Nov 4, 2017 · 6 comments
Closed

Approx 0 comparison fails #1079

doubleday opened this issue Nov 4, 2017 · 6 comments

Comments

@doubleday
Copy link

Description

 CHECK(0.0f == Approx(0.0f + std::numeric_limits<float>::epsilon()));

fails with

.... mytest.cpp:78: FAILED:
  CHECK(0.0f == Approx(0.0f + std::numeric_limits<float>::epsilon()) )
with expansion:
  0.0f == Approx( 0.0000001192 )

I believe this is due to changing the default of margin to 0.

Is this the intended behaviour?

Changing the margin to default to epsilon fixes this for me:

    Approx::Approx ( double value )
    :   m_epsilon( std::numeric_limits<float>::epsilon()*100 ),
        m_margin( m_epsilon ),
        m_scale( 0.0 ),
        m_value( value )
    {}
@horenmar
Copy link
Member

horenmar commented Nov 4, 2017

Approx::margin has always been set to 0. What has changed is Approx::scale.

Anyway, yes the new behaviour is intended. By relative margin the machine epsilon (std::numeric_limits<float>::epsilon()) is quite far from zero.

@doubleday
Copy link
Author

doubleday commented Nov 4, 2017

Ah yes sorry. Of course epsilon is pretty far from 0...

But still (just because as of a sudden my tests fail..).

Is this also expected behaviour?

NumericsTest.cpp:78: FAILED:
  CHECK( 0.0f == Approx(std::nextafter(0.0f, 1.0f)) )
with expansion:
  0.0f == Approx( 0.0 )

@doubleday
Copy link
Author

doubleday commented Nov 4, 2017

I just did a test with google's version for float comparisons...

which as a test looks (and works) like this:

CHECK(Float(0.0f).AlmostEquals(Float(std::nextafter(0.0f, 1.0f))));

For this test I extracted the comparison functionality only:
https://gist.github.com/doubleday/08490faeb2885ac35dcd6170712de07f

The IEEE bit magic is way beyond me but maybe it could be used as inspiration ....

@horenmar
Copy link
Member

horenmar commented Nov 4, 2017

Fundamentally there are 3 ways of comparing floating point numbers

  1. Check for an absolute difference. This is the naive approach of if (fabs(a - b) <= margin), and has a rather obvious problem in that the same margin doesn't work well for small and large numbers.
  2. Check for relative difference. This is essentially the same approach as above, except that you scale margin by size of inputs to the function, ie if (fabs(a - b) <= epsilon * max(fabs(a), fabs(b)). This approach is fairly intuitive, as long as you do not run into sufficiently small (ie denormalized) numbers.
  3. Check for ULP (Units in Last Place) difference. 1 ULP difference means that there is no representable number between the two compared numbers. 2 ULP difference means that there can be at most 1 representable number between the two compared. This model has the advantage of autoscaling up and down, but is in no way intuitive.

Approx provides for 1) and 2)*, because these are simpler to work with. We are planning to provide a Matcher for 3)(#1074), and also for 1) because ULP's do not quite work around 0 and denormals either.

* Note that we scale by Approx's value, and not by the larger of the two compared values.

@doubleday
Copy link
Author

Thanks for the details.

I'm testing SIMD (SSE,NEON) code against a scalar CPU version (the truth). It's in the audio domain and values are in [-1,1] and often in the problematic 0 range.

I'll probably either go with some version of ULP diff or settle on some sensible margin for the tests.

@tjwrona
Copy link

tjwrona commented Nov 18, 2020

@doubleday I found a pretty decent workaround if all of your values are on the scale of ~1. I created a wrapper function around Approx that sets the scale of Approx to 1. This will make it work for 0 and as far as I know it shouldn't be a problem as long as all of the numbers you are approximating are not absurdly large or small.

#include <catch2/catch_approx.hpp>

Catch::Approx approx(const double value)
{
   return Catch::Approx{ value }.scale(1);
}

You can then call this function as follows:

CHECK(0.0f == approx(0));

And it should work :)

KaneTW pushed a commit to SimulaVR/monado that referenced this issue Sep 20, 2022
To avoid an issue with catch2::Approx defaults applied around 0
See catchorg/Catch2#1079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants