openturns icon indicating copy to clipboard operation
openturns copied to clipboard

[Debian] DistFunc_binomial issues in tests on arm64 and ppc64el

Open pgrt opened this issue 2 years ago • 1 comments

Hello,

During the Debian packaging of version 1.18, I noticed the following test failure on the architectures arm64 and ppc64el :

343/492 Test #343: cppcheck_DistFunc_binomial .......................................***Failed 0.15 sec --- /<<PKGBUILDDIR>>/lib/test/t_DistFunc_binomial.expout 2021-11-10 07:42:45.000000000 +0000 +++ /<<PKGBUILDDIR>>/build-python3.10/lib/test/t_DistFunc_binomial.out 2022-04-11 22:20:54.053398612 +0000 @@ -70,7 +70,7 @@ dBinomial(11, 0.1, 6)=0.000273 dBinomial(11, 0.1, 7)=2.17e-05 dBinomial(11, 0.1, 8)=1.2e-06 -dBinomial(11, 0.1, 9)=4.46e-08 +dBinomial(11, 0.1, 9)=4.45e-08 dBinomial(11, 0.1, 10)=9.9e-10 dBinomial(11, 0.1, 11)=1e-11 dBinomial(12, 0.1, 0)=0.282 @@ -193,11 +193,11 @@ dBinomial(5, 0.5, 4)=0.156 dBinomial(5, 0.5, 5)=0.0312 dBinomial(6, 0.5, 0)=0.0156 -dBinomial(6, 0.5, 1)=0.0938 +dBinomial(6, 0.5, 1)=0.0937 dBinomial(6, 0.5, 2)=0.234 dBinomial(6, 0.5, 3)=0.312 dBinomial(6, 0.5, 4)=0.234 -dBinomial(6, 0.5, 5)=0.0938 +dBinomial(6, 0.5, 5)=0.0937 dBinomial(6, 0.5, 6)=0.0156 dBinomial(7, 0.5, 0)=0.00781 dBinomial(7, 0.5, 1)=0.0547

I guess one only has to decrease the number of digits in the output to correct this issue. Maybe you would not like to do it, in which case I would make it a Debian-specific change.

Best,

Pierre

pgrt avatar May 14 '22 20:05 pgrt

it's ok to fix it in ot

jschueller avatar May 16 '22 08:05 jschueller

I found out that this is due to the floating-point expression contraction optimization. One can work-around it by setting "-ffp-contract=off" in CXXFLAGS. Not an incorrect result per say, but our reference is x86_64 where such optimizations are not available.

jschueller avatar Nov 22 '22 16:11 jschueller

The root cause is Table-maker's_dilemma, i.e. a "5" digit, followed by a long list of zeros. Hence, the result depends on the rounding of the last digit.

This failure reveals that the unit tests should not use a comparison to a reference text file, and use assertions instead. In Scilab, I used this method:

  • A CSV datafile which contains reference values with 17 significant digits : https://gitlab.com/scilab/forge/distfun/-/blob/master/tests/unit_tests/bino/binomiale.dataset.csv
  • A script based on assertions and well chosen relative tolerances : https://gitlab.com/scilab/forge/distfun/-/blob/master/tests/unit_tests/bino/binopdf.tst

mbaudin47 avatar Dec 05 '22 15:12 mbaudin47

I've tried to understand the problem with

-dBinomial(11, 0.1, 9)=4.46e-08
+dBinomial(11, 0.1, 9)=4.45e-08

-dBinomial(6, 0.5, 1)=0.0938
+dBinomial(6, 0.5, 1)=0.0937

-dBinomial(6, 0.5, 5)=0.0938
+dBinomial(6, 0.5, 5)=0.0937

So I ran the following R script:

options(digits=17)

print("dBinomial(11, 0.1, 9)=")
print(dbinom(x=9, size=11, prob=.1))

print("dBinomial(6, 0.5, 1)=")
print(dbinom(x=1, size=6, prob=.5))

print("dBinomial(6, 0.5, 5) = ")
print(dbinom(x=5, size=6, prob=.5))

Output:

[1] "dBinomial(11, 0.1, 9)="
[1] 4.4550000000000038e-08
[1] "dBinomial(6, 0.5, 1)="
[1] 0.093749999999999986
[1] "dBinomial(6, 0.5, 5) = "
[1] 0.093750000000000028

josephmure avatar Dec 05 '22 15:12 josephmure

we can do that, it's more involving than a self contained script because files have to be copied by cmake though

jschueller avatar Dec 06 '22 08:12 jschueller