math icon indicating copy to clipboard operation
math copied to clipboard

Floating point precision loss under arm64 (FP_CONTRACT)

Open andrjohns opened this issue 2 years ago • 2 comments

Description

The inv_Phi prim tests fail under arm64 (both gcc and clang):

root@19459e2892a7:/math# uname -a
Linux 19459e2892a7 5.15.49-linuxkit-pr #1 SMP PREEMPT Thu May 25 07:27:39 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux

test/unit/math/prim/fun/inv_Phi_test --gtest_output="xml:test/unit/math/prim/fun/inv_Phi_test.xml"
Running main() from lib/benchmark_1.5.1/googletest/googletest/src/gtest_main.cc
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from MathFunctions
[ RUN      ] MathFunctions.inv_Phi
[       OK ] MathFunctions.inv_Phi (0 ms)
[ RUN      ] MathFunctions.Equal
test/unit/math/prim/fun/inv_Phi_test.cpp:54: Failure
The difference between exact[i] and inv_Phi(p[i]) is 1.7763568394002505e-15, which exceeds 1.5e-15, where
exact[i] evaluates to -5.9978070150076865,
inv_Phi(p[i]) evaluates to -5.9978070150076848, and
1.5e-15 evaluates to 1.4999999999999999e-15.
test/unit/math/prim/fun/inv_Phi_test.cpp:54: Failure
The difference between exact[i] and inv_Phi(p[i]) is 1.7763568394002505e-15, which exceeds 1.5e-15, where
exact[i] evaluates to -5.1993375821928165,
inv_Phi(p[i]) evaluates to -5.1993375821928183, and
1.5e-15 evaluates to 1.4999999999999999e-15.
test/unit/math/prim/fun/inv_Phi_test.cpp:54: Failure
The difference between exact[i] and inv_Phi(p[i]) is 1.0169642905566434e-12, which exceeds 1.5e-15, where
exact[i] evaluates to 4.2648907939228247,
inv_Phi(p[i]) evaluates to 4.2648907939238416, and
1.5e-15 evaluates to 1.4999999999999999e-15.
test/unit/math/prim/fun/inv_Phi_test.cpp:54: Failure
The difference between exact[i] and inv_Phi(p[i]) is 9.7845287427844596e-11, which exceeds 1.5e-15, where
exact[i] evaluates to 5.1993375821928165,
inv_Phi(p[i]) evaluates to 5.1993375822906618, and
1.5e-15 evaluates to 1.4999999999999999e-15.
test/unit/math/prim/fun/inv_Phi_test.cpp:54: Failure
The difference between exact[i] and inv_Phi(p[i]) is 4.5939518855675487e-09, which exceeds 1.5e-15, where
exact[i] evaluates to 5.9978070150076865,
inv_Phi(p[i]) evaluates to 5.9978070196016384, and
1.5e-15 evaluates to 1.4999999999999999e-15.
[  FAILED  ] MathFunctions.Equal (0 ms)
[ RUN      ] MathFunctions.inv_Phi_inf
[       OK ] MathFunctions.inv_Phi_inf (0 ms)
[ RUN      ] MathFunctions.inv_Phi_nan
[       OK ] MathFunctions.inv_Phi_nan (0 ms)
[----------] 4 tests from MathFunctions (0 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 3 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MathFunctions.Equal

 1 FAILED TEST
test/unit/math/prim/fun/inv_Phi_test --gtest_output="xml:test/unit/math/prim/fun/inv_Phi_test.xml" failed
exit now (08/28/23 20:00:12 UTC)

This appears to be due to the compiler contracting floating-point operators, since the inv_Phi function involves an approximation with several multiply-adds.

If I update my make/local to include:

CXXFLAGS += -ffp-contract=off

Then all passes successfully:

Running main() from lib/benchmark_1.5.1/googletest/googletest/src/gtest_main.cc
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from MathFunctions
[ RUN      ] MathFunctions.inv_Phi
[       OK ] MathFunctions.inv_Phi (0 ms)
[ RUN      ] MathFunctions.Equal
[       OK ] MathFunctions.Equal (0 ms)
[ RUN      ] MathFunctions.inv_Phi_inf
[       OK ] MathFunctions.inv_Phi_inf (0 ms)
[ RUN      ] MathFunctions.inv_Phi_nan
[       OK ] MathFunctions.inv_Phi_nan (0 ms)
[----------] 4 tests from MathFunctions (0 ms total)

[----------] Global test environment tear-down
[==========] 4 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 4 tests.

Should we add -ffp-contract=off as a standard flag for all systems? Or just for arm64?

Current Version:

v4.6.2

andrjohns avatar Aug 28 '23 20:08 andrjohns

I think for arm it makes sense but idt we need it for other systems

SteveBronder avatar Aug 28 '23 21:08 SteveBronder

I think for arm it makes sense but idt we need it for other systems

Sounds good to me! Will open a PR in a tick

andrjohns avatar Aug 28 '23 22:08 andrjohns