[Libreoffice-commits] core.git: Branch 'feature/fixes11' - 5 commits - sc/qa sc/source

Tor Lillqvist tml at collabora.com
Thu Oct 15 05:48:01 PDT 2015


 sc/qa/unit/data/xls/systematic.xls       |binary
 sc/qa/unit/opencl-test.cxx               |   94 +++++++++++++++++++++++++++++++
 sc/source/core/opencl/formulagroupcl.cxx |   63 +++++++++++---------
 3 files changed, 129 insertions(+), 28 deletions(-)

New commits:
commit 3369a86c41fb57df84de95ae523e3678c6be703c
Author: Tor Lillqvist <tml at collabora.com>
Date:   Thu Oct 15 12:59:53 2015 +0300

    tdf#94924: Add a more systematic OpenCL unit test
    
    Avoid the horrible convention of hard-coding in a C++ unit test code
    addresses of data in the spreadsheet document being tested. Instead,
    mark the expected (= as calculated by Excel) and calculated (by
    LibreOffice) formula results, rectangular blocks of data, so that the
    C++ code can easily find it, and then compare. This is much more
    flexible. No need to edit hardoded row and column numbers in the C++
    code when adding more test data.
    
    The systematic.xls file has documentation on how to maintain it.
    
    Change-Id: I4fb088fe21831dd3b3213d21916460a708aa0842

diff --git a/sc/qa/unit/data/xls/systematic.xls b/sc/qa/unit/data/xls/systematic.xls
new file mode 100755
index 0000000..b3427b5
Binary files /dev/null and b/sc/qa/unit/data/xls/systematic.xls differ
diff --git a/sc/qa/unit/opencl-test.cxx b/sc/qa/unit/opencl-test.cxx
index 17b0e11..7848627 100644
--- a/sc/qa/unit/opencl-test.cxx
+++ b/sc/qa/unit/opencl-test.cxx
@@ -66,6 +66,7 @@ public:
     virtual bool load( const OUString &rFilter, const OUString &rURL,
             const OUString &rUserData, SfxFilterFlags nFilterFlags,
             SotClipboardFormatId nClipboardID, unsigned int nFilterVersion) SAL_OVERRIDE;
+    void testSystematic();
     void testSharedFormulaXLS();
 #if 0
     void testSharedFormulaXLSGroundWater();
@@ -299,6 +300,7 @@ public:
     void testFinancialMDurationFormula1();
 
     CPPUNIT_TEST_SUITE(ScOpenCLTest);
+    CPPUNIT_TEST(testSystematic);
     CPPUNIT_TEST(testSharedFormulaXLS);
     CPPUNIT_TEST(testFinacialFormula);
     CPPUNIT_TEST(testStatisticalFormulaFisher);
@@ -702,6 +704,98 @@ void ScOpenCLTest::testSharedFormulaXLSGroundWater()
 }
 #endif
 
+void ScOpenCLTest::testSystematic()
+{
+    if(!initTestEnv("systematic.", XLS, false))
+        return;
+
+    ScDocument& rDoc = xDocSh->GetDocument();
+    rDoc.CalcAll();
+
+    int nAVertBegin(0), nAVertEnd(0), nBVertBegin(0), nBVertEnd(0);
+    int nAHorEnd(0), nBHorEnd(0);
+
+    int nRow, nCol;
+    for (nRow = 0; nRow < 1000; ++nRow)
+    {
+        if (rDoc.GetString(ScAddress(0, nRow, 0)) == "a")
+        {
+            nAVertBegin = nRow + 1;
+
+            for (nCol = 0; nCol < 1000; ++nCol)
+            {
+                if (rDoc.GetString(ScAddress(nCol, nRow, 0)) != "a")
+                {
+                    nAHorEnd = nCol;
+                    break;
+                }
+            }
+            break;
+        }
+    }
+    for (; nRow < 1000; ++nRow)
+    {
+        if (rDoc.GetString(ScAddress(0, nRow, 0)) != "a")
+        {
+            nAVertEnd = nRow;
+            break;
+        }
+    }
+
+    for (; nRow < 1000; ++nRow)
+    {
+        if (rDoc.GetString(ScAddress(0, nRow, 0)) == "b")
+        {
+            nBVertBegin = nRow + 1;
+
+            for (nCol = 0; nCol < 1000; ++nCol)
+            {
+                if (rDoc.GetString(ScAddress(nCol, nRow, 0)) != "b")
+                {
+                    nBHorEnd = nCol;
+                    break;
+                }
+            }
+            break;
+        }
+    }
+    for (; nRow < 1000; ++nRow)
+    {
+        if (rDoc.GetString(ScAddress(0, nRow, 0)) != "b")
+        {
+            nBVertEnd = nRow;
+            break;
+        }
+    }
+
+    assert(nAVertBegin != 0);
+    assert(nBVertBegin != 0);
+    assert(nAVertEnd > nAVertBegin + 100);
+    assert(nBVertEnd > nBVertBegin + 100);
+    assert((nAVertEnd-nAVertBegin) == (nBVertEnd-nBVertBegin));
+    assert(nAHorEnd > 10);
+    assert(nBHorEnd > 10);
+    assert(nAHorEnd == nBHorEnd);
+
+    for (SCROW i = nAVertBegin; i < nAVertEnd; ++i)
+    {
+        for (int j = 1; j < nAHorEnd; ++j)
+        {
+            double fLibre = rDoc.GetValue(ScAddress(j, i, 0));
+            double fExcel = rDoc.GetValue(ScAddress(j, nBVertBegin + (i - nAVertBegin), 0));
+
+            const OString sFailedMessage =
+                OString(static_cast<sal_Char>('A'+j)) +
+                OString::number(i+1) +
+                "!=" +
+                OString(static_cast<sal_Char>('A'+j)) +
+                OString::number(nBVertBegin+(i-nAVertBegin)+1);
+            CPPUNIT_ASSERT_DOUBLES_EQUAL_MESSAGE(sFailedMessage.getStr(), fExcel, fLibre, 1e-10);
+        }
+    }
+}
+
+
 void ScOpenCLTest::testSharedFormulaXLS()
 {
     if(!initTestEnv("sum_ex.", XLS, false))
commit 9b9192f7c12c763a4a51884d8405a5dbf44ecec7
Author: Tor Lillqvist <tml at collabora.com>
Date:   Thu Oct 15 12:37:55 2015 +0300

    tdf#94924: Return correct result 0 from OpenCL MIN and MAX when all args empty
    
    Used the same style as existing code, added a new virtual isMinOrMax()
    and add some special casing in Reduction::GenSlidingWindowFunction(),
    and fsim_count() and fmax_count() functions that count how many
    non-NaN numbers we actually see. As such, I am not sure at all that
    this is an ideal way to do this, but will have to do for now.
    
    Change-Id: I846a8d24f4563f8fae1a45971a4ce202ed918487

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index 104fd5b..fc77718 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -63,6 +63,18 @@ static const char* publicFunc =
  "    (*p) += t?0:1;\n"
  "    return t?b:a+b;\n"
  "}\n"
+ "double fmin_count(double a, double b, __private int *p) {\n"
+ "    double result = fmin(a, b);\n"
+ "    bool t = isnan(result);\n"
+ "    (*p) += t?0:1;\n"
+ "    return result;\n"
+ "}\n"
+ "double fmax_count(double a, double b, __private int *p) {\n"
+ "    double result = fmax(a, b);\n"
+ "    bool t = isnan(result);\n"
+ "    (*p) += t?0:1;\n"
+ "    return result;\n"
+ "}\n"
  "double fsum(double a, double b) { return isNan(a)?b:a+b; }\n"
  "double legalize(double a, double b) { return isNan(a)?b:a;}\n"
  "double fsub(double a, double b) { return a-b; }\n"
@@ -1697,7 +1709,7 @@ public:
         ss << ") {\n";
         ss << "double tmp = " << GetBottom() << ";\n";
         ss << "int gid0 = get_global_id(0);\n";
-        if (isAverage())
+        if (isAverage() || isMinOrMax())
             ss << "int nCount = 0;\n";
         ss << "double tmpBottom;\n";
         unsigned i = vSubArguments.size();
@@ -1777,12 +1789,17 @@ public:
             ss <<
                 "if (nCount==0)\n"
                 "    return CreateDoubleError(errDivisionByZero);\n";
+        else if (isMinOrMax())
+            ss <<
+                "if (nCount==0)\n"
+                "    return 0;\n";
         ss << "return tmp";
         if (isAverage())
             ss << "*pow((double)nCount,-1.0)";
         ss << ";\n}";
     }
     virtual bool isAverage() const { return false; }
+    virtual bool isMinOrMax() const { return false; }
     virtual bool takeString() const SAL_OVERRIDE { return false; }
     virtual bool takeNumeric() const SAL_OVERRIDE { return true; }
 };
@@ -2197,12 +2214,13 @@ class OpMin : public Reduction
 public:
     OpMin( int nResultSize ) : Reduction(nResultSize) {}
 
-    virtual std::string GetBottom() SAL_OVERRIDE { return "MAXFLOAT"; }
+    virtual std::string GetBottom() SAL_OVERRIDE { return "NAN"; }
     virtual std::string Gen2( const std::string& lhs, const std::string& rhs ) const SAL_OVERRIDE
     {
-        return "fmin(" + lhs + "," + rhs + ")";
+        return "fmin_count(" + lhs + "," + rhs + ", &nCount)";
     }
     virtual std::string BinFuncName() const SAL_OVERRIDE { return "min"; }
+    virtual bool isMinOrMax() const SAL_OVERRIDE { return true; }
 };
 
 class OpMax : public Reduction
@@ -2210,12 +2228,13 @@ class OpMax : public Reduction
 public:
     OpMax( int nResultSize ) : Reduction(nResultSize) {}
 
-    virtual std::string GetBottom() SAL_OVERRIDE { return "-MAXFLOAT"; }
+    virtual std::string GetBottom() SAL_OVERRIDE { return "NAN"; }
     virtual std::string Gen2( const std::string& lhs, const std::string& rhs ) const SAL_OVERRIDE
     {
-        return "fmax(" + lhs + "," + rhs + ")";
+        return "fmax_count(" + lhs + "," + rhs + ", &nCount)";
     }
     virtual std::string BinFuncName() const SAL_OVERRIDE { return "max"; }
+    virtual bool isMinOrMax() const SAL_OVERRIDE { return true; }
 };
 
 class OpSumProduct : public SumOfProduct
commit e22489cb47b73afef68ba56ad8295f2db2ff28ff
Author: Tor Lillqvist <tml at collabora.com>
Date:   Thu Oct 15 11:45:49 2015 +0300

    tdf#94924: Return correct #DIV/0! error from AVERAGE in the OpenCL case
    
    Change-Id: If7326fd1242d90ff92e62d141714960476198605

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index 14cb66f..104fd5b 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -1773,6 +1773,10 @@ public:
                 ss << ";\n";
             }
         }
+        if (isAverage())
+            ss <<
+                "if (nCount==0)\n"
+                "    return CreateDoubleError(errDivisionByZero);\n";
         ss << "return tmp";
         if (isAverage())
             ss << "*pow((double)nCount,-1.0)";
@@ -2121,7 +2125,7 @@ public:
         ss << "fsum_count(" << lhs << "," << rhs << ", &nCount)";
         return ss.str();
     }
-    virtual std::string BinFuncName() const SAL_OVERRIDE { return "fsum"; }
+    virtual std::string BinFuncName() const SAL_OVERRIDE { return "average"; }
     virtual bool isAverage() const SAL_OVERRIDE { return true; }
 };
 
commit d8911340cf5e68f23b5d1715b96b01c7af6fc458
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Oct 14 21:47:20 2015 +0300

    tdf#94924: Fix handling of empty cells in OpenCL division
    
    Not sure why the code from f5e7207053b857b6903a0ab9c161bed9ad7bcee9
    did not produce correct results any longer. Anyway, now OpenCL
    division works right in case of empty or zero cells.
    
    Clearly I need to add unit tests to make sure this stuff keeps
    working. In later commits.
    
    Change-Id: I93b787ad5da453af1601768308fb614a332ed142

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index 05dbff3..14cb66f 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -2171,20 +2171,16 @@ public:
         {
             ss <<
                 "if (isnan(" << vSubArguments[argno]->GenSlidingWindowDeclRef() << ")) {\n"
-                "    if (GetDoubleErrorValue(" << vSubArguments[argno]->GenSlidingWindowDeclRef() << ") == errNoValue)\n"
-                "        return CreateDoubleError(errDivisionByZero);\n"
+                "    return CreateDoubleError(errDivisionByZero);\n"
                 "}\n";
             return true;
         }
         else if (argno == 0)
         {
             ss <<
-                "if (isnan(" << vSubArguments[argno]->GenSlidingWindowDeclRef() << ")) {\n"
-                "    if (GetDoubleErrorValue(" << vSubArguments[argno]->GenSlidingWindowDeclRef() << ") == errNoValue) {\n"
-                "        if (" << vSubArguments[1]->GenSlidingWindowDeclRef() << " == 0)\n"
-                "            return CreateDoubleError(errDivisionByZero);\n"
-                "        return 0;\n"
-                "    }\n"
+                "if (isnan(" << vSubArguments[argno]->GenSlidingWindowDeclRef() << ") &&\n"
+                "    !(isnan(" << vSubArguments[1]->GenSlidingWindowDeclRef() << ") || " << vSubArguments[1]->GenSlidingWindowDeclRef() << " == 0)) {\n"
+                "    return 0;\n"
                 "}\n";
         }
         return false;
commit 4825be5aa09b7f67078cec9ae5b24203e49e724a
Author: Tor Lillqvist <tml at collabora.com>
Date:   Wed Oct 14 21:14:04 2015 +0300

    tdf#94924: Fix handling of empty cells in OpenCL subtraction
    
    We get correct result by simplifying the code;)
    
    No need to have the outer "if (gid0 < X)" test around the calculation
    code generated by Reduction::GenSlidingWindowFunction(). The lhs and
    rhs check the gid0 range themselves and that leads to the desired
    result for subtraction.
    
    While fixing this I noticed that the handling of empty cells in
    division is also wrong. Will fix in another commit.
    
    Change-Id: Ia45bd81e692a17b0453cc79cd4673a00e119562a

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index e413c8c..05dbff3 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -1733,13 +1733,8 @@ public:
                 assert(pCur);
                 assert(pCur->GetType() != formula::svDoubleVectorRef);
 
-                if (pCur->GetType() == formula::svSingleVectorRef)
-                {
-                    const formula::SingleVectorRefToken* pSVR =
-                        static_cast<const formula::SingleVectorRefToken*>(pCur);
-                    ss << "if (gid0 < " << pSVR->GetArrayLength() << "){\n";
-                }
-                else if (pCur->GetType() == formula::svDouble)
+                if (pCur->GetType() == formula::svSingleVectorRef ||
+                    pCur->GetType() == formula::svDouble)
                 {
                     ss << "{\n";
                 }
@@ -1770,13 +1765,6 @@ public:
                 ss << ";\n";
                 ss << "    }\n";
                 ss << "}\n";
-                if (vSubArguments[i]->GetFormulaToken()->GetType() ==
-                        formula::svSingleVectorRef && ZeroReturnZero())
-                {
-                    ss << "else{\n";
-                    ss << "        return 0;\n";
-                    ss << "    }\n";
-                }
             }
             else
             {


More information about the Libreoffice-commits mailing list