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

Tor Lillqvist tml at collabora.com
Fri Oct 16 07:13:39 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 9db43e72088faa8326ca941b84d422726448293a
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
    Signed-off-by: Michael Meeks <michael.meeks at collabora.com>

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 127c268387e25a1f1c4b75b5991a83d383be7b92
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
    Signed-off-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index f49dda8..2667f5d 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -64,6 +64,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"
@@ -1698,7 +1710,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();
@@ -1778,12 +1790,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; }
 };
@@ -2198,12 +2215,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
@@ -2211,12 +2229,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 a6fa35ab0efc1d0f0d74e70898cc31be9d672190
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
    Signed-off-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index 79220e0..f49dda8 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -1774,6 +1774,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)";
@@ -2122,7 +2126,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 958ef1a5bff0b7d0879a4ae34a5e7f27eadd903b
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
    Signed-off-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index 255a546..79220e0 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -2172,20 +2172,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 b76c450eb7f6517fae24a0d7ea6431663959adf3
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
    Signed-off-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx
index e5324cd..255a546 100644
--- a/sc/source/core/opencl/formulagroupcl.cxx
+++ b/sc/source/core/opencl/formulagroupcl.cxx
@@ -1734,13 +1734,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";
                 }
@@ -1771,13 +1766,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