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 2eac5c406f10f10c943d0c128dfe6a5db3972e1a Author: Tor Lillqvist <[email protected]> Date: Wed Oct 14 21:14:04 2015 +0300 tdf#94924: Fix several OpenCL problems Squash several fixes and a unit test into one: + 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. + 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. + Return correct #DIV/0! error from AVERAGE in the OpenCL case + 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. + 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: Ib6e613251bd4a4c96525869611781624cf472ad2 Signed-off-by: Michael Meeks <[email protected]> Reviewed-on: https://gerrit.libreoffice.org/19413 Reviewed-by: Tor Lillqvist <[email protected]> Tested-by: Tor Lillqvist <[email protected]> Reviewed-by: Eike Rathke <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> Tested-by: Caolán McNamara <[email protected]> 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)) diff --git a/sc/source/core/opencl/formulagroupcl.cxx b/sc/source/core/opencl/formulagroupcl.cxx index e5324cd..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(); @@ -1734,13 +1746,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 +1778,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 { @@ -1786,12 +1786,21 @@ public: ss << ";\n"; } } + if (isAverage()) + 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; } }; @@ -2134,7 +2143,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; } }; @@ -2184,20 +2193,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; @@ -2210,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 @@ -2223,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
_______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
