Bug 99730

Created attachment 124910 [details]
Possible patch using a fuzzy test for zero

I get the following error when running the test suite on libreoffice-5.1.2.2

/sc/qa/unit/ucalc_formula.cxx:6841:Test::testFuncMDETERM
equality assertion failed
- Expected: 0
- Actual  : 6.34413156928661e-17
- Calculation of MDETERM incorrect for singular integer matrix

ucalc_formula.cxx:6841:Assertion
Test name: Test::testFuncMDETERM
equality assertion failed
- Expected: 0
- Actual  : 6.34413156928661e-17
- Calculation of MDETERM incorrect for singular integer matrix

Failures !!!
Run: 224   Failure total: 1   Failures: 1   Errors: 0


The system is x86_64 Linux with a Intel(R) Core(TM) i5-5200U CPU @ 2.20GHz CPU
using GCC 4.8.5. The error is reproducable on my system.

Commits introducing this test are c43442dfef3f5b21bec40054b56f406efdc5a84f 8140309d636d4a870875f2dd75ed3dfff2c0fbaf  (*) 222b2f9370bfc05473249ab3f78e9adca67754bf  (**)

Related bug is #32834.

The first commit introduces the test using an equality test for zero, (*) fixes
this issue by using a fuzzy test for zero, while (**) removes this test for
64bit platforms.

Note that is is a very bad idea to check for equality on floating point values
and it should always be a fuzzy test (such as in (*)). This even applies if one
can guarantee that the double was converted from an integer value (as in the
test case).

I propose to revert 222b2f9370bfc05473249ab3f78e9adca67754bf (reintroducing a
error bound of 1e-4) or applying the patch below which introduces an error
tolerance of 1e-12. (And also states the error tolerance in the error message.)

Note:
Using the LU (or LUP) decomposition to calculate the determinant of a matrix is
sometimes numerically unstable and can be wrong by an order of magnitude.
Cf section 'Limitations' of https://www.mathworks.com/help/matlab/ref/det.html.

Patch (also attached):

diff -u a/sc/qa/unit/ucalc_formula.cxx b/sc/qa/unit/ucalc_formula.cxx
--- a/sc/qa/unit/ucalc_formula.cxx
+++ b/sc/qa/unit/ucalc_formula.cxx
@@ -6847,17 +6847,10 @@
         aFormulaBuffer[13] = static_cast<sal_Unicode>( '0' + nSize );
         m_pDoc->SetString(aPos, aFormulaBuffer.toString());

-        // On crappy 32-bit targets, presumably without extended precision on
-        // interim results or optimization not catching it, this test fails
-        // when comparing to 0.0, so have a narrow error margin. See also
-        // commit message of 8140309d636d4a870875f2dd75ed3dfff2c0fbaf
-#if SAL_TYPES_SIZEOFPOINTER == 4
-        CPPUNIT_ASSERT_DOUBLES_EQUAL_MESSAGE("Calculation of MDETERM incorrect for singular integer matrix",
+        // Do not compare with equality instead use an error margin.
+        // This applies to both 32bit and 64bit.
+        CPPUNIT_ASSERT_DOUBLES_EQUAL_MESSAGE("Calculation of MDETERM incorrect for singular integer matrix (error tolerance 1e-12)",
                 0.0, m_pDoc->GetValue(aPos), 1e-12);
-#else
-        CPPUNIT_ASSERT_EQUAL_MESSAGE("Calculation of MDETERM incorrect for singular integer matrix",
-                0.0, m_pDoc->GetValue(aPos));
-#endif
     }

     int aVals[] = {23, 31, 13, 12, 34, 64, 34, 31, 98, 32, 33, 63, 45, 54, 65, 76};