[Libreoffice-commits] core.git: Branch 'libreoffice-5-0-3' - sc/qa sc/source
Tor Lillqvist
tml at collabora.com
Thu Oct 22 08:51:35 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 2eac5c406f10f10c943d0c128dfe6a5db3972e1a
Author: Tor Lillqvist <tml at collabora.com>
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 <michael.meeks at collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/19413
Reviewed-by: Tor Lillqvist <tml at collabora.com>
Tested-by: Tor Lillqvist <tml at collabora.com>
Reviewed-by: Eike Rathke <erack at redhat.com>
Reviewed-by: Caolán McNamara <caolanm at redhat.com>
Tested-by: Caolán McNamara <caolanm at redhat.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))
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
More information about the Libreoffice-commits
mailing list