Negative zero does not pass compared to positive zero

[gregersn]

Describe the bug
If a variable holds the floating point value of -0.0f / -0x1.777a5cp-24, requiring it to be equal to 0.0f does not pass.

Expected behavior
That -0.0f == 0.0f

Reproduction steps
Steps to reproduce the bug.
float t = -0x1.777a5cp-24; REQUIRE(t == 0.0f);

Platform information:

• OS:Ubuntu 19.10
• Compiler+version: gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008
• Catch version: 2.9.0

Additional context
Doing REQUIRE(abs(t) == 0.0f) will pass.

[fodinabor]

I don't consider this a bug.. This is intended C++ behaviour.. Floating points should therefore never be compared with ==, but with std::abs(a-b)<threshold .
This is supported in Catch2 with REQUIRE(t == Approx(0.0f))

Have a look at the docs: https://github.com/catchorg/Catch2/blob/master/docs/assertions.md#floating-point-comparisons

[gregersn]

That is what I tried first, but gave the same result, I filed this bug after talking to Xarn on Discord.
Anyways

../tests/tests-vector.cpp:136: FAILED:
  REQUIRE( v.y == Approx(0.0f) )
with expansion:
  -0.0f == Approx( 0.0 )

[fodinabor]

OK, then this is the real issue, that the Approx doesn't catch it, but the plain == is thought to fail in this case..

[horenmar]

The suspicious part of this is that it passes after the negative value is passed through abs. Otherwise, if it was just a case of small value not being equal (nor Approx equal) to 0, then that would be the expected behaviour.

[fodinabor]

ehm.. that actually makes sense to me..

1. abs is only defined on ints -> the very small value is truncated to an actual 0 and therefore is equal to 0.0f

2. after some more digging (https://godbolt.org/z/7pPsbB), I guess the issue is

   Catch2/include/internal/catch_approx.cpp

   Line 56 in 02ee130

   || marginComparison(m_value, other, m_epsilon * (m_scale + std::fabs(std::isinf(m_value)? 0 : m_value)));

   that multiplying by m_value (which is 0.0) makes the comparison margin 0 again, hence Approx doesn't do anything useful..

why not multiply by 1.0, instead of by m_value?

[horenmar]

abs is only defined on ints

🤦‍♂ I keep forgetting this, because std::abs does have an overload for floating point numbers.

@gregersn Does it still pass if you instead write std::abs(x) == Approx(0)?

As to the second, because if it multiplied by 1. the allowed margin would not scale with value, turning m_epsilon into slightly complicated m_margin. The first comparison is absolute difference, the second is relative -- and for 0, any relative difference remains 0.

[gregersn]

../tests/tests-vector.cpp:136: FAILED:
  REQUIRE( std::abs(v.y) == Approx(0.0f) )
with expansion:
  0.0f == Approx( 0.0 )

[horenmar]

Yeah in that case the answer is "not actually a bug, sorry" 🤷‍♂

The lhs is some small value, that is however still non-zero. You need to set some "close-enough" value for your Approx instance via margin, because relative comparisons do nothing when you start at 0.

[dnmiller]

While the developers are free to argue this isn't a bug, there does seem to be a need for clarification in the docs. If try the test case

REQUIRE( std::abs(sin(2.0 * std::numbers::pi)) == Approx(0.0));

then, like the original reporter, I get a failure with

[ctest]   REQUIRE( std::abs(sin(2.0 * std::numbers::pi)) == Approx(0.0) )
[ctest] with expansion:
[ctest]   0.0 == Approx( 0.0 )

but if I use what the docs say are the default arguments and code up the test

REQUIRE(std::abs(sin(2.0 * std::numbers::pi)) == Approx(0.0).margin(std::numeric_limits<float>::epsilon()*100));

then the test passes. If there isn't a bug here then I think there is at least a documentation issue. It seems like explicit vs. implicit margins give different results.

[horenmar]

@dnmiller

from documentation.

margin - margin serves to set the the absolute value by which a result can differ from Approx's value before it is rejected. By default set to 0.0.