Ableton / link

Ableton Link
Other
1.09k stars 149 forks source link

Fix floating point test failure #134

Closed hack3ric closed 1 year ago

hack3ric commented 1 year ago

See https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1784.

Despite using Approx in floating-point, the tests fail on riscv64 because Approx uses relative margin comparison, and thus 0.0 == Approx(n) will always fail for n != 0.0. Use new floating-point matchers for those comparing with zero (or close to zero).

float comparisons uses a larger margin than double ones.

The failure log is shown below:

==> Starting check()...

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
LinkCoreTest is a Catch v2.13.9 host application.
Run with -? for options

-------------------------------------------------------------------------------
LinearRegression
  TwoPoints
-------------------------------------------------------------------------------
/usr/src/debug/abletonlink/link-Link-3.0.6/src/ableton/link/tst_LinearRegression.cpp:45
...............................................................................

/usr/src/debug/abletonlink/link-Link-3.0.6/src/ableton/link/tst_LinearRegression.cpp:53: FAILED:
  CHECK( 0.0 == Approx(result.second) )
with expansion:
  0.0 == Approx( -0.0 )

-------------------------------------------------------------------------------
LinearRegression
  TwoPoints Float
-------------------------------------------------------------------------------
/usr/src/debug/abletonlink/link-Link-3.0.6/src/ableton/link/tst_LinearRegression.cpp:72
...............................................................................

/usr/src/debug/abletonlink/link-Link-3.0.6/src/ableton/link/tst_LinearRegression.cpp:80: FAILED:
  CHECK( 0.f == Approx(result.second) )
with expansion:
  0.0f == Approx( -0.0011631348 )

===============================================================================
test cases:     14 |     13 passed | 1 failed
assertions: 524519 | 524517 passed | 2 failed
fgo-ableton commented 1 year ago

Thanks a lot for the PR! Unfortunately CI fails. Do you want to fix that. Then I could merge. Otherwise I would fix it and update to Catch::Matchers::WithinAbs.

hack3ric commented 1 year ago

It should pass CI now.

fgo-ableton commented 1 year ago

Thanks!