search

xtensor-stack / xtensor-blas

BLAS extension to xtensor
BSD 3-Clause "New" or "Revised" License
155 stars 54 forks source link

relax tolerance in the xlapack.solveCholesky test #212

Closed drew-parsons closed 2 years ago

drew-parsons commented 2 years ago

The xlapack.solveCholesky test in test_lapack.cpp fails on 32-bit architectures (i386/i686): [ RUN ] xlapack.solveCholesky /tmp/autopkgtest-lxc.gg3nslld/downtmp/autopkgtest_tmp/test_lapack.cpp:166: Failure Expected equality of these values: x_expected[i] Which is: 0.13757507429403265 x[i] Which is: 0.13757507429403248 [ FAILED ] xlapack.solveCholesky (0 ms)

This patch relaxes test tolerance by using EXPECT_NEAR with abstol=2e-16 instead of EXPECT_DOUBLE_EQ.

Fixes Issue #211.

drew-parsons commented 2 years ago

Don't merge this PR until we confirm it fixes the tests on 32-bit arches.

drew-parsons commented 2 years ago

Curiously, applying tolerance 2e-16 gives this warning

The abs_error parameter 2e-16 evaluates to 2e-16 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 2.2204460492503131e-16, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.

According to that, the i386 test should have been within tolerance in the first place (0.13757507429403265 - 0.13757507429403248 = 1.6653e-16). This patch is now set to 5e-16, we'll know soon if that's relaxed enough.

SylvainCorlay commented 2 years ago

This patch is now set to 5e-16, we'll know soon if that's relaxed enough.

Let me know if that's fine with the 32 bits testing you got in Debian!

drew-parsons commented 2 years ago

Looks like 5e-16 is enough. i386 and armhf are both passing now, https://ci.debian.net/data/autopkgtest/testing/i386/x/xtensor-blas/15983027/log.gz https://ci.debian.net/data/autopkgtest/testing/armhf/x/xtensor-blas/15982850/log.gz

Eventually test logs will appear at https://ci.debian.net/packages/x/xtensor-blas/

Thumbs up from me for the PR now.