[Libreoffice-commits] core.git: Branch 'libreoffice-4-4' - officecfg/registry sc/source
Tor Lillqvist
tml at collabora.com
Mon Mar 9 05:00:53 PDT 2015
officecfg/registry/schema/org/openoffice/Office/Calc.xcs | 4 ++--
sc/source/core/data/grouptokenconverter.cxx | 12 ++++++++++++
sc/source/core/opencl/op_statistical.cxx | 15 +++++++++------
sc/source/core/tool/calcconfig.cxx | 6 ++++++
4 files changed, 29 insertions(+), 8 deletions(-)
New commits:
commit e3b511cc9968a14f08e3661f1f2de9ce83cc2d36
Author: Tor Lillqvist <tml at collabora.com>
Date: Thu Mar 5 18:03:27 2015 +0200
Fix bugs in the OpenCL implementation of some statistical functions
Don't return negative values from ScGroupTokenConverter::trimLength(). It
doesn't make sense, we should return zero instead. At the two call sites,
there are tests against a zero having been returned, but not against a
negative value. And the return value is even passed as the nArrayLength
parameter to a formula::DoubleVectorRefToken constructor, which is of type
size_t, thus unsigned. Passing for instance -4 to it ends up being interpreted
as 18446744073709551612, which has fun consequences. I got a crash from a
spreadsheet with formulas that referenced some empty cells.
Set #VALUE! error in COVAR() and PEARSON() OpenCL implementation when
appropriate. Returning a "bare" NaN with no "double error" semantic payload
does not make sense. Bare NaNs are displayed in Calc as 'nan' which probably
is not what we want.
Set #VALUE! error in COVAR() OpenCL implementation when appropriate. Returning
-DBL_MAX doesn't make sense. The traditional C++ implementation and other
spreadsheet products return an error.
Set #DIV/0! error in VAR() OpenCL implementation when appropriate. Returning
DBL_MAX doesn't make sense. The traditional C++ implementation and other
spreadsheet products return an error.
Return a #DIV/0! error in one case and #VALUE! in another in the OpenCL
SLOPE() implementation instead of bare NANs. There are still many places in
this function where the code bluntly returns a bare NAN. That is always the
wrong thing to do. However, it is not certain which error code is the right
error in each case. One would have to check in each case how to get to that
place in the code, and compare to what the reference C++ implementation and
other spreadsheet products do in each case.
Return #VALUE! instead of NaN in the OpenCL NORMSINV()
Add NORMSDIST, VAR, CORREL, COVAR, PEARSON and SLOPE to the OpenCL-enabled
default opcode subset. Having these statistical functions perform fast is
essential in many cases, and their implementations seem to be correct now.
Change-Id: Idb202756f5b64e30b9bb87c00e340b8060694509
Reviewed-on: https://gerrit.libreoffice.org/14769
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
Tested-by: Miklos Vajna <vmiklos at collabora.co.uk>
diff --git a/officecfg/registry/schema/org/openoffice/Office/Calc.xcs b/officecfg/registry/schema/org/openoffice/Office/Calc.xcs
index b532729..8748054 100644
--- a/officecfg/registry/schema/org/openoffice/Office/Calc.xcs
+++ b/officecfg/registry/schema/org/openoffice/Office/Calc.xcs
@@ -1364,9 +1364,9 @@
<desc>The list of operator and function opcodes for which to use OpenCL. If a
formula contains only these operators and functions, it
might be calculated using OpenCL.</desc>
- <!-- numeric values correspond to +;-;*;/;RAND;SIN;COS;TAN;ATAN;EXP;LN;SQRT;NORMSINV;ROUND;POWER;SUMPRODUCT;MIN;MAX;SUM;PRODUCT;AVERAGE;COUNT;NORMDIST;SUMIFS -->
+ <!-- numeric values correspond to +;-;*;/;RAND;SIN;COS;TAN;ATAN;EXP;LN;SQRT;NORMSDIST;NORMSINV;ROUND;POWER;SUMPRODUCT;MIN;MAX;SUM;PRODUCT;AVERAGE;COUNT;VAR;NORMDIST;CORREL;COVAR;PEARSON;SLOPE;SUMIFS -->
</info>
- <value>40;41;42;43;66;82;83;84;88;102;103;104;149;204;209;213;222;223;224;225;226;227;236;403</value>
+ <value>40;41;42;43;66;82;83;84;88;102;103;104;146;149;204;209;213;222;223;224;225;226;227;231;236;343;344;345;348;403</value>
</prop>
<prop oor:name="OpenCLAutoSelect" oor:type="xs:boolean" oor:nillable="false">
<!-- UIHints: Tools - Options Spreadsheet Formula -->
diff --git a/sc/source/core/data/grouptokenconverter.cxx b/sc/source/core/data/grouptokenconverter.cxx
index bfb6de3..da3964c 100644
--- a/sc/source/core/data/grouptokenconverter.cxx
+++ b/sc/source/core/data/grouptokenconverter.cxx
@@ -63,7 +63,19 @@ SCROW ScGroupTokenConverter::trimLength(SCTAB nTab, SCCOL nCol1, SCCOL nCol2, SC
SCROW nLastRow = nRow + nRowLen - 1; // current last row.
nLastRow = mrDoc.GetLastDataRow(nTab, nCol1, nCol2, nLastRow);
if (nLastRow < (nRow + nRowLen - 1))
+ {
+ // This can end up negative! Was that the original intent, or
+ // is it accidental? Was it not like that originally but the
+ // surrounding conditions changed?
nRowLen = nLastRow - nRow + 1;
+ // Anyway, let's assume it doesn't make sense to return a
+ // negative value here. But should we then return 0 or 1? In
+ // the "Column is empty" case below, we return 1, why!? And,
+ // at the callsites there are tests for a zero value returned
+ // from this function (but not for a negative one).
+ if (nRowLen < 0)
+ nRowLen = 0;
+ }
else if (nLastRow == 0)
// Column is empty.
nRowLen = 1;
diff --git a/sc/source/core/opencl/op_statistical.cxx b/sc/source/core/opencl/op_statistical.cxx
index 7fe8eed..0b7c572 100644
--- a/sc/source/core/opencl/op_statistical.cxx
+++ b/sc/source/core/opencl/op_statistical.cxx
@@ -248,7 +248,7 @@ void OpVar::GenSlidingWindowFunction(std::stringstream &ss,
}
}
ss << " if (fCount <= 1.0)\n";
- ss << " return DBL_MAX;\n";
+ ss << " return CreateDoubleError(errDivisionByZero);\n";
ss << " else\n";
ss << " return vSum * pow(fCount - 1.0,-1.0);\n";
ss << "}\n";
@@ -3283,7 +3283,7 @@ void OpSlope::GenSlidingWindowFunction(std::stringstream &ss,
ss << " }\n";
ss << " if (fCount < 1.0)\n";
- ss << " return NAN;\n";
+ ss << " return CreateDoubleError(errNoValue);\n";
ss << " else\n";
ss << " {\n";
ss << " fMeanX = fSumX * pow(fCount,-1.0);\n";
@@ -3349,7 +3349,7 @@ void OpSlope::GenSlidingWindowFunction(std::stringstream &ss,
ss << " fSumSqrDeltaX += (argX-fMeanX) * (argX-fMeanX);\n";
ss << " }\n";
ss << " if(fSumSqrDeltaX == 0.0)\n";
- ss << " return NAN;\n";
+ ss << " return CreateDoubleError(errDivisionByZero);\n";
ss << " else\n";
ss << " {\n";
ss << " return fSumDeltaXDeltaY*pow(fSumSqrDeltaX,-1.0);\n";
@@ -4048,6 +4048,8 @@ void OpPearson::GenSlidingWindowFunction(
ss << " }\n";
ss << " double tmp = ( fSumDeltaXDeltaY / ";
ss << "sqrt( fSumX * fSumY));\n\t";
+ ss << " if (isnan(tmp))\n";
+ ss << " return CreateDoubleError(errNoValue);\n";
ss << " return tmp;\n";
ss << "}\n";
}
@@ -5594,8 +5596,9 @@ void OpNormsinv:: GenSlidingWindowFunction
ss <<"}\n";
ss << "z = q < 0.0 ? (-1)*z : z;\n";
ss <<"}\n";
- ss <<"double tmp = z;\n";
- ss <<"return tmp;\n";
+ ss <<"if (isnan(z))\n";
+ ss <<" return CreateDoubleError(errNoValue);\n";
+ ss <<"return z;\n";
ss <<"}\n";
}
void OpMedian::GenSlidingWindowFunction(
@@ -8191,7 +8194,7 @@ void OpCovar::GenSlidingWindowFunction(std::stringstream& ss,
ss << " }\n";
}
ss << " if(cnt < 1) {\n";
- ss << " return -DBL_MAX;\n";
+ ss << " return CreateDoubleError(errNoValue);\n";
ss << " }\n";
ss << " else {\n";
ss << " vMean0 = vSum0 / cnt;\n";
diff --git a/sc/source/core/tool/calcconfig.cxx b/sc/source/core/tool/calcconfig.cxx
index 144d5a84..f1e7bf0 100644
--- a/sc/source/core/tool/calcconfig.cxx
+++ b/sc/source/core/tool/calcconfig.cxx
@@ -53,6 +53,7 @@ void ScCalcConfig::setOpenCLConfigToDefault()
maOpenCLSubsetOpCodes.insert(ocExp);
maOpenCLSubsetOpCodes.insert(ocLn);
maOpenCLSubsetOpCodes.insert(ocSqrt);
+ maOpenCLSubsetOpCodes.insert(ocStdNormDist);
maOpenCLSubsetOpCodes.insert(ocSNormInv);
maOpenCLSubsetOpCodes.insert(ocRound);
maOpenCLSubsetOpCodes.insert(ocPower);
@@ -63,7 +64,12 @@ void ScCalcConfig::setOpenCLConfigToDefault()
maOpenCLSubsetOpCodes.insert(ocProduct);
maOpenCLSubsetOpCodes.insert(ocAverage);
maOpenCLSubsetOpCodes.insert(ocCount);
+ maOpenCLSubsetOpCodes.insert(ocVar);
maOpenCLSubsetOpCodes.insert(ocNormDist);
+ maOpenCLSubsetOpCodes.insert(ocCorrel);
+ maOpenCLSubsetOpCodes.insert(ocCovar);
+ maOpenCLSubsetOpCodes.insert(ocPearson);
+ maOpenCLSubsetOpCodes.insert(ocSlope);
maOpenCLSubsetOpCodes.insert(ocSumIfs);
}
More information about the Libreoffice-commits
mailing list