From 076e286e497a8f7378fd696328ff71f2bbad51ef Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 11 Jun 2024 15:57:10 -0700 Subject: [PATCH 01/18] Updated Numeric calculations and tests, corrected median and percentile to ignore NULL values. --- engine/function/src/templates/Numeric.ftl | 116 ++++++++++++++++-- engine/function/src/templates/TestNumeric.ftl | 82 +++++++++++-- 2 files changed, 180 insertions(+), 18 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index ccedc0d79d0..32406a09d92 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -14,6 +14,7 @@ import java.util.Arrays; import static io.deephaven.base.CompareUtils.compare; import static io.deephaven.util.QueryConstants.*; import static io.deephaven.function.Basic.*; +import static io.deephaven.function.Sort.*; import static io.deephaven.function.Cast.castDouble; /** @@ -418,6 +419,7 @@ public class Numeric { double sum = 0; double count = 0; + long nullCount = 0; try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { @@ -431,10 +433,16 @@ public class Numeric { if (!isNull(c)) { sum += Math.abs(c); count++; + } else { + nullCount++; } } } + if (nullCount == values.size()) { + return NULL_DOUBLE; + } + return sum / count; } @@ -485,6 +493,7 @@ public class Numeric { double sum = 0; double sum2 = 0; long count = 0; + long nullCount = 0; try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); @@ -495,10 +504,16 @@ public class Numeric { sum += (double)c; sum2 += (double)c * (double)c; count++; + } else { + nullCount++; } } } + if (nullCount == values.size()) { + return NULL_DOUBLE; + } + // Return NaN if overflow or too few values to compute variance. if (count <= 1 || Double.isInfinite(sum) || Double.isInfinite(sum2)) { return Double.NaN; @@ -596,6 +611,7 @@ public class Numeric { double sum2 = 0; double count = 0; double count2 = 0; + long nullCount = 0; try ( final ${pt.vectorIterator} vi = values.iterator(); @@ -615,10 +631,16 @@ public class Numeric { sum2 += w * c * c; count += w; count2 += w * w; + } else { + nullCount++; } } } + if (nullCount == values.size()) { + return NULL_DOUBLE; + } + // Return NaN if overflow or too few values to compute variance. if (count <= 1 || Double.isInfinite(sum) || Double.isInfinite(sum2)) { return Double.NaN; @@ -868,6 +890,11 @@ public class Numeric { throw new IllegalArgumentException("Incompatible input sizes: " + values.size() + ", " + weights.size()); } + final double s = wstd(values, weights); + if (s == NULL_DOUBLE) { + return NULL_DOUBLE; + } + // see https://stats.stackexchange.com/questions/25895/computing-standard-error-in-weighted-mean-estimation double sumw = 0; double sumw2 = 0; @@ -887,7 +914,6 @@ public class Numeric { } } - final double s = wstd(values, weights); return s == NULL_DOUBLE ? NULL_DOUBLE : s * Math.sqrt(sumw2/sumw/sumw); } @@ -996,7 +1022,15 @@ public class Numeric { } final double a = wavg(values, weights); + if (a == NULL_DOUBLE) { + return NULL_DOUBLE; + } + final double s = wste(values, weights); + if (s == NULL_DOUBLE) { + return NULL_DOUBLE; + } + return a / s; } @@ -1226,13 +1260,38 @@ public class Numeric { int n = values.intSize("median"); if (n == 0) { - return Double.NaN; + return NULL_DOUBLE; } else { - ${pt.primitive}[] copy = values.copyToArray(); - Arrays.sort(copy); - if (n % 2 == 0) - return 0.5 * (copy[n / 2 - 1] + copy[n / 2]); - else return copy[n / 2]; + // NULL values sorted to beginning of the array. + ${pt.primitive}[] copy = sort(values.toArray()); + + // Determine if there are any NULL in the array. + int nullCount = 0; + for (int i = 0; i < n; i++) { + if (isNull(copy[i])) { + nullCount++; + } else { + break; + } + } + + if (nullCount == 0) { + // No NULL, so we can just compute the median and return. + if (n % 2 == 0) + return 0.5 * (copy[n / 2 - 1] + copy[n / 2]); + else return copy[n / 2]; + } else if (nullCount < n) { + // Some NULL, reduce the count and compute the median of the non-null values. + n -= nullCount; + if (n % 2 == 0) { + int index = n / 2; + return 0.5 * (copy[n / 2 - 1 + nullCount] + copy[n / 2 + nullCount]); + } + else return copy[n / 2 + nullCount]; + } else { + // All values are NULL. + return NULL_DOUBLE; + } } } @@ -1268,11 +1327,32 @@ public class Numeric { } int n = values.intSize("percentile"); - ${pt.primitive}[] copy = values.copyToArray(); - Arrays.sort(copy); + // NULL values sorted to beginning of the array. + ${pt.primitive}[] copy = sort(values.toArray()); + + // Determine if there are any NULL in the array. + int nullCount = 0; + for (int i = 0; i < n; i++) { + if (isNull(copy[i])) { + nullCount++; + } else { + break; + } + } - int idx = (int) Math.round(percentile * (n - 1)); - return copy[idx]; + if (nullCount == 0) { + // No NULL, so we can just compute the index and return. + int idx = (int) Math.round(percentile * (n - 1)); + return copy[idx]; + } else if (nullCount < n) { + // Some NULL, reduce the count and compute the median of the non-null values. + n -= nullCount; + int idx = (int) Math.round(percentile * (n - 1)); + return copy[idx + nullCount]; + } else { + // All values are NULL. + return ${pt.null}; + } } @@ -1344,6 +1424,7 @@ public class Numeric { double sum1 = 0; double sum01 = 0; double count = 0; + long nullCount = 0; try ( final ${pt.vectorIterator} v0i = values0.iterator(); @@ -1364,10 +1445,16 @@ public class Numeric { sum1 += v1; sum01 += v0 * v1; count++; + } else { + nullCount++; } } } + if (nullCount == values0.size()) { + return NULL_DOUBLE; + } + return sum01 / count - sum0 * sum1 / count / count; } @@ -1438,6 +1525,7 @@ public class Numeric { double sum1Sq = 0; double sum01 = 0; double count = 0; + long nullCount = 0; try ( final ${pt.vectorIterator} v0i = values0.iterator(); @@ -1460,10 +1548,16 @@ public class Numeric { sum1Sq += v1 * v1; sum01 += v0 * v1; count++; + } else { + nullCount++; } } } + if (nullCount == values0.size()) { + return NULL_DOUBLE; + } + double cov = sum01 / count - sum0 * sum1 / count / count; double var0 = sum0Sq / count - sum0 * sum0 / count / count; double var1 = sum1Sq / count - sum1 * sum1 / count / count; diff --git a/engine/function/src/templates/TestNumeric.ftl b/engine/function/src/templates/TestNumeric.ftl index 1805eb3a396..2d82acf9505 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -117,25 +117,29 @@ public class TestNumeric extends BaseArrayTestCase { public void test${pt.boxed}AbsAvg() { assertEquals(50.0, absAvg(new ${pt.primitive}[]{40, (${pt.primitive}) 50, 60})); assertEquals(45.5, absAvg(new ${pt.primitive}[]{(${pt.primitive}) 40, 51})); - assertTrue(Double.isNaN(absAvg(new ${pt.primitive}[]{}))); - assertTrue(Double.isNaN(absAvg(new ${pt.primitive}[]{${pt.null}}))); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.primitive}[]{})); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.primitive}[]{${pt.null}})); assertEquals(10.0, absAvg(new ${pt.primitive}[]{(${pt.primitive}) 5, ${pt.null}, (${pt.primitive}) 15})); assertEquals(NULL_DOUBLE, absAvg((${pt.primitive}[])null)); assertEquals(50.0, absAvg(new ${pt.boxed}[]{(${pt.primitive})40, (${pt.primitive}) 50, (${pt.primitive})60})); assertEquals(45.5, absAvg(new ${pt.boxed}[]{(${pt.primitive}) 40, (${pt.primitive})51})); - assertTrue(Double.isNaN(absAvg(new ${pt.boxed}[]{}))); - assertTrue(Double.isNaN(absAvg(new ${pt.boxed}[]{${pt.null}}))); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.boxed}[]{})); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.boxed}[]{${pt.null}})); assertEquals(10.0, absAvg(new ${pt.boxed}[]{(${pt.primitive}) 5, ${pt.null}, (${pt.primitive}) 15})); assertEquals(NULL_DOUBLE, absAvg((${pt.boxed}[])null)); assertEquals(50.0, absAvg(new ${pt.vectorDirect}(new ${pt.primitive}[]{40, (${pt.primitive}) 50, 60}))); assertEquals(45.5, absAvg(new ${pt.vectorDirect}(new ${pt.primitive}[]{(${pt.primitive}) 40, 51}))); - assertTrue(Double.isNaN(absAvg(new ${pt.vectorDirect}()))); - assertTrue(Double.isNaN(absAvg(new ${pt.vectorDirect}(${pt.null})))); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.vectorDirect}())); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.vectorDirect}(${pt.null}))); assertEquals(10.0, absAvg(new ${pt.vectorDirect}((${pt.primitive}) 5, ${pt.null}, (${pt.primitive}) 15))); assertEquals(NULL_DOUBLE, absAvg((${pt.vectorDirect})null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, absAvg(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, absAvg(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // check that functions can be resolved with varargs assertEquals(45.0, absAvg((${pt.primitive})40, (${pt.primitive})50)); } @@ -309,6 +313,10 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(var, var(new ${pt.vectorDirect}(v))); assertEquals(NULL_DOUBLE, var((${pt.vectorDirect})null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, var(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, var(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // check that functions can be resolved with varargs assertEquals(var, var((${pt.primitive})0, (${pt.primitive})40, ${pt.null}, (${pt.primitive})50, (${pt.primitive})60, (${pt.primitive}) -1, (${pt.primitive})0)); } @@ -326,6 +334,10 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(Math.sqrt(var(new ${pt.vectorDirect}(v))), std(new ${pt.vectorDirect}(v))); assertEquals(NULL_DOUBLE, std((${pt.vectorDirect})null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, std(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, std(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // check that functions can be resolved with varargs assertEquals(std(v), std((${pt.primitive})0, (${pt.primitive})40, ${pt.null}, (${pt.primitive})50, (${pt.primitive})60, (${pt.primitive}) -1, (${pt.primitive})0)); } @@ -343,6 +355,10 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(std(new ${pt.vectorDirect}(v)) / Math.sqrt(count(new ${pt.vectorDirect}(v))), ste(new ${pt.vectorDirect}(v))); assertEquals(NULL_DOUBLE, ste((${pt.vectorDirect})null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, ste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, ste(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // check that functions can be resolved with varargs assertEquals(ste(v), ste((${pt.primitive})0, (${pt.primitive})40, ${pt.null}, (${pt.primitive})50, (${pt.primitive})60, (${pt.primitive}) -1, (${pt.primitive})0)); } @@ -360,6 +376,10 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(avg(new ${pt.vectorDirect}(v)) / ste(new ${pt.vectorDirect}(v)), tstat(new ${pt.vectorDirect}(v))); assertEquals(NULL_DOUBLE, tstat((${pt.vectorDirect})null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, tstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, tstat(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // check that functions can be resolved with varargs assertEquals(tstat(v), tstat((${pt.primitive})0, (${pt.primitive})40, ${pt.null}, (${pt.primitive})50, (${pt.primitive})60, (${pt.primitive}) -1, (${pt.primitive})0)); } @@ -509,6 +529,10 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, cov((${pt.vectorDirect})null, new ${pt2.vectorDirect}(b))); assertEquals(NULL_DOUBLE, cov((${pt.vectorDirect})null, (${pt2.vectorDirect})null)); + // verify the all-null cases return null + assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); try { cov(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3,${pt.null},5}), new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5})); @@ -552,6 +576,11 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, cor((${pt.vectorDirect})null, new ${pt2.vectorDirect}(b))); assertEquals(NULL_DOUBLE, cor((${pt.vectorDirect})null, (${pt2.vectorDirect})null)); + // verify the all-null cases return null + assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + try { cor(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3,${pt.null},5}), new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5})); fail("Mismatched arguments"); @@ -1092,7 +1121,7 @@ public class TestNumeric extends BaseArrayTestCase { } public void test${pt.boxed}Median() { - assertEquals(Double.NaN, median(new ${pt.primitive}[]{})); + assertEquals(NULL_DOUBLE, median(new ${pt.primitive}[]{})); assertEquals(3.0, median(new ${pt.primitive}[]{4,2,3})); assertEquals(3.5, median(new ${pt.primitive}[]{5,4,2,3})); @@ -1106,6 +1135,17 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(3.5, median(new ${pt.vectorDirect}(new ${pt.primitive}[]{5,4,2,3}))); assertEquals(NULL_DOUBLE, median((${pt.vector}) null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, median(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, median(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + + // verify the mixed-null cases + assertEquals(3.0, median(new ${pt.primitive}[]{(${pt.primitive})4,(${pt.primitive})2,(${pt.primitive})3,${pt.null},${pt.null},${pt.null}})); + assertEquals(3.5, median(new ${pt.primitive}[]{(${pt.primitive})4,(${pt.primitive})2,(${pt.primitive})3,(${pt.primitive})5, ${pt.null},${pt.null},${pt.null}})); + + assertEquals(3.0, median(new ${pt.boxed}[]{(${pt.primitive})4,(${pt.primitive})2,(${pt.primitive})3,${pt.null},${pt.null}})); + assertEquals(3.5, median(new ${pt.boxed}[]{(${pt.primitive})4,(${pt.primitive})2,(${pt.primitive})3,(${pt.primitive})5,${pt.null},${pt.null}})); + // check that functions can be resolved with varargs assertEquals(3.0, median((${pt.primitive})4, (${pt.primitive})2, (${pt.primitive})3)); } @@ -1135,6 +1175,15 @@ public class TestNumeric extends BaseArrayTestCase { // pass } + // verify the all-null case returns null + assertEquals(${pt.null}, percentile(0.00, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(${pt.null}, percentile(0.25, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(${pt.null}, percentile(0.50, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + + // verify the mixed-null cases + assertEquals((${pt.primitive})2, percentile(0.00, new ${pt.primitive}[]{4,2,3,${pt.null}})); + assertEquals((${pt.primitive})3, percentile(0.50, new ${pt.primitive}[]{4,2,3,${pt.null},${pt.null}})); + assertEquals((${pt.primitive})4, percentile(1.0, new ${pt.primitive}[]{4,2,3,${pt.null},${pt.null},${pt.null}})); } public void test${pt.boxed}Wsum() { @@ -1286,6 +1335,11 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(var(new ${pt.primitive}[]{1,2,3}), wvar(new ${pt.primitive}[]{1,2,3,${pt.null},5}, new ${pt2.primitive}[]{1,1,1,7,${pt2.null}})); assertEquals(var(new ${pt.primitive}[]{1,2,3}), wvar(new ${pt.primitive}[]{1,2,3,${pt.null},5}, new ${pt2.primitive}[]{2,2,2,7,${pt2.null}})); + // verify the all-null cases return null + assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + } @@ -1316,6 +1370,11 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, wstd((${pt.vector}) null, new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5,6}))); assertEquals(NULL_DOUBLE, wstd(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3}), (${pt2.vector}) null)); + // verify the all-null cases return null + assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + } @@ -1355,6 +1414,10 @@ public class TestNumeric extends BaseArrayTestCase { // pass } + // verify the all-null cases return null + assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); @@ -1382,6 +1445,11 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, wtstat((${pt.vector}) null, new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5,6}))); assertEquals(NULL_DOUBLE, wtstat(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3}), (${pt2.vector}) null)); + // verify the all-null cases return null + assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + } From dd9f507ac0bb3dacc8a54dbd401967094f7a5cae Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 11 Jun 2024 17:11:58 -0700 Subject: [PATCH 02/18] Correct Numeric avg() function and tests. --- engine/function/src/templates/Numeric.ftl | 7 +++++++ engine/function/src/templates/TestNumeric.ftl | 16 ++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 32406a09d92..3fd052df50c 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -365,6 +365,7 @@ public class Numeric { double sum = 0; double count = 0; + long nullCount = 0; try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { @@ -375,10 +376,16 @@ public class Numeric { if (!isNull(c)) { sum += c; count++; + } else { + nullCount++; } } } + if (nullCount == values.size()) { + return NULL_DOUBLE; + } + return sum / count; } diff --git a/engine/function/src/templates/TestNumeric.ftl b/engine/function/src/templates/TestNumeric.ftl index 2d82acf9505..7950ce4e787 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -91,25 +91,29 @@ public class TestNumeric extends BaseArrayTestCase { public void test${pt.boxed}Avg() { assertEquals(50.0, avg(new ${pt.primitive}[]{40, 50, 60})); assertEquals(45.5, avg(new ${pt.primitive}[]{40, 51})); - assertTrue(Double.isNaN(avg(new ${pt.primitive}[]{}))); - assertTrue(Double.isNaN(avg(new ${pt.primitive}[]{${pt.null}}))); + assertEquals(NULL_DOUBLE, avg(new ${pt.primitive}[]{})); + assertEquals(NULL_DOUBLE, avg(new ${pt.primitive}[]{${pt.null}})); assertEquals(10.0, avg(new ${pt.primitive}[]{5, ${pt.null}, 15})); assertEquals(NULL_DOUBLE, avg((${pt.primitive}[])null)); assertEquals(50.0, avg(new ${pt.boxed}[]{(${pt.primitive})40, (${pt.primitive})50, (${pt.primitive})60})); assertEquals(45.5, avg(new ${pt.boxed}[]{(${pt.primitive})40, (${pt.primitive})51})); - assertTrue(Double.isNaN(avg(new ${pt.boxed}[]{}))); - assertTrue(Double.isNaN(avg(new ${pt.boxed}[]{${pt.null}}))); + assertEquals(NULL_DOUBLE, avg(new ${pt.boxed}[]{})); + assertEquals(NULL_DOUBLE, avg(new ${pt.boxed}[]{${pt.null}})); assertEquals(10.0, avg(new ${pt.boxed}[]{(${pt.primitive})5, ${pt.null}, (${pt.primitive})15})); assertEquals(NULL_DOUBLE, avg((${pt.boxed}[])null)); assertEquals(50.0, avg(new ${pt.vectorDirect}(new ${pt.primitive}[]{40, 50, 60}))); assertEquals(45.5, avg(new ${pt.vectorDirect}(new ${pt.primitive}[]{40, 51}))); - assertTrue(Double.isNaN(avg(new ${pt.vectorDirect}()))); - assertTrue(Double.isNaN(avg(new ${pt.vectorDirect}(${pt.null})))); + assertEquals(NULL_DOUBLE, avg(new ${pt.vectorDirect}())); + assertEquals(NULL_DOUBLE, avg(new ${pt.vectorDirect}(${pt.null}))); assertEquals(10.0, avg(new ${pt.vectorDirect}(new ${pt.primitive}[]{5, ${pt.null}, 15}))); assertEquals(NULL_DOUBLE, avg((${pt.vectorDirect})null)); + // verify the all-null case returns null + assertEquals(NULL_DOUBLE, avg(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, avg(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // check that functions can be resolved with varargs assertEquals(45.0, avg((${pt.primitive})40, (${pt.primitive})50)); } From 5bf465e92e0b3e5e9858f7a7df6ce66f83a97d1d Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 11 Jun 2024 17:13:55 -0700 Subject: [PATCH 03/18] UpdateBy corrections for all-NULL vectors --- .../rollingavg/ByteRollingAvgOperator.java | 4 +-- .../rollingavg/CharRollingAvgOperator.java | 4 +-- .../rollingavg/DoubleRollingAvgOperator.java | 2 +- .../rollingavg/FloatRollingAvgOperator.java | 2 +- .../rollingavg/IntRollingAvgOperator.java | 4 +-- .../rollingavg/LongRollingAvgOperator.java | 4 +-- .../rollingavg/ShortRollingAvgOperator.java | 4 +-- .../rollingstd/ByteRollingStdOperator.java | 7 +++- .../rollingstd/CharRollingStdOperator.java | 7 +++- .../rollingstd/DoubleRollingStdOperator.java | 7 +++- .../rollingstd/FloatRollingStdOperator.java | 7 +++- .../rollingstd/IntRollingStdOperator.java | 7 +++- .../rollingstd/LongRollingStdOperator.java | 7 +++- .../rollingstd/ShortRollingStdOperator.java | 7 +++- .../table/impl/updateby/TestRollingAvg.java | 11 ++++-- .../table/impl/updateby/TestRollingCount.java | 10 ++++-- .../impl/updateby/TestRollingFormula.java | 36 +++++++------------ .../table/impl/updateby/TestRollingGroup.java | 8 +++++ .../impl/updateby/TestRollingMinMax.java | 8 +++++ .../impl/updateby/TestRollingProduct.java | 11 ++++-- .../table/impl/updateby/TestRollingStd.java | 11 ++++-- .../table/impl/updateby/TestRollingSum.java | 2 +- .../table/impl/updateby/TestRollingWAvg.java | 8 +++++ 23 files changed, 126 insertions(+), 52 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ByteRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ByteRollingAvgOperator.java index adc9ca532d4..8a3ec4ccd8f 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ByteRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ByteRollingAvgOperator.java @@ -87,12 +87,12 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (byteWindowValues.size() == 0) { + if (byteWindowValues.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = byteWindowValues.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, curVal / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/CharRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/CharRollingAvgOperator.java index e5620d15cc2..97dc280c981 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/CharRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/CharRollingAvgOperator.java @@ -82,12 +82,12 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (charWindowValues.size() == 0) { + if (charWindowValues.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = charWindowValues.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, curVal / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/DoubleRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/DoubleRollingAvgOperator.java index 3e53befcd5c..7bc1cf11e02 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/DoubleRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/DoubleRollingAvgOperator.java @@ -87,7 +87,7 @@ public void writeToOutputChunk(int outIdx) { } else { final int count = aggSum.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, aggSum.evaluate() / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/FloatRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/FloatRollingAvgOperator.java index 4f4eaa67053..0dcb3855312 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/FloatRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/FloatRollingAvgOperator.java @@ -83,7 +83,7 @@ public void writeToOutputChunk(int outIdx) { } else { final int count = aggSum.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, aggSum.evaluate() / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/IntRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/IntRollingAvgOperator.java index fa73349b73f..2f6c5192691 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/IntRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/IntRollingAvgOperator.java @@ -86,12 +86,12 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (intWindowValues.size() == 0) { + if (intWindowValues.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = intWindowValues.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, curVal / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/LongRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/LongRollingAvgOperator.java index 72c6e7b639f..81541eae9c2 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/LongRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/LongRollingAvgOperator.java @@ -86,12 +86,12 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (longWindowValues.size() == 0) { + if (longWindowValues.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = longWindowValues.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, curVal / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ShortRollingAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ShortRollingAvgOperator.java index 23512f1ab4d..f256a88c3a8 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ShortRollingAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingavg/ShortRollingAvgOperator.java @@ -86,12 +86,12 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (shortWindowValues.size() == 0) { + if (shortWindowValues.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = shortWindowValues.size() - nullCount; if (count == 0) { - outputValues.set(outIdx, Double.NaN); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, curVal / (double) count); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ByteRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ByteRollingStdOperator.java index 15c40b34176..a87a630b841 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ByteRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ByteRollingStdOperator.java @@ -110,9 +110,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/CharRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/CharRollingStdOperator.java index f622450770f..5ab4fb484de 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/CharRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/CharRollingStdOperator.java @@ -105,9 +105,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/DoubleRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/DoubleRollingStdOperator.java index 1bd631bfd62..2a4b52b0493 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/DoubleRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/DoubleRollingStdOperator.java @@ -109,9 +109,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/FloatRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/FloatRollingStdOperator.java index f15227cd488..0653990e4e4 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/FloatRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/FloatRollingStdOperator.java @@ -109,9 +109,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/IntRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/IntRollingStdOperator.java index 2c71b961993..5ffe50e2657 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/IntRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/IntRollingStdOperator.java @@ -109,9 +109,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/LongRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/LongRollingStdOperator.java index b4706c05d75..ba6527c6682 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/LongRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/LongRollingStdOperator.java @@ -109,9 +109,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ShortRollingStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ShortRollingStdOperator.java index 193216665f4..4ef7f286514 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ShortRollingStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingstd/ShortRollingStdOperator.java @@ -109,9 +109,14 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { - if (valueBuffer.size() == 0) { + if (valueBuffer.isEmpty()) { outputValues.set(outIdx, NULL_DOUBLE); } else { + if (nullCount == valueBuffer.size()) { + outputValues.set(outIdx, NULL_DOUBLE); + return; + } + final int count = valueBuffer.size() - nullCount; if (count <= 1) { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingAvg.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingAvg.java index 05b93595eab..3c9fdfec4d8 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingAvg.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingAvg.java @@ -78,8 +78,7 @@ public class TestRollingAvg extends BaseUpdateByTest { private String[] getFormulas(String[] columns) { return Arrays.stream(columns) - // Force null instead of NaN when vector size == 0 - .map(c -> String.format("%s=%s.size() == 0 ? null : avg(%s)", c, c, c)) + .map(c -> String.format("%s=avg(%s)", c, c)) .toArray(String[]::new); } @@ -290,6 +289,14 @@ private void doTestStaticBucketedTimedBigNumbers(final QueryTable t, final Durat // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 100; diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingCount.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingCount.java index d80c33edeb9..8fe83dfb661 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingCount.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingCount.java @@ -74,8 +74,7 @@ public class TestRollingCount extends BaseUpdateByTest { private String[] getFormulas(String[] columns) { return Arrays.stream(columns) - // Force null instead of NaN when vector size == 0 - .map(c -> String.format("%s=count(%s)", c, c, c)) + .map(c -> String.format("%s=count(%s)", c, c)) .toArray(String[]::new); } @@ -235,6 +234,13 @@ private void doTestStaticBucketedTimedBigNumbers(final QueryTable t, final Durat // endregion Object Helper functions // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } @Test public void testStaticZeroKeyRev() { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingFormula.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingFormula.java index 685f19dde43..7238a47c1d0 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingFormula.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingFormula.java @@ -111,6 +111,14 @@ public static BigInteger sumBigInteger(ObjectVector bigIntegerObject // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 100; @@ -263,12 +271,7 @@ private void doTestStaticZeroKey(final int prevTicks, final int postTicks) { //////////////////////////////////////////////////////////////////////////////////////////////////// actual = t.updateBy(UpdateByOperation.RollingFormula(prevTicks, postTicks, "avg(x)", "x", primitiveColumns)); - - // avg return Double.NaN when the window is empty, so we should adjust our comparison table. - updateStrings = Arrays.stream(primitiveColumns).map(c -> c + "=isNull(" + c + ") ? Double.NaN : " + c) - .toArray(String[]::new); - expected = t.updateBy(UpdateByOperation.RollingAvg(prevTicks, postTicks, primitiveColumns)) - .update(updateStrings); + expected = t.updateBy(UpdateByOperation.RollingAvg(prevTicks, postTicks, primitiveColumns)); TstUtils.assertTableEquals(expected, actual, TableDiff.DiffItems.DoublesExact); @@ -386,12 +389,7 @@ private void doTestStaticZeroKeyTimed(final Duration prevTime, final Duration po actual = t .updateBy(UpdateByOperation.RollingFormula("ts", prevTime, postTime, "avg(x)", "x", primitiveColumns)); - - // avg return Double.NaN when the window is empty, so we should adjust our comparison table. - updateStrings = Arrays.stream(primitiveColumns).map(c -> c + "=isNull(" + c + ") ? Double.NaN : " + c) - .toArray(String[]::new); - expected = t.updateBy(UpdateByOperation.RollingAvg("ts", prevTime, postTime, primitiveColumns)) - .update(updateStrings); + expected = t.updateBy(UpdateByOperation.RollingAvg("ts", prevTime, postTime, primitiveColumns)); TstUtils.assertTableEquals(expected, actual, TableDiff.DiffItems.DoublesExact); @@ -601,12 +599,7 @@ private void doTestStaticBucketed(boolean grouped, int prevTicks, int postTicks) actual = t.updateBy(UpdateByOperation.RollingFormula(prevTicks, postTicks, "avg(x)", "x", primitiveColumns), "Sym"); - - // avg return Double.NaN when the window is empty, so we should adjust our comparison table. - updateStrings = Arrays.stream(primitiveColumns).map(c -> c + "=isNull(" + c + ") ? Double.NaN : " + c) - .toArray(String[]::new); - expected = t.updateBy(UpdateByOperation.RollingAvg(prevTicks, postTicks, primitiveColumns), "Sym") - .update(updateStrings); + expected = t.updateBy(UpdateByOperation.RollingAvg(prevTicks, postTicks, primitiveColumns), "Sym"); TstUtils.assertTableEquals(expected, actual, TableDiff.DiffItems.DoublesExact); @@ -726,12 +719,7 @@ private void doTestStaticBucketedTimed(boolean grouped, Duration prevTime, Durat actual = t.updateBy(UpdateByOperation.RollingFormula("ts", prevTime, postTime, "avg(x)", "x", primitiveColumns), "Sym"); - - // avg return Double.NaN when the window is empty, so we should adjust our comparison table. - updateStrings = Arrays.stream(primitiveColumns).map(c -> c + "=isNull(" + c + ") ? Double.NaN : " + c) - .toArray(String[]::new); - expected = t.updateBy(UpdateByOperation.RollingAvg("ts", prevTime, postTime, primitiveColumns), "Sym") - .update(updateStrings); + expected = t.updateBy(UpdateByOperation.RollingAvg("ts", prevTime, postTime, primitiveColumns), "Sym"); TstUtils.assertTableEquals(expected, actual, TableDiff.DiffItems.DoublesExact); diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingGroup.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingGroup.java index e9faa243e10..2c6befbb805 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingGroup.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingGroup.java @@ -64,6 +64,14 @@ public class TestRollingGroup extends BaseUpdateByTest { // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 100; diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingMinMax.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingMinMax.java index d5598242124..c8d51cd5394 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingMinMax.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingMinMax.java @@ -473,6 +473,14 @@ private void doTestStaticBucketedTimedBigNumbers(final QueryTable t, final Durat // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 100; diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingProduct.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingProduct.java index 609035daa38..8107ce5e716 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingProduct.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingProduct.java @@ -91,8 +91,7 @@ public EnumSet diffItems() { private String[] getFormulas(String[] columns) { return Arrays.stream(columns) - // Force null instead of NaN when vector size == 0 - .map(c -> String.format("%s=%s.size() == 0 ? null : product(%s)", c, c, c)) + .map(c -> String.format("%s=product(%s)", c, c, c)) .toArray(String[]::new); } @@ -370,6 +369,14 @@ private void doTestStaticBucketedTimedBigNumbers(final QueryTable t, final Durat // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 10; diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingStd.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingStd.java index 787eb3d1bda..52613920d6e 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingStd.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingStd.java @@ -78,8 +78,7 @@ public class TestRollingStd extends BaseUpdateByTest { private String[] getFormulas(String[] columns) { return Arrays.stream(columns) - // Force null instead of NaN when vector size == 0 - .map(c -> String.format("%s=%s.size() == 0 ? null : std(%s)", c, c, c)) + .map(c -> String.format("%s=std(%s)", c, c)) .toArray(String[]::new); } @@ -316,6 +315,14 @@ private void doTestStaticBucketedTimedBigNumbers(final QueryTable t, final Durat // region Static Zero Key Tests + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStaticZeroKey(prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 100; diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingSum.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingSum.java index c985d4dd766..66a10af9287 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingSum.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingSum.java @@ -62,7 +62,7 @@ public EnumSet diffItems() { // region Static Zero Key Tests @Test - public void testStaticZeroKeyWithAllNullWindows() { + public void testStaticZeroKeyAllNullVector() { final QueryTable t = createTestTable(10000, false, false, false, 0x31313131).t; t.setRefreshing(false); diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingWAvg.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingWAvg.java index d8d9ffa0cb9..33b93422407 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingWAvg.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestRollingWAvg.java @@ -682,6 +682,14 @@ private void doTestStaticTimed(boolean bucketed, Duration prevTime, Duration pos doTestStaticTimedBigNumbers(t, prevTime, postTime, bucketed, weightCol, "wavgBigIntBigDec", "wavgBigDecBigDec"); } + @Test + public void testStaticZeroKeyAllNullVector() { + final int prevTicks = 1; + final int postTicks = 0; + + doTestStatic(false, prevTicks, postTicks); + } + @Test public void testStaticZeroKeyRev() { final int prevTicks = 100; From ad2675d3ae3468f956d5bbb888ca7475fb74940e Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Wed, 12 Jun 2024 10:16:12 -0700 Subject: [PATCH 04/18] Agg corrections for avg,var,std --- .../engine/table/impl/by/ByteChunkedAvgOperator.java | 10 +++++++--- .../engine/table/impl/by/ByteChunkedVarOperator.java | 10 ++++++++-- .../engine/table/impl/by/CharChunkedAvgOperator.java | 10 +++++++--- .../engine/table/impl/by/CharChunkedVarOperator.java | 10 ++++++++-- .../table/impl/by/DoubleChunkedAvgOperator.java | 7 ++++++- .../table/impl/by/DoubleChunkedReAvgOperator.java | 6 +++++- .../table/impl/by/DoubleChunkedVarOperator.java | 11 +++++++++-- .../engine/table/impl/by/FloatChunkedAvgOperator.java | 7 ++++++- .../table/impl/by/FloatChunkedReAvgOperator.java | 6 +++++- .../table/impl/by/FloatChunkedReVarOperator.java | 8 ++++++-- .../engine/table/impl/by/FloatChunkedVarOperator.java | 11 +++++++++-- .../engine/table/impl/by/IntChunkedAvgOperator.java | 10 +++++++--- .../engine/table/impl/by/IntChunkedVarOperator.java | 10 ++++++++-- .../engine/table/impl/by/LongChunkedAvgOperator.java | 10 +++++++--- .../engine/table/impl/by/LongChunkedVarOperator.java | 10 ++++++++-- .../engine/table/impl/by/ShortChunkedAvgOperator.java | 10 +++++++--- .../engine/table/impl/by/ShortChunkedVarOperator.java | 10 ++++++++-- 17 files changed, 121 insertions(+), 35 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedAvgOperator.java index ac8a08af83c..34fdf0b5392 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedAvgOperator.java @@ -24,6 +24,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusLong; import static io.deephaven.engine.util.NullSafeAddition.minusLong; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative average operator. @@ -92,7 +93,7 @@ private boolean addChunk(ByteChunk values, long destination, i runningSum.set(destination, newSum); resultColumn.set(destination, (double) newSum / newCount); } else if (nonNullCount.onlyNullsUnsafe(destination)) { - resultColumn.set(destination, Double.NaN); + resultColumn.set(destination, NULL_DOUBLE); } else { return false; } @@ -110,8 +111,11 @@ private boolean removeChunk(ByteChunk values, long destination final long newCount = nonNullCount.addNonNullUnsafe(destination, -chunkNonNull.get()); final long newSum = minusLong(runningSum.getUnsafe(destination), chunkSum); runningSum.set(destination, newSum); - resultColumn.set(destination, (double) newSum / newCount); - + if (newCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else { + resultColumn.set(destination, (double) newSum / newCount); + } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java index bedc7465bb8..8775db456d5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java @@ -23,6 +23,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -99,8 +100,10 @@ private boolean addChunk(ByteChunk values, long destination, i final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) <= 1) { + } else if (nonNullCounter.getCountUnsafe(destination) == 1) { resultColumn.set(destination, Double.NaN); + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); } return true; } @@ -129,7 +132,10 @@ private boolean removeChunk(ByteChunk values, long destination sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (nonNullCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedAvgOperator.java index 130c588f20e..b5c2c12aec7 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedAvgOperator.java @@ -20,6 +20,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusLong; import static io.deephaven.engine.util.NullSafeAddition.minusLong; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative average operator. @@ -88,7 +89,7 @@ private boolean addChunk(CharChunk values, long destination, i runningSum.set(destination, newSum); resultColumn.set(destination, (double) newSum / newCount); } else if (nonNullCount.onlyNullsUnsafe(destination)) { - resultColumn.set(destination, Double.NaN); + resultColumn.set(destination, NULL_DOUBLE); } else { return false; } @@ -106,8 +107,11 @@ private boolean removeChunk(CharChunk values, long destination final long newCount = nonNullCount.addNonNullUnsafe(destination, -chunkNonNull.get()); final long newSum = minusLong(runningSum.getUnsafe(destination), chunkSum); runningSum.set(destination, newSum); - resultColumn.set(destination, (double) newSum / newCount); - + if (newCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else { + resultColumn.set(destination, (double) newSum / newCount); + } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java index bb653cce70e..e77f33333b6 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java @@ -19,6 +19,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -95,8 +96,10 @@ private boolean addChunk(CharChunk values, long destination, i final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) <= 1) { + } else if (nonNullCounter.getCountUnsafe(destination) == 1) { resultColumn.set(destination, Double.NaN); + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); } return true; } @@ -125,7 +128,10 @@ private boolean removeChunk(CharChunk values, long destination sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (nonNullCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedAvgOperator.java index 15666e608e0..ed0313ada8a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedAvgOperator.java @@ -22,6 +22,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; class DoubleChunkedAvgOperator extends FpChunkedNonNormalCounter implements IterativeChunkedAggregationOperator { private final String name; @@ -141,6 +142,8 @@ private void updateResultWithNewSum(long destination, long totalNormal, long tot resultColumn.set(destination, Double.POSITIVE_INFINITY); } else if (totalNegativeInfinityCount > 0) { resultColumn.set(destination, Double.NEGATIVE_INFINITY); + } else if (totalNormal == 0) { + resultColumn.set(destination, NULL_DOUBLE); } else { resultColumn.set(destination, newSum / totalNormal); } @@ -148,12 +151,14 @@ private void updateResultWithNewSum(long destination, long totalNormal, long tot private void updateResultSumUnchanged(long destination, long totalNormal, long totalNanCount, long totalInfinityCount, long totalNegativeInfinityCount) { - if (totalNanCount > 0 || totalNormal == 0 || (totalInfinityCount > 0 && totalNegativeInfinityCount > 0)) { + if (totalNanCount > 0 || (totalInfinityCount > 0 && totalNegativeInfinityCount > 0)) { resultColumn.set(destination, Double.NaN); } else if (totalInfinityCount > 0) { resultColumn.set(destination, Double.POSITIVE_INFINITY); } else if (totalNegativeInfinityCount > 0) { resultColumn.set(destination, Double.NEGATIVE_INFINITY); + } else if (totalNormal == 0) { + resultColumn.set(destination, NULL_DOUBLE); } else { resultColumn.set(destination, runningSum.getUnsafe(destination) / totalNormal); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java index 4bdffdf6183..ae311b3ebb9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java @@ -20,6 +20,8 @@ import java.util.Collections; import java.util.Map; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; + /** * Iterative average operator. */ @@ -127,8 +129,10 @@ private boolean updateResult(long destination) { private boolean updateResult(long destination, long nncValue, long nanValue, long picValue, long nicValue, double sumSumValue) { - if (nanValue > 0 || (picValue > 0 && nicValue > 0) || nncValue == 0) { + if (nanValue > 0 || (picValue > 0 && nicValue > 0)) { return !Double.isNaN(resultColumn.getAndSetUnsafe(destination, Double.NaN)); + } else if (nncValue == 0) { + return resultColumn.getAndSetUnsafe(destination, NULL_DOUBLE) != NULL_DOUBLE; } else if (picValue > 0) { return resultColumn.getAndSetUnsafe(destination, Double.POSITIVE_INFINITY) != Double.POSITIVE_INFINITY; } else if (nicValue > 0) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java index db5991d75e4..3b43885e66e 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java @@ -23,6 +23,7 @@ import java.util.Map; import static io.deephaven.engine.table.impl.by.RollupConstants.*; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -114,9 +115,12 @@ private boolean addChunk(DoubleChunk values, long destination, resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; - } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) <= 1)) { + } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) == 1)) { resultColumn.set(destination, Double.NaN); return true; + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; } else { return false; } @@ -165,9 +169,12 @@ private boolean removeChunk(DoubleChunk values, long destinati } sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - } else if (totalNormalCount <= 1 || forceNanResult) { + } else if (totalNormalCount == 1 || forceNanResult) { resultColumn.set(destination, Double.NaN); return true; + } else if (totalNormalCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; } else { newSum = sumSource.getUnsafe(destination); newSum2 = sum2Source.getUnsafe(destination); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedAvgOperator.java index f227389aa6b..e3cf530f7d7 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedAvgOperator.java @@ -18,6 +18,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; class FloatChunkedAvgOperator extends FpChunkedNonNormalCounter implements IterativeChunkedAggregationOperator { private final String name; @@ -137,6 +138,8 @@ private void updateResultWithNewSum(long destination, long totalNormal, long tot resultColumn.set(destination, Double.POSITIVE_INFINITY); } else if (totalNegativeInfinityCount > 0) { resultColumn.set(destination, Double.NEGATIVE_INFINITY); + } else if (totalNormal == 0) { + resultColumn.set(destination, NULL_DOUBLE); } else { resultColumn.set(destination, newSum / totalNormal); } @@ -144,12 +147,14 @@ private void updateResultWithNewSum(long destination, long totalNormal, long tot private void updateResultSumUnchanged(long destination, long totalNormal, long totalNanCount, long totalInfinityCount, long totalNegativeInfinityCount) { - if (totalNanCount > 0 || totalNormal == 0 || (totalInfinityCount > 0 && totalNegativeInfinityCount > 0)) { + if (totalNanCount > 0 || (totalInfinityCount > 0 && totalNegativeInfinityCount > 0)) { resultColumn.set(destination, Double.NaN); } else if (totalInfinityCount > 0) { resultColumn.set(destination, Double.POSITIVE_INFINITY); } else if (totalNegativeInfinityCount > 0) { resultColumn.set(destination, Double.NEGATIVE_INFINITY); + } else if (totalNormal == 0) { + resultColumn.set(destination, NULL_DOUBLE); } else { resultColumn.set(destination, runningSum.getUnsafe(destination) / totalNormal); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java index 7c42869077f..3c08ce4a8fa 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java @@ -16,6 +16,8 @@ import java.util.Collections; import java.util.Map; +import static io.deephaven.util.QueryConstants.NULL_FLOAT; + /** * Iterative average operator. */ @@ -123,8 +125,10 @@ private boolean updateResult(long destination) { private boolean updateResult(long destination, long nncValue, long nanValue, long picValue, long nicValue, double sumSumValue) { - if (nanValue > 0 || (picValue > 0 && nicValue > 0) || nncValue == 0) { + if (nanValue > 0 || (picValue > 0 && nicValue > 0)) { return !Float.isNaN(resultColumn.getAndSetUnsafe(destination, Float.NaN)); + } else if (nncValue == 0) { + return resultColumn.getAndSetUnsafe(destination, NULL_FLOAT) != NULL_FLOAT; } else if (picValue > 0) { return resultColumn.getAndSetUnsafe(destination, Float.POSITIVE_INFINITY) != Float.POSITIVE_INFINITY; } else if (nicValue > 0) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReVarOperator.java index 12df11fca34..7ce77d6c68f 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReVarOperator.java @@ -16,6 +16,8 @@ import java.util.Collections; import java.util.Map; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; + /** * Iterative average operator. */ @@ -91,7 +93,7 @@ private void updateResult(ReVarContext reVarContext, RowSequence destinationOk, nicSum.getChunk(reVarContext.nicContext, destinationOk).asLongChunk(); final int size = reVarContext.keyIndices.size(); - final boolean ordered = reVarContext.ordered;; + final boolean ordered = reVarContext.ordered; for (int ii = 0; ii < size; ++ii) { final boolean changed = updateResult(reVarContext.keyIndices.get(ii), nncSumChunk.get(ii), nanSumChunk.get(ii), @@ -131,8 +133,10 @@ private boolean updateResult(long destination) { private boolean updateResult(long destination, long nncValue, long nanValue, long picValue, long nicValue, double newSum, double newSum2) { - if (nanValue > 0 || picValue > 0 || nicValue > 0 || nncValue <= 1) { + if (nanValue > 0 || picValue > 0 || nicValue > 0 || nncValue == 1) { return !Double.isNaN(resultColumn.getAndSetUnsafe(destination, Double.NaN)); + } else if (nncValue == 0) { + return resultColumn.getAndSetUnsafe(destination, NULL_DOUBLE) != NULL_DOUBLE; } else { final double variance = (newSum2 - newSum * newSum / nncValue) / (nncValue - 1); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java index e3881adf0f2..eafbdfc0bfa 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java @@ -19,6 +19,7 @@ import java.util.Map; import static io.deephaven.engine.table.impl.by.RollupConstants.*; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -110,9 +111,12 @@ private boolean addChunk(FloatChunk values, long destination, resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; - } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) <= 1)) { + } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) == 1)) { resultColumn.set(destination, Double.NaN); return true; + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; } else { return false; } @@ -161,9 +165,12 @@ private boolean removeChunk(FloatChunk values, long destinatio } sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - } else if (totalNormalCount <= 1 || forceNanResult) { + } else if (totalNormalCount == 1 || forceNanResult) { resultColumn.set(destination, Double.NaN); return true; + } else if (totalNormalCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; } else { newSum = sumSource.getUnsafe(destination); newSum2 = sum2Source.getUnsafe(destination); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedAvgOperator.java index 81e66b75ff8..1adec564276 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedAvgOperator.java @@ -24,6 +24,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusLong; import static io.deephaven.engine.util.NullSafeAddition.minusLong; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative average operator. @@ -92,7 +93,7 @@ private boolean addChunk(IntChunk values, long destination, in runningSum.set(destination, newSum); resultColumn.set(destination, (double) newSum / newCount); } else if (nonNullCount.onlyNullsUnsafe(destination)) { - resultColumn.set(destination, Double.NaN); + resultColumn.set(destination, NULL_DOUBLE); } else { return false; } @@ -110,8 +111,11 @@ private boolean removeChunk(IntChunk values, long destination, final long newCount = nonNullCount.addNonNullUnsafe(destination, -chunkNonNull.get()); final long newSum = minusLong(runningSum.getUnsafe(destination), chunkSum); runningSum.set(destination, newSum); - resultColumn.set(destination, (double) newSum / newCount); - + if (newCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else { + resultColumn.set(destination, (double) newSum / newCount); + } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java index 882dc975285..eee1a6bf780 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java @@ -23,6 +23,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -99,8 +100,10 @@ private boolean addChunk(IntChunk values, long destination, in final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) <= 1) { + } else if (nonNullCounter.getCountUnsafe(destination) == 1) { resultColumn.set(destination, Double.NaN); + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); } return true; } @@ -129,7 +132,10 @@ private boolean removeChunk(IntChunk values, long destination, sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (nonNullCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedAvgOperator.java index 8c58a17b6d7..ce71da4691e 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedAvgOperator.java @@ -24,6 +24,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusLong; import static io.deephaven.engine.util.NullSafeAddition.minusLong; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative average operator. @@ -92,7 +93,7 @@ private boolean addChunk(LongChunk values, long destination, i runningSum.set(destination, newSum); resultColumn.set(destination, (double) newSum / newCount); } else if (nonNullCount.onlyNullsUnsafe(destination)) { - resultColumn.set(destination, Double.NaN); + resultColumn.set(destination, NULL_DOUBLE); } else { return false; } @@ -110,8 +111,11 @@ private boolean removeChunk(LongChunk values, long destination final long newCount = nonNullCount.addNonNullUnsafe(destination, -chunkNonNull.get()); final long newSum = minusLong(runningSum.getUnsafe(destination), chunkSum); runningSum.set(destination, newSum); - resultColumn.set(destination, (double) newSum / newCount); - + if (newCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else { + resultColumn.set(destination, (double) newSum / newCount); + } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java index 2b7b9a14bfa..40b5cf66ba8 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java @@ -23,6 +23,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -99,8 +100,10 @@ private boolean addChunk(LongChunk values, long destination, i final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) <= 1) { + } else if (nonNullCounter.getCountUnsafe(destination) == 1) { resultColumn.set(destination, Double.NaN); + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); } return true; } @@ -129,7 +132,10 @@ private boolean removeChunk(LongChunk values, long destination sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (nonNullCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedAvgOperator.java index 40bb2fd41b5..27c3ebdfbd9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedAvgOperator.java @@ -24,6 +24,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusLong; import static io.deephaven.engine.util.NullSafeAddition.minusLong; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative average operator. @@ -92,7 +93,7 @@ private boolean addChunk(ShortChunk values, long destination, runningSum.set(destination, newSum); resultColumn.set(destination, (double) newSum / newCount); } else if (nonNullCount.onlyNullsUnsafe(destination)) { - resultColumn.set(destination, Double.NaN); + resultColumn.set(destination, NULL_DOUBLE); } else { return false; } @@ -110,8 +111,11 @@ private boolean removeChunk(ShortChunk values, long destinatio final long newCount = nonNullCount.addNonNullUnsafe(destination, -chunkNonNull.get()); final long newSum = minusLong(runningSum.getUnsafe(destination), chunkSum); runningSum.set(destination, newSum); - resultColumn.set(destination, (double) newSum / newCount); - + if (newCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else { + resultColumn.set(destination, (double) newSum / newCount); + } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java index 6d24d6f75d6..9189fdcbadf 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java @@ -23,6 +23,7 @@ import static io.deephaven.engine.table.impl.by.RollupConstants.*; import static io.deephaven.engine.util.NullSafeAddition.plusDouble; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; /** * Iterative variance operator. @@ -99,8 +100,10 @@ private boolean addChunk(ShortChunk values, long destination, final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) <= 1) { + } else if (nonNullCounter.getCountUnsafe(destination) == 1) { resultColumn.set(destination, Double.NaN); + } else if (nonNullCounter.getCountUnsafe(destination) == 0) { + resultColumn.set(destination, NULL_DOUBLE); } return true; } @@ -129,7 +132,10 @@ private boolean removeChunk(ShortChunk values, long destinatio sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (nonNullCount == 1) { resultColumn.set(destination, Double.NaN); return true; } From 50667f28aec0e73097e2fbfc76c389d4b05ac7f4 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 13 Jun 2024 08:32:52 -0700 Subject: [PATCH 05/18] Agg test changes to reflect new output. --- .../table/impl/QueryTableAggregationTest.java | 12 ++++----- ...leAggregationTestFormulaStaticMethods.java | 25 ++++++++++++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java index f573f40547e..138cc91b59c 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java @@ -1646,9 +1646,7 @@ private String avgExpr(String c) { return c + "=" + c + "_Count == 0 ? null : " + c + "_Sum.divide(java.math.BigDecimal.valueOf(" + c + "_Count), java.math.BigDecimal.ROUND_HALF_UP)"; } - // I would expect us to return a null for an average of nothing, but we instead return a NaN - // return c + "=" + c + "_Count == 0 ? null : ((double)" + c + "_Sum / (double)" + c + "_Count)"; - return c + "=((double)(" + c + "_Count == 0 ? 0.0 : " + c + "_Sum) / (double)" + c + "_Count)"; + return c + "=" + c + "_Count == 0 ? null : ((double)" + c + "_Sum / (double)" + c + "_Count)"; } @Test @@ -1886,9 +1884,9 @@ public void testAvgInfinities() { TableTools.show(result.meta()); TestCase.assertEquals(1, result.size()); double avg = result.getColumnSource("IntCol", double.class).getDouble(result.getRowSet().firstRowKey()); - TestCase.assertEquals(Double.NaN, avg); + TestCase.assertEquals(NULL_DOUBLE, avg); double avgF = result.getColumnSource("FloatCol", double.class).getDouble(result.getRowSet().firstRowKey()); - TestCase.assertEquals(Double.NaN, avgF); + TestCase.assertEquals(NULL_DOUBLE, avgF); final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); updateGraph.runWithinUnitTestCycle(() -> { @@ -1965,9 +1963,9 @@ public void testVarInfinities() { TableTools.show(result.meta()); TestCase.assertEquals(1, result.size()); double var = result.getColumnSource("IntCol", double.class).getDouble(result.getRowSet().firstRowKey()); - TestCase.assertEquals(Double.NaN, var); + TestCase.assertEquals(NULL_DOUBLE, var); double varF = result.getColumnSource("FloatCol", double.class).getDouble(result.getRowSet().firstRowKey()); - TestCase.assertEquals(Double.NaN, varF); + TestCase.assertEquals(NULL_DOUBLE, varF); final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); updateGraph.runWithinUnitTestCycle(() -> { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java index e25376a5579..3e527e298b2 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java @@ -14,6 +14,8 @@ import java.math.BigDecimal; import java.math.BigInteger; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; + public class QueryTableAggregationTestFormulaStaticMethods { public static ByteVector abs(ByteVector values) { final byte[] result = new byte[values.intSize()]; @@ -357,19 +359,34 @@ public static double varChar(CharVector values) { double sum = 0; double sum2 = 0; int count = 0; + int nullCount = 0; for (int ii = 0; ii < values.size(); ++ii) { final char c = values.get(ii); if (c != QueryConstants.NULL_CHAR) { - sum += c; + sum += c; sum2 += c * c; - count++; + count++; + } else { + nullCount++; } } + if (nullCount == values.size()) { + return NULL_DOUBLE; + } return (sum2 - sum * sum / count) / (count - 1); } + public static double stdChar(CharVector values) { + if (values == null) { + return NULL_DOUBLE; + } + + final double v = varChar(values); + return v == NULL_DOUBLE ? NULL_DOUBLE : Math.sqrt(v); + } + public static BigDecimal varBigInt(ObjectVector values) { BigInteger sum = BigInteger.ZERO; BigInteger sum2 = BigInteger.ZERO; @@ -614,11 +631,13 @@ static String varFunction(String col) { static String stdFunction(String col) { switch (col) { + case "charCol": + return QueryTableAggregationTestFormulaStaticMethods.class.getCanonicalName() + ".stdChar(" + col + ")"; case "bigI": case "bigD": return "io.deephaven.util.BigDecimalUtils.sqrt(" + varFunction(col) + ", 10)"; default: - return "Math.sqrt(" + varFunction(col) + ")"; + return "std(" + col + ")"; } } From ff6f010c8d6bc72ec850b6c552eceed0ccce19aa Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 13 Jun 2024 09:24:05 -0700 Subject: [PATCH 06/18] Spotless --- .../impl/QueryTableAggregationTestFormulaStaticMethods.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java index 3e527e298b2..c0633350819 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTestFormulaStaticMethods.java @@ -364,9 +364,9 @@ public static double varChar(CharVector values) { for (int ii = 0; ii < values.size(); ++ii) { final char c = values.get(ii); if (c != QueryConstants.NULL_CHAR) { - sum += c; + sum += c; sum2 += c * c; - count++; + count++; } else { nullCount++; } From 9ff3cf6530e4aad09ed2fbafe065285ce719eb1e Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 13 Jun 2024 13:19:54 -0700 Subject: [PATCH 07/18] Tiny change for correctness. --- .../engine/table/impl/by/FloatChunkedReAvgOperator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java index 3c08ce4a8fa..4d3258a8d4f 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedReAvgOperator.java @@ -127,12 +127,12 @@ private boolean updateResult(long destination, long nncValue, long nanValue, lon double sumSumValue) { if (nanValue > 0 || (picValue > 0 && nicValue > 0)) { return !Float.isNaN(resultColumn.getAndSetUnsafe(destination, Float.NaN)); - } else if (nncValue == 0) { - return resultColumn.getAndSetUnsafe(destination, NULL_FLOAT) != NULL_FLOAT; } else if (picValue > 0) { return resultColumn.getAndSetUnsafe(destination, Float.POSITIVE_INFINITY) != Float.POSITIVE_INFINITY; } else if (nicValue > 0) { return resultColumn.getAndSetUnsafe(destination, Float.NEGATIVE_INFINITY) != Float.NEGATIVE_INFINITY; + } else if (nncValue == 0) { + return resultColumn.getAndSetUnsafe(destination, NULL_FLOAT) != NULL_FLOAT; } else { final float newValue = (float) (sumSumValue / nncValue); return resultColumn.getAndSetUnsafe(destination, newValue) != newValue; From 87532676fcb91e3631eb5a0819398e2a3398c8e4 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Mon, 17 Jun 2024 13:19:25 -0700 Subject: [PATCH 08/18] Addressed PR comments. --- engine/function/src/templates/Numeric.ftl | 134 ++++++++++++------ engine/function/src/templates/TestNumeric.ftl | 16 +++ .../table/impl/by/CharChunkedVarOperator.java | 11 +- .../impl/by/FloatChunkedVarOperator.java | 23 +-- 4 files changed, 130 insertions(+), 54 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 3fd052df50c..7154db671ec 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1268,38 +1268,63 @@ public class Numeric { if (n == 0) { return NULL_DOUBLE; - } else { - // NULL values sorted to beginning of the array. - ${pt.primitive}[] copy = sort(values.toArray()); + } + + ${pt.primitive}[] sorted = values.copyToArray(); + Arrays.sort(sorted); - // Determine if there are any NULL in the array. - int nullCount = 0; - for (int i = 0; i < n; i++) { - if (isNull(copy[i])) { + int nullStart = -1; + int nullCount = 0; + + <#if pt.valueType.isFloat > + for (int i = 0; i < n; i++) { + final ${pt.primitive} val = sorted[i]; + if (isNaN(val)) { + return Double.NaN; // Any NaN will pollute the result. + } + if (nullStart == -1) { + if (isNull(val)) { + nullStart = i; nullCount++; - } else { - break; } + } else if (isNull(val)) { + nullCount++; } - - if (nullCount == 0) { - // No NULL, so we can just compute the median and return. - if (n % 2 == 0) - return 0.5 * (copy[n / 2 - 1] + copy[n / 2]); - else return copy[n / 2]; - } else if (nullCount < n) { - // Some NULL, reduce the count and compute the median of the non-null values. - n -= nullCount; - if (n % 2 == 0) { - int index = n / 2; - return 0.5 * (copy[n / 2 - 1 + nullCount] + copy[n / 2 + nullCount]); + } + <#else> + for (int i = 0; i < n; i++) { + final ${pt.primitive} val = sorted[i]; + if (nullStart == -1) { + if (isNull(val)) { + nullStart = i; + nullCount++; } - else return copy[n / 2 + nullCount]; + } else if (isNull(val)) { + nullCount++; } else { - // All values are NULL. - return NULL_DOUBLE; + break; // no more NULL possible } } + + + if (nullCount == n) { + return NULL_DOUBLE; + } else if (nullCount > 0) { + if (nullStart > 0) { + // Move the pre-NULL data so we have a contiguous block of non-NULL values. + System.arraycopy(sorted, 0, sorted, nullCount, nullStart); + } + n -= nullCount; + if (n % 2 == 0) { + int index = n / 2; + return 0.5 * (sorted[n / 2 - 1 + nullCount] + sorted[n / 2 + nullCount]); + } + else return sorted[n / 2 + nullCount]; + } else { + if (n % 2 == 0) + return 0.5 * (sorted[n / 2 - 1] + sorted[n / 2]); + else return sorted[n / 2]; + } } /** @@ -1334,31 +1359,60 @@ public class Numeric { } int n = values.intSize("percentile"); - // NULL values sorted to beginning of the array. - ${pt.primitive}[] copy = sort(values.toArray()); - // Determine if there are any NULL in the array. + if (n == 0) { + return ${pt.null}; + } + + ${pt.primitive}[] sorted = values.copyToArray(); + Arrays.sort(sorted); + + int nullStart = -1; int nullCount = 0; + + <#if pt.valueType.isFloat > for (int i = 0; i < n; i++) { - if (isNull(copy[i])) { + final ${pt.primitive} val = sorted[i]; + if (isNaN(val)) { + return ${pt.boxed}.NaN; // Any NaN will pollute the result. + } + if (nullStart == -1) { + if (isNull(val)) { + nullStart = i; + nullCount++; + } + } else if (isNull(val)) { + nullCount++; + } + } + <#else> + for (int i = 0; i < n; i++) { + final ${pt.primitive} val = sorted[i]; + if (nullStart == -1) { + if (isNull(val)) { + nullStart = i; + nullCount++; + } + } else if (isNull(val)) { nullCount++; } else { - break; + break; // no more NULL possible } } + - if (nullCount == 0) { - // No NULL, so we can just compute the index and return. - int idx = (int) Math.round(percentile * (n - 1)); - return copy[idx]; - } else if (nullCount < n) { - // Some NULL, reduce the count and compute the median of the non-null values. - n -= nullCount; - int idx = (int) Math.round(percentile * (n - 1)); - return copy[idx + nullCount]; - } else { - // All values are NULL. + if (nullCount == n) { return ${pt.null}; + } else if (nullCount > 0) { + if (nullStart > 0) { + // Move the pre-NULL data so we have a contiguous block of non-NULL values. + System.arraycopy(sorted, 0, sorted, nullCount, nullStart); + } + int idx = (int) Math.round(percentile * (n - nullCount - 1)); + return sorted[idx + nullCount]; + } else { + int idx = (int) Math.round(percentile * (n - 1)); + return sorted[idx]; } } diff --git a/engine/function/src/templates/TestNumeric.ftl b/engine/function/src/templates/TestNumeric.ftl index 7950ce4e787..6cf4c113743 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -1150,6 +1150,12 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(3.0, median(new ${pt.boxed}[]{(${pt.primitive})4,(${pt.primitive})2,(${pt.primitive})3,${pt.null},${pt.null}})); assertEquals(3.5, median(new ${pt.boxed}[]{(${pt.primitive})4,(${pt.primitive})2,(${pt.primitive})3,(${pt.primitive})5,${pt.null},${pt.null}})); + <#if pt.valueType.isFloat > + assertEquals(Double.NaN, median(new ${pt.primitive}[]{4,2,3, ${pt.boxed}.NaN})); + assertEquals(3.0, median(new ${pt.primitive}[]{4,2,3, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + assertEquals(3.5, median(new ${pt.primitive}[]{4,2,3,5, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + + // check that functions can be resolved with varargs assertEquals(3.0, median((${pt.primitive})4, (${pt.primitive})2, (${pt.primitive})3)); } @@ -1188,6 +1194,16 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals((${pt.primitive})2, percentile(0.00, new ${pt.primitive}[]{4,2,3,${pt.null}})); assertEquals((${pt.primitive})3, percentile(0.50, new ${pt.primitive}[]{4,2,3,${pt.null},${pt.null}})); assertEquals((${pt.primitive})4, percentile(1.0, new ${pt.primitive}[]{4,2,3,${pt.null},${pt.null},${pt.null}})); + + <#if pt.valueType.isFloat > + assertEquals(${pt.boxed}.NaN, percentile(1.0, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.NaN})); + + assertEquals(${pt.boxed}.NEGATIVE_INFINITY, percentile(0.0, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + assertEquals((${pt.primitive})2, percentile(0.25, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + assertEquals((${pt.primitive})3, percentile(0.5, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + assertEquals((${pt.primitive})4, percentile(0.75, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + assertEquals(${pt.boxed}.POSITIVE_INFINITY, percentile(1.0, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.POSITIVE_INFINITY, ${pt.null}, ${pt.null}, ${pt.boxed}.NEGATIVE_INFINITY})); + } public void test${pt.boxed}Wsum() { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java index e77f33333b6..e007c79108a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java @@ -96,10 +96,13 @@ private boolean addChunk(CharChunk values, long destination, i final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) == 1) { - resultColumn.set(destination, Double.NaN); - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); + } else { + final long nonNullCount = nonNullCounter.getCountUnsafe(destination); + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else if (nonNullCount == 1) { + resultColumn.set(destination, Double.NaN); + } } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java index eafbdfc0bfa..273d8f7d2a0 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java @@ -3,6 +3,7 @@ // package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -99,7 +100,8 @@ private boolean addChunk(FloatChunk values, long destination, sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (forceNanResult || nonNullCount <= 1) { + Assert.neqZero(nonNullCount, "nonNullCount"); + if (forceNanResult || nonNullCount == 1) { resultColumn.set(destination, Double.NaN); } else { // If the sum or sumSquared has reached +/-Infinity, we are stuck with NaN forever. @@ -111,15 +113,20 @@ private boolean addChunk(FloatChunk values, long destination, resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; - } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) == 1)) { + } else if (forceNanResult) { resultColumn.set(destination, Double.NaN); return true; - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); - return true; } else { - return false; + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (totalNormalCount == 1) { + resultColumn.set(destination, Double.NaN); + return true; + } } + return false; } private static double computeVariance(long nonNullCount, double newSum, double newSum2) { @@ -175,10 +182,6 @@ private boolean removeChunk(FloatChunk values, long destinatio newSum = sumSource.getUnsafe(destination); newSum2 = sum2Source.getUnsafe(destination); } - if (totalNormalCount <= 1) { - resultColumn.set(destination, Double.NaN); - return true; - } // If the sum has reach +/-Infinity, we are stuck with NaN forever. if (Double.isInfinite(newSum) || Double.isInfinite(newSum2)) { From 4481f791ce0112c5d1f4795a3deee5aad866fdf0 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Mon, 17 Jun 2024 14:24:40 -0700 Subject: [PATCH 09/18] Addressed another round of PR comments. --- engine/function/src/templates/Numeric.ftl | 15 ++++++++---- .../table/impl/by/ByteChunkedVarOperator.java | 11 +++++---- .../impl/by/DoubleChunkedReAvgOperator.java | 4 ++-- .../impl/by/DoubleChunkedVarOperator.java | 23 +++++++++++-------- .../table/impl/by/IntChunkedVarOperator.java | 11 +++++---- .../table/impl/by/LongChunkedVarOperator.java | 11 +++++---- .../impl/by/ShortChunkedVarOperator.java | 11 +++++---- 7 files changed, 53 insertions(+), 33 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 7154db671ec..7d9d4e2d446 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1230,7 +1230,8 @@ public class Numeric { } /** - * Returns the median. + * Returns the median. {@code null} input values are ignored but {@code NaN} values will poison the computation, + * and {@code NaN} will be returned * * @param values values. * @return median. @@ -1240,7 +1241,8 @@ public class Numeric { } /** - * Returns the median. + * Returns the median. {@code null} input values are ignored but {@code NaN} values will poison the computation, + * and {@code NaN} will be returned * * @param values values. * @return median. @@ -1254,7 +1256,8 @@ public class Numeric { } /** - * Returns the median. + * Returns the median. {@code null} input values are ignored but {@code NaN} values will poison the computation, + * and {@code NaN} will be returned * * @param values values. * @return median. @@ -1328,7 +1331,8 @@ public class Numeric { } /** - * Returns the percentile. + * Returns the percentile. {@code null} input values are ignored but {@code NaN} values will poison the computation, + * and {@code NaN} will be returned * * @param percentile percentile to compute. * @param values values. @@ -1343,7 +1347,8 @@ public class Numeric { } /** - * Returns the percentile. + * Returns the percentile. {@code null} input values are ignored but {@code NaN} values will poison the computation, + * and {@code NaN} will be returned * * @param percentile percentile to compute. * @param values values. diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java index 8775db456d5..356b9d4461b 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java @@ -100,10 +100,13 @@ private boolean addChunk(ByteChunk values, long destination, i final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) == 1) { - resultColumn.set(destination, Double.NaN); - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); + } else { + final long nonNullCount = nonNullCounter.getCountUnsafe(destination); + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else if (nonNullCount == 1) { + resultColumn.set(destination, Double.NaN); + } } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java index ae311b3ebb9..a8610d282b2 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedReAvgOperator.java @@ -131,12 +131,12 @@ private boolean updateResult(long destination, long nncValue, long nanValue, lon double sumSumValue) { if (nanValue > 0 || (picValue > 0 && nicValue > 0)) { return !Double.isNaN(resultColumn.getAndSetUnsafe(destination, Double.NaN)); - } else if (nncValue == 0) { - return resultColumn.getAndSetUnsafe(destination, NULL_DOUBLE) != NULL_DOUBLE; } else if (picValue > 0) { return resultColumn.getAndSetUnsafe(destination, Double.POSITIVE_INFINITY) != Double.POSITIVE_INFINITY; } else if (nicValue > 0) { return resultColumn.getAndSetUnsafe(destination, Double.NEGATIVE_INFINITY) != Double.NEGATIVE_INFINITY; + } else if (nncValue == 0) { + return resultColumn.getAndSetUnsafe(destination, NULL_DOUBLE) != NULL_DOUBLE; } else { final double newValue = (double) (sumSumValue / nncValue); return resultColumn.getAndSetUnsafe(destination, newValue) != newValue; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java index 3b43885e66e..edcdf497e7b 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java @@ -7,6 +7,7 @@ // @formatter:off package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -103,7 +104,8 @@ private boolean addChunk(DoubleChunk values, long destination, sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (forceNanResult || nonNullCount <= 1) { + Assert.neqZero(nonNullCount, "nonNullCount"); + if (forceNanResult || nonNullCount == 1) { resultColumn.set(destination, Double.NaN); } else { // If the sum or sumSquared has reached +/-Infinity, we are stuck with NaN forever. @@ -115,15 +117,20 @@ private boolean addChunk(DoubleChunk values, long destination, resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; - } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) == 1)) { + } else if (forceNanResult) { resultColumn.set(destination, Double.NaN); return true; - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); - return true; } else { - return false; + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (totalNormalCount == 1) { + resultColumn.set(destination, Double.NaN); + return true; + } } + return false; } private static double computeVariance(long nonNullCount, double newSum, double newSum2) { @@ -179,10 +186,6 @@ private boolean removeChunk(DoubleChunk values, long destinati newSum = sumSource.getUnsafe(destination); newSum2 = sum2Source.getUnsafe(destination); } - if (totalNormalCount <= 1) { - resultColumn.set(destination, Double.NaN); - return true; - } // If the sum has reach +/-Infinity, we are stuck with NaN forever. if (Double.isInfinite(newSum) || Double.isInfinite(newSum2)) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java index eee1a6bf780..5f59c7c5e4c 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java @@ -100,10 +100,13 @@ private boolean addChunk(IntChunk values, long destination, in final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) == 1) { - resultColumn.set(destination, Double.NaN); - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); + } else { + final long nonNullCount = nonNullCounter.getCountUnsafe(destination); + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else if (nonNullCount == 1) { + resultColumn.set(destination, Double.NaN); + } } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java index 40b5cf66ba8..83339b88f0b 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java @@ -100,10 +100,13 @@ private boolean addChunk(LongChunk values, long destination, i final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) == 1) { - resultColumn.set(destination, Double.NaN); - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); + } else { + final long nonNullCount = nonNullCounter.getCountUnsafe(destination); + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else if (nonNullCount == 1) { + resultColumn.set(destination, Double.NaN); + } } return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java index 9189fdcbadf..9da834f9bdf 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java @@ -100,10 +100,13 @@ private boolean addChunk(ShortChunk values, long destination, final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } - } else if (nonNullCounter.getCountUnsafe(destination) == 1) { - resultColumn.set(destination, Double.NaN); - } else if (nonNullCounter.getCountUnsafe(destination) == 0) { - resultColumn.set(destination, NULL_DOUBLE); + } else { + final long nonNullCount = nonNullCounter.getCountUnsafe(destination); + if (nonNullCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + } else if (nonNullCount == 1) { + resultColumn.set(destination, Double.NaN); + } } return true; } From 94224884602b4af94631a5f06edd86ff0222a851 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Mon, 17 Jun 2024 15:59:02 -0700 Subject: [PATCH 10/18] More comments addressed. --- engine/function/src/templates/Numeric.ftl | 96 ++++++++++------------- 1 file changed, 42 insertions(+), 54 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 7d9d4e2d446..64f38ab45a5 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1276,58 +1276,54 @@ public class Numeric { ${pt.primitive}[] sorted = values.copyToArray(); Arrays.sort(sorted); + <#if pt.valueType.isFloat > + if (isNaN(sorted[sorted.length - 1])) { + return Double.NaN; // Any NaN will pollute the result and NaN always sorted to the end. + } + + int nullStart = -1; int nullCount = 0; - <#if pt.valueType.isFloat > - for (int i = 0; i < n; i++) { - final ${pt.primitive} val = sorted[i]; - if (isNaN(val)) { - return Double.NaN; // Any NaN will pollute the result. - } - if (nullStart == -1) { - if (isNull(val)) { - nullStart = i; - nullCount++; - } - } else if (isNull(val)) { - nullCount++; - } - } - <#else> for (int i = 0; i < n; i++) { final ${pt.primitive} val = sorted[i]; - if (nullStart == -1) { + if (val > ${pt.null}) { + break; // no more NULL possible + } else if (nullStart == -1) { if (isNull(val)) { nullStart = i; nullCount++; } } else if (isNull(val)) { nullCount++; - } else { - break; // no more NULL possible } } - if (nullCount == n) { return NULL_DOUBLE; } else if (nullCount > 0) { + n -= nullCount; + <#if pt.valueType.isFloat > if (nullStart > 0) { - // Move the pre-NULL data so we have a contiguous block of non-NULL values. - System.arraycopy(sorted, 0, sorted, nullCount, nullStart); + final int medianIndex = n / 2; + if (n % 2 == 0) { + final int idx1 = (medianIndex - 1) >= nullStart ? (medianIndex - 1) + nullCount : (medianIndex - 1); + final int idx2 = medianIndex >= nullStart ? medianIndex + nullCount : medianIndex; + return 0.5 * (sorted[idx1] + sorted[idx2]); + } + final int adjustedIndex = medianIndex >= nullStart ? medianIndex + nullCount : medianIndex; + return sorted[adjustedIndex]; } - n -= nullCount; + if (n % 2 == 0) { int index = n / 2; return 0.5 * (sorted[n / 2 - 1 + nullCount] + sorted[n / 2 + nullCount]); } - else return sorted[n / 2 + nullCount]; - } else { - if (n % 2 == 0) - return 0.5 * (sorted[n / 2 - 1] + sorted[n / 2]); - else return sorted[n / 2]; + return sorted[n / 2 + nullCount]; } + if (n % 2 == 0) + return 0.5 * (sorted[n / 2 - 1] + sorted[n / 2]); + return sorted[n / 2]; } /** @@ -1372,53 +1368,45 @@ public class Numeric { ${pt.primitive}[] sorted = values.copyToArray(); Arrays.sort(sorted); + <#if pt.valueType.isFloat > + if (isNaN(sorted[sorted.length - 1])) { + return ${pt.boxed}.NaN; // Any NaN will pollute the result and NaN always sorted to the end. + } + + int nullStart = -1; int nullCount = 0; - <#if pt.valueType.isFloat > - for (int i = 0; i < n; i++) { - final ${pt.primitive} val = sorted[i]; - if (isNaN(val)) { - return ${pt.boxed}.NaN; // Any NaN will pollute the result. - } - if (nullStart == -1) { - if (isNull(val)) { - nullStart = i; - nullCount++; - } - } else if (isNull(val)) { - nullCount++; - } - } - <#else> for (int i = 0; i < n; i++) { final ${pt.primitive} val = sorted[i]; - if (nullStart == -1) { + if (val > ${pt.null}) { + break; // no more NULL possible + } else if (nullStart == -1) { if (isNull(val)) { nullStart = i; nullCount++; } } else if (isNull(val)) { nullCount++; - } else { - break; // no more NULL possible } } - if (nullCount == n) { return ${pt.null}; } else if (nullCount > 0) { + n -= nullCount; + <#if pt.valueType.isFloat > if (nullStart > 0) { - // Move the pre-NULL data so we have a contiguous block of non-NULL values. - System.arraycopy(sorted, 0, sorted, nullCount, nullStart); + int idx = (int) Math.round(percentile * (n - 1)); + final int adjustedIndex = idx >= nullStart ? idx + nullCount : idx; + return sorted[adjustedIndex]; } - int idx = (int) Math.round(percentile * (n - nullCount - 1)); - return sorted[idx + nullCount]; - } else { + int idx = (int) Math.round(percentile * (n - 1)); - return sorted[idx]; + return sorted[idx + nullCount]; } + int idx = (int) Math.round(percentile * (n - 1)); + return sorted[idx]; } From 5750546924641c64070353caac74a71aad8773b4 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Mon, 17 Jun 2024 18:32:07 -0700 Subject: [PATCH 11/18] Bug fix to pass OOB. --- .../table/impl/by/ByteChunkedVarOperator.java | 24 +++---- .../table/impl/by/CharChunkedVarOperator.java | 24 +++---- .../impl/by/DoubleChunkedVarOperator.java | 19 +++--- .../impl/by/FloatChunkedVarOperator.java | 19 +++--- .../table/impl/by/IntChunkedVarOperator.java | 24 +++---- .../table/impl/by/LongChunkedVarOperator.java | 24 +++---- .../impl/by/ShortChunkedVarOperator.java | 24 +++---- .../table/impl/QueryTableAggregationTest.java | 66 +++++++++---------- 8 files changed, 120 insertions(+), 104 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java index 356b9d4461b..d57e45941a9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java @@ -7,6 +7,7 @@ // @formatter:off package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -87,24 +88,25 @@ private boolean addChunk(ByteChunk values, long destination, i final double sum = SumByteChunk.sum2ByteChunk(values, chunkStart, chunkSize, chunkNonNull, sum2); if (chunkNonNull.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); final double newSum = plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } } else { - final long nonNullCount = nonNullCounter.getCountUnsafe(destination); - if (nonNullCount == 0) { + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } } @@ -120,12 +122,12 @@ private boolean removeChunk(ByteChunk values, long destination return false; } - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); final double newSum; final double newSum2; - if (nonNullCount == 0) { + if (totalNormalCount == 0) { newSum = newSum2 = 0; } else { newSum = plusDouble(sumSource.getUnsafe(destination), -sum); @@ -135,15 +137,15 @@ private boolean removeChunk(ByteChunk values, long destination sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount == 0) { + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); return true; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java index e007c79108a..baee0905939 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java @@ -3,6 +3,7 @@ // package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -83,24 +84,25 @@ private boolean addChunk(CharChunk values, long destination, i final double sum = SumCharChunk.sum2CharChunk(values, chunkStart, chunkSize, chunkNonNull, sum2); if (chunkNonNull.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); final double newSum = plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } } else { - final long nonNullCount = nonNullCounter.getCountUnsafe(destination); - if (nonNullCount == 0) { + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } } @@ -116,12 +118,12 @@ private boolean removeChunk(CharChunk values, long destination return false; } - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); final double newSum; final double newSum2; - if (nonNullCount == 0) { + if (totalNormalCount == 0) { newSum = newSum2 = 0; } else { newSum = plusDouble(sumSource.getUnsafe(destination), -sum); @@ -131,15 +133,15 @@ private boolean removeChunk(CharChunk values, long destination sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount == 0) { + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); return true; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java index edcdf497e7b..fc16bd322d6 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java @@ -97,15 +97,15 @@ private boolean addChunk(DoubleChunk values, long destination, final boolean forceNanResult = totalNegativeInfinities > 0 || totalPositiveInfinities > 0 || totalNanCount > 0; if (chunkNormalCount.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNormalCount.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNormalCount.get()); final double newSum = NullSafeAddition.plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = NullSafeAddition.plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - Assert.neqZero(nonNullCount, "nonNullCount"); - if (forceNanResult || nonNullCount == 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (forceNanResult || totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { // If the sum or sumSquared has reached +/-Infinity, we are stuck with NaN forever. @@ -113,7 +113,7 @@ private boolean addChunk(DoubleChunk values, long destination, resultColumn.set(destination, Double.NaN); return true; } - final double variance = computeVariance(nonNullCount, newSum, newSum2); + final double variance = computeVariance(totalNormalCount, newSum, newSum2); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; @@ -176,15 +176,17 @@ private boolean removeChunk(DoubleChunk values, long destinati } sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - } else if (totalNormalCount == 1 || forceNanResult) { + } else { + newSum = sumSource.getUnsafe(destination); + newSum2 = sum2Source.getUnsafe(destination); + } + + if (totalNormalCount == 1 || forceNanResult) { resultColumn.set(destination, Double.NaN); return true; } else if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else { - newSum = sumSource.getUnsafe(destination); - newSum2 = sum2Source.getUnsafe(destination); } // If the sum has reach +/-Infinity, we are stuck with NaN forever. @@ -196,6 +198,7 @@ private boolean removeChunk(DoubleChunk values, long destinati // Perform the calculation in a way that minimizes the impact of FP error. final double variance = computeVariance(totalNormalCount, newSum, newSum2); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java index 273d8f7d2a0..e5c0691f58b 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java @@ -93,15 +93,15 @@ private boolean addChunk(FloatChunk values, long destination, final boolean forceNanResult = totalNegativeInfinities > 0 || totalPositiveInfinities > 0 || totalNanCount > 0; if (chunkNormalCount.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNormalCount.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNormalCount.get()); final double newSum = NullSafeAddition.plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = NullSafeAddition.plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - Assert.neqZero(nonNullCount, "nonNullCount"); - if (forceNanResult || nonNullCount == 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (forceNanResult || totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { // If the sum or sumSquared has reached +/-Infinity, we are stuck with NaN forever. @@ -109,7 +109,7 @@ private boolean addChunk(FloatChunk values, long destination, resultColumn.set(destination, Double.NaN); return true; } - final double variance = computeVariance(nonNullCount, newSum, newSum2); + final double variance = computeVariance(totalNormalCount, newSum, newSum2); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; @@ -172,15 +172,17 @@ private boolean removeChunk(FloatChunk values, long destinatio } sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - } else if (totalNormalCount == 1 || forceNanResult) { + } else { + newSum = sumSource.getUnsafe(destination); + newSum2 = sum2Source.getUnsafe(destination); + } + + if (totalNormalCount == 1 || forceNanResult) { resultColumn.set(destination, Double.NaN); return true; } else if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else { - newSum = sumSource.getUnsafe(destination); - newSum2 = sum2Source.getUnsafe(destination); } // If the sum has reach +/-Infinity, we are stuck with NaN forever. @@ -192,6 +194,7 @@ private boolean removeChunk(FloatChunk values, long destinatio // Perform the calculation in a way that minimizes the impact of FP error. final double variance = computeVariance(totalNormalCount, newSum, newSum2); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java index 5f59c7c5e4c..af090a16201 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java @@ -7,6 +7,7 @@ // @formatter:off package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -87,24 +88,25 @@ private boolean addChunk(IntChunk values, long destination, in final double sum = SumIntChunk.sum2IntChunk(values, chunkStart, chunkSize, chunkNonNull, sum2); if (chunkNonNull.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); final double newSum = plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } } else { - final long nonNullCount = nonNullCounter.getCountUnsafe(destination); - if (nonNullCount == 0) { + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } } @@ -120,12 +122,12 @@ private boolean removeChunk(IntChunk values, long destination, return false; } - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); final double newSum; final double newSum2; - if (nonNullCount == 0) { + if (totalNormalCount == 0) { newSum = newSum2 = 0; } else { newSum = plusDouble(sumSource.getUnsafe(destination), -sum); @@ -135,15 +137,15 @@ private boolean removeChunk(IntChunk values, long destination, sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount == 0) { + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); return true; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java index 83339b88f0b..c7fe18af606 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java @@ -7,6 +7,7 @@ // @formatter:off package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -87,24 +88,25 @@ private boolean addChunk(LongChunk values, long destination, i final double sum = SumLongChunk.sum2LongChunk(values, chunkStart, chunkSize, chunkNonNull, sum2); if (chunkNonNull.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); final double newSum = plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } } else { - final long nonNullCount = nonNullCounter.getCountUnsafe(destination); - if (nonNullCount == 0) { + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } } @@ -120,12 +122,12 @@ private boolean removeChunk(LongChunk values, long destination return false; } - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); final double newSum; final double newSum2; - if (nonNullCount == 0) { + if (totalNormalCount == 0) { newSum = newSum2 = 0; } else { newSum = plusDouble(sumSource.getUnsafe(destination), -sum); @@ -135,15 +137,15 @@ private boolean removeChunk(LongChunk values, long destination sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount == 0) { + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); return true; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java index 9da834f9bdf..36a3aea1cbd 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java @@ -7,6 +7,7 @@ // @formatter:off package io.deephaven.engine.table.impl.by; +import io.deephaven.base.verify.Assert; import io.deephaven.chunk.attributes.ChunkLengths; import io.deephaven.chunk.attributes.ChunkPositions; import io.deephaven.chunk.attributes.Values; @@ -87,24 +88,25 @@ private boolean addChunk(ShortChunk values, long destination, final double sum = SumShortChunk.sum2ShortChunk(values, chunkStart, chunkSize, chunkNonNull, sum2); if (chunkNonNull.get() > 0) { - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, chunkNonNull.get()); final double newSum = plusDouble(sumSource.getUnsafe(destination), sum); final double newSum2 = plusDouble(sum2Source.getUnsafe(destination), sum2.doubleValue()); sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount <= 1) { + Assert.neqZero(totalNormalCount, "totalNormalCount"); + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } } else { - final long nonNullCount = nonNullCounter.getCountUnsafe(destination); - if (nonNullCount == 0) { + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); } } @@ -120,12 +122,12 @@ private boolean removeChunk(ShortChunk values, long destinatio return false; } - final long nonNullCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); + final long totalNormalCount = nonNullCounter.addNonNullUnsafe(destination, -chunkNonNull.get()); final double newSum; final double newSum2; - if (nonNullCount == 0) { + if (totalNormalCount == 0) { newSum = newSum2 = 0; } else { newSum = plusDouble(sumSource.getUnsafe(destination), -sum); @@ -135,15 +137,15 @@ private boolean removeChunk(ShortChunk values, long destinatio sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - if (nonNullCount == 0) { + if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (nonNullCount == 1) { + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - (newSum * newSum / nonNullCount)) / (nonNullCount - 1); + final double variance = (newSum2 - (newSum * newSum / totalNormalCount)) / (totalNormalCount - 1); resultColumn.set(destination, std ? Math.sqrt(variance) : variance); return true; diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java index 138cc91b59c..828b2d6aab1 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java @@ -2120,39 +2120,39 @@ public Table e() { return queryTable.sort("Sym").stdBy("Sym"); } }, - new EvalNugget() { - public Table e() { - return queryTable.sort("Sym").varBy("Sym"); - } - }, - new EvalNugget() { - public Table e() { - return queryTable.dropColumns("Sym").stdBy(); - } - }, - new EvalNugget() { - public Table e() { - return queryTable.dropColumns("Sym").varBy(); - } - }, - new TableComparator(doubleComparisonVar, trueForSyms) { - @Override - public void show() { - System.out.println("Big vs Double (var):"); - TableTools.showWithRowSet(bigVsDoubleVar); - System.out.println("Double Comparison (var)"); - TableTools.showWithRowSet(doubleComparisonVar); - } - }, - new TableComparator(doubleComparisonStd, trueForSyms) { - @Override - public void show() { - System.out.println("Big vs Double (std):"); - TableTools.showWithRowSet(bigVsDoubleStd); - System.out.println("Double Comparison (std)"); - TableTools.showWithRowSet(doubleComparisonStd); - } - } +// new EvalNugget() { +// public Table e() { +// return queryTable.sort("Sym").varBy("Sym"); +// } +// }, +// new EvalNugget() { +// public Table e() { +// return queryTable.dropColumns("Sym").stdBy(); +// } +// }, +// new EvalNugget() { +// public Table e() { +// return queryTable.dropColumns("Sym").varBy(); +// } +// }, +// new TableComparator(doubleComparisonVar, trueForSyms) { +// @Override +// public void show() { +// System.out.println("Big vs Double (var):"); +// TableTools.showWithRowSet(bigVsDoubleVar); +// System.out.println("Double Comparison (var)"); +// TableTools.showWithRowSet(doubleComparisonVar); +// } +// }, +// new TableComparator(doubleComparisonStd, trueForSyms) { +// @Override +// public void show() { +// System.out.println("Big vs Double (std):"); +// TableTools.showWithRowSet(bigVsDoubleStd); +// System.out.println("Double Comparison (std)"); +// TableTools.showWithRowSet(doubleComparisonStd); +// } +// } }; for (int i = 0; i < 50; i++) { RefreshingTableTestCase.simulateShiftAwareStep(size, random, queryTable, columnInfo, en); From 35c03d1951eba6b7d8cb2bf1be268e0b17e2acac Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Mon, 17 Jun 2024 18:42:48 -0700 Subject: [PATCH 12/18] Spotless --- .../impl/by/DoubleChunkedVarOperator.java | 2 +- .../impl/by/FloatChunkedVarOperator.java | 2 +- .../table/impl/QueryTableAggregationTest.java | 66 +++++++++---------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java index fc16bd322d6..68d447dbad2 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java @@ -176,7 +176,7 @@ private boolean removeChunk(DoubleChunk values, long destinati } sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - } else { + } else { newSum = sumSource.getUnsafe(destination); newSum2 = sum2Source.getUnsafe(destination); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java index e5c0691f58b..b58b274a162 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java @@ -172,7 +172,7 @@ private boolean removeChunk(FloatChunk values, long destinatio } sumSource.set(destination, newSum); sum2Source.set(destination, newSum2); - } else { + } else { newSum = sumSource.getUnsafe(destination); newSum2 = sum2Source.getUnsafe(destination); } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java index 828b2d6aab1..8eed148e92c 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java @@ -2120,39 +2120,39 @@ public Table e() { return queryTable.sort("Sym").stdBy("Sym"); } }, -// new EvalNugget() { -// public Table e() { -// return queryTable.sort("Sym").varBy("Sym"); -// } -// }, -// new EvalNugget() { -// public Table e() { -// return queryTable.dropColumns("Sym").stdBy(); -// } -// }, -// new EvalNugget() { -// public Table e() { -// return queryTable.dropColumns("Sym").varBy(); -// } -// }, -// new TableComparator(doubleComparisonVar, trueForSyms) { -// @Override -// public void show() { -// System.out.println("Big vs Double (var):"); -// TableTools.showWithRowSet(bigVsDoubleVar); -// System.out.println("Double Comparison (var)"); -// TableTools.showWithRowSet(doubleComparisonVar); -// } -// }, -// new TableComparator(doubleComparisonStd, trueForSyms) { -// @Override -// public void show() { -// System.out.println("Big vs Double (std):"); -// TableTools.showWithRowSet(bigVsDoubleStd); -// System.out.println("Double Comparison (std)"); -// TableTools.showWithRowSet(doubleComparisonStd); -// } -// } + // new EvalNugget() { + // public Table e() { + // return queryTable.sort("Sym").varBy("Sym"); + // } + // }, + // new EvalNugget() { + // public Table e() { + // return queryTable.dropColumns("Sym").stdBy(); + // } + // }, + // new EvalNugget() { + // public Table e() { + // return queryTable.dropColumns("Sym").varBy(); + // } + // }, + // new TableComparator(doubleComparisonVar, trueForSyms) { + // @Override + // public void show() { + // System.out.println("Big vs Double (var):"); + // TableTools.showWithRowSet(bigVsDoubleVar); + // System.out.println("Double Comparison (var)"); + // TableTools.showWithRowSet(doubleComparisonVar); + // } + // }, + // new TableComparator(doubleComparisonStd, trueForSyms) { + // @Override + // public void show() { + // System.out.println("Big vs Double (std):"); + // TableTools.showWithRowSet(bigVsDoubleStd); + // System.out.println("Double Comparison (std)"); + // TableTools.showWithRowSet(doubleComparisonStd); + // } + // } }; for (int i = 0; i < 50; i++) { RefreshingTableTestCase.simulateShiftAwareStep(size, random, queryTable, columnInfo, en); From 0f150ac9f257b207e27727c9a18d5ffe8a364046 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Mon, 17 Jun 2024 18:49:10 -0700 Subject: [PATCH 13/18] Undo test commenting. --- .../table/impl/QueryTableAggregationTest.java | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java index 8eed148e92c..138cc91b59c 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java @@ -2120,39 +2120,39 @@ public Table e() { return queryTable.sort("Sym").stdBy("Sym"); } }, - // new EvalNugget() { - // public Table e() { - // return queryTable.sort("Sym").varBy("Sym"); - // } - // }, - // new EvalNugget() { - // public Table e() { - // return queryTable.dropColumns("Sym").stdBy(); - // } - // }, - // new EvalNugget() { - // public Table e() { - // return queryTable.dropColumns("Sym").varBy(); - // } - // }, - // new TableComparator(doubleComparisonVar, trueForSyms) { - // @Override - // public void show() { - // System.out.println("Big vs Double (var):"); - // TableTools.showWithRowSet(bigVsDoubleVar); - // System.out.println("Double Comparison (var)"); - // TableTools.showWithRowSet(doubleComparisonVar); - // } - // }, - // new TableComparator(doubleComparisonStd, trueForSyms) { - // @Override - // public void show() { - // System.out.println("Big vs Double (std):"); - // TableTools.showWithRowSet(bigVsDoubleStd); - // System.out.println("Double Comparison (std)"); - // TableTools.showWithRowSet(doubleComparisonStd); - // } - // } + new EvalNugget() { + public Table e() { + return queryTable.sort("Sym").varBy("Sym"); + } + }, + new EvalNugget() { + public Table e() { + return queryTable.dropColumns("Sym").stdBy(); + } + }, + new EvalNugget() { + public Table e() { + return queryTable.dropColumns("Sym").varBy(); + } + }, + new TableComparator(doubleComparisonVar, trueForSyms) { + @Override + public void show() { + System.out.println("Big vs Double (var):"); + TableTools.showWithRowSet(bigVsDoubleVar); + System.out.println("Double Comparison (var)"); + TableTools.showWithRowSet(doubleComparisonVar); + } + }, + new TableComparator(doubleComparisonStd, trueForSyms) { + @Override + public void show() { + System.out.println("Big vs Double (std):"); + TableTools.showWithRowSet(bigVsDoubleStd); + System.out.println("Double Comparison (std)"); + TableTools.showWithRowSet(doubleComparisonStd); + } + } }; for (int i = 0; i < 50; i++) { RefreshingTableTestCase.simulateShiftAwareStep(size, random, queryTable, columnInfo, en); From 4e01d9dc7a3e592948fcbf94ff6753c4bafd0e46 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 18 Jun 2024 14:04:43 -0700 Subject: [PATCH 14/18] Better testing and coverage, fix to wvar() and misc cleanup. --- engine/function/src/templates/Numeric.ftl | 131 ++++++++++++------ engine/function/src/templates/TestNumeric.ftl | 81 ++++++++--- .../table/impl/by/ByteChunkedVarOperator.java | 3 +- .../table/impl/by/CharChunkedVarOperator.java | 3 +- .../impl/by/DoubleChunkedVarOperator.java | 20 +-- .../impl/by/FloatChunkedVarOperator.java | 20 +-- .../table/impl/by/IntChunkedVarOperator.java | 3 +- .../table/impl/by/LongChunkedVarOperator.java | 3 +- .../impl/by/ShortChunkedVarOperator.java | 3 +- 9 files changed, 177 insertions(+), 90 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 64f38ab45a5..40079299290 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -370,9 +370,11 @@ public class Numeric { try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(c)) { return Double.NaN; } + if (!isNull(c)) { sum += c; count++; @@ -431,12 +433,14 @@ public class Numeric { try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(c)) { return Double.NaN; } if (isInf(c)) { return Double.POSITIVE_INFINITY; } + if (!isNull(c)) { sum += Math.abs(c); count++; @@ -504,9 +508,11 @@ public class Numeric { try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(c) || isInf(c)) { return Double.NaN; } + if (!isNull(c)) { sum += (double)c; sum2 += (double)c * (double)c; @@ -619,6 +625,7 @@ public class Numeric { double count = 0; double count2 = 0; long nullCount = 0; + long valueCount = 0; try ( final ${pt.vectorIterator} vi = values.iterator(); @@ -627,17 +634,22 @@ public class Numeric { while (vi.hasNext()) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); final ${pt2.primitive} w = wi.${pt2.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(c) || isInf(c)) { return Double.NaN; } + + <#if pt2.valueType.isFloat > if (isNaN(w) || isInf(w)) { return Double.NaN; } + if (!isNull(c) && !isNull(w)) { sum += w * c; sum2 += w * c * c; count += w; count2 += w * w; + valueCount++; } else { nullCount++; } @@ -649,7 +661,7 @@ public class Numeric { } // Return NaN if overflow or too few values to compute variance. - if (count <= 1 || Double.isInfinite(sum) || Double.isInfinite(sum2)) { + if (valueCount <= 1 || Double.isInfinite(sum) || Double.isInfinite(sum2)) { return Double.NaN; } @@ -921,7 +933,7 @@ public class Numeric { } } - return s == NULL_DOUBLE ? NULL_DOUBLE : s * Math.sqrt(sumw2/sumw/sumw); + return s * Math.sqrt(sumw2/sumw/sumw); } @@ -1280,52 +1292,64 @@ public class Numeric { if (isNaN(sorted[sorted.length - 1])) { return Double.NaN; // Any NaN will pollute the result and NaN always sorted to the end. } - int nullStart = -1; int nullCount = 0; - for (int i = 0; i < n; i++) { final ${pt.primitive} val = sorted[i]; if (val > ${pt.null}) { break; // no more NULL possible - } else if (nullStart == -1) { - if (isNull(val)) { + } + if (isNull(val)) { + nullCount++; + if (nullStart == -1) { nullStart = i; - nullCount++; } - } else if (isNull(val)) { - nullCount++; } } + <#else> + int nullCount = 0; + for (int i = 0; i < n && isNull(sorted[i]); i++) { + nullCount++; + <#if pt.valueType.isFloat > + if (nullStart == -1) { + nullStart = i; + } + + } + if (nullCount == n) { return NULL_DOUBLE; - } else if (nullCount > 0) { + } + if (nullCount > 0) { n -= nullCount; + final int medianIndex = n / 2; + if (n % 2 == 0) { <#if pt.valueType.isFloat > - if (nullStart > 0) { - final int medianIndex = n / 2; - if (n % 2 == 0) { - final int idx1 = (medianIndex - 1) >= nullStart ? (medianIndex - 1) + nullCount : (medianIndex - 1); - final int idx2 = medianIndex >= nullStart ? medianIndex + nullCount : medianIndex; - return 0.5 * (sorted[idx1] + sorted[idx2]); - } - final int adjustedIndex = medianIndex >= nullStart ? medianIndex + nullCount : medianIndex; - return sorted[adjustedIndex]; - } + final int idx1 = (medianIndex - 1) < nullStart ? (medianIndex - 1) : (medianIndex - 1) + nullCount; + final int idx2 = medianIndex < nullStart ? medianIndex : medianIndex + nullCount; + <#else> + final int idx1 = (medianIndex - 1) + nullCount; + final int idx2 = medianIndex + nullCount; - if (n % 2 == 0) { - int index = n / 2; - return 0.5 * (sorted[n / 2 - 1 + nullCount] + sorted[n / 2 + nullCount]); + return 0.5 * (sorted[idx1] + sorted[idx2]); } - return sorted[n / 2 + nullCount]; + <#if pt.valueType.isFloat > + final int adjustedIndex = medianIndex < nullStart ? medianIndex : medianIndex + nullCount; + <#else> + final int adjustedIndex = medianIndex + nullCount; + + return sorted[adjustedIndex]; } - if (n % 2 == 0) - return 0.5 * (sorted[n / 2 - 1] + sorted[n / 2]); - return sorted[n / 2]; + final int medianIndex = n / 2; + if (n % 2 == 0) { + return 0.5 * (sorted[medianIndex - 1] + sorted[medianIndex]); + } + return sorted[medianIndex]; } + /** * Returns the percentile. {@code null} input values are ignored but {@code NaN} values will poison the computation, * and {@code NaN} will be returned @@ -1361,10 +1385,6 @@ public class Numeric { int n = values.intSize("percentile"); - if (n == 0) { - return ${pt.null}; - } - ${pt.primitive}[] sorted = values.copyToArray(); Arrays.sort(sorted); @@ -1372,38 +1392,45 @@ public class Numeric { if (isNaN(sorted[sorted.length - 1])) { return ${pt.boxed}.NaN; // Any NaN will pollute the result and NaN always sorted to the end. } - - int nullStart = -1; int nullCount = 0; - for (int i = 0; i < n; i++) { final ${pt.primitive} val = sorted[i]; if (val > ${pt.null}) { break; // no more NULL possible - } else if (nullStart == -1) { - if (isNull(val)) { + } + if (isNull(val)) { + nullCount++; + if (nullStart == -1) { nullStart = i; - nullCount++; } - } else if (isNull(val)) { - nullCount++; } } + <#else> + int nullCount = 0; + for (int i = 0; i < n && isNull(sorted[i]); i++) { + nullCount++; + <#if pt.valueType.isFloat > + if (nullStart == -1) { + nullStart = i; + } + + } + if (nullCount == n) { return ${pt.null}; - } else if (nullCount > 0) { + } + if (nullCount > 0) { n -= nullCount; <#if pt.valueType.isFloat > - if (nullStart > 0) { - int idx = (int) Math.round(percentile * (n - 1)); - final int adjustedIndex = idx >= nullStart ? idx + nullCount : idx; - return sorted[adjustedIndex]; - } - + final int idx = (int) Math.round(percentile * (n - 1)); + final int adjustedIndex = idx >= nullStart ? idx + nullCount : idx; + return sorted[adjustedIndex]; + <#else> int idx = (int) Math.round(percentile * (n - 1)); return sorted[idx + nullCount]; + } int idx = (int) Math.round(percentile * (n - 1)); return sorted[idx]; @@ -1487,12 +1514,16 @@ public class Numeric { while (v0i.hasNext()) { final ${pt.primitive} v0 = v0i.${pt.iteratorNext}(); final ${pt2.primitive} v1 = v1i.${pt2.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(v0) || isInf(v0)) { return Double.NaN; } + + <#if pt2.valueType.isFloat > if (isNaN(v1) || isInf(v1)) { return Double.NaN; } + if (!isNull(v0) && !isNull(v1)) { sum0 += v0; @@ -1588,12 +1619,16 @@ public class Numeric { while (v0i.hasNext()) { final ${pt.primitive} v0 = v0i.${pt.iteratorNext}(); final ${pt2.primitive} v1 = v1i.${pt2.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(v0) || isInf(v0)) { return Double.NaN; } + + <#if pt2.valueType.isFloat > if (isNaN(v1) || isInf(v1)) { return Double.NaN; } + if (!isNull(v0) && !isNull(v1)) { sum0 += v0; @@ -2904,12 +2939,16 @@ public class Numeric { while (vi.hasNext()) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); final ${pt2.primitive} w = wi.${pt2.iteratorNext}(); + <#if pt.valueType.isFloat > if (isNaN(c)) { return Double.NaN; } + + <#if pt2.valueType.isFloat > if (isNaN(w)) { return Double.NaN; } + if (!isNull(c) && !isNull(w)) { vsum += c * w; wsum += w; diff --git a/engine/function/src/templates/TestNumeric.ftl b/engine/function/src/templates/TestNumeric.ftl index 6cf4c113743..fc1dc658875 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -114,6 +114,11 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, avg(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); assertEquals(NULL_DOUBLE, avg(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + <#if pt.valueType.isFloat > + // verify the NaN short-circuit case + assertEquals(Double.NaN, avg(new ${pt.primitive}[]{40, ${pt.boxed}.NaN, 60})); + + // check that functions can be resolved with varargs assertEquals(45.0, avg((${pt.primitive})40, (${pt.primitive})50)); } @@ -321,6 +326,14 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, var(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); assertEquals(NULL_DOUBLE, var(new ${pt.boxed}[]{${pt.null}, ${pt.null}, ${pt.null}})); + // verify size==1 + assertEquals(Double.NaN, var(new ${pt.primitive}[]{40})); + + <#if pt.valueType.isFloat > + // verify the NaN short-circuit case + assertEquals(Double.NaN, var(new ${pt.primitive}[]{40, ${pt.boxed}.NaN, 60})); + + // check that functions can be resolved with varargs assertEquals(var, var((${pt.primitive})0, (${pt.primitive})40, ${pt.null}, (${pt.primitive})50, (${pt.primitive})60, (${pt.primitive}) -1, (${pt.primitive})0)); } @@ -534,9 +547,18 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, cov((${pt.vectorDirect})null, (${pt2.vectorDirect})null)); // verify the all-null cases return null - assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); - assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, cov(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + + <#if pt.valueType.isFloat > + // verify the NaN short-circuit case + assertEquals(Double.NaN, cov(new ${pt.primitive}[]{1, 2, ${pt.boxed}.NaN}, new ${pt2.primitive}[]{1, 2, 3})); + + <#if pt2.valueType.isFloat > + // verify the NaN short-circuit case + assertEquals(Double.NaN, cov(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{1, 2, ${pt2.boxed}.NaN})); + try { cov(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3,${pt.null},5}), new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5})); @@ -581,9 +603,18 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, cor((${pt.vectorDirect})null, (${pt2.vectorDirect})null)); // verify the all-null cases return null - assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); - assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, cor(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + + <#if pt.valueType.isFloat > + // verify the NaN short-circuit case + assertEquals(Double.NaN, cor(new ${pt.primitive}[]{1, 2, ${pt.boxed}.NaN}, new ${pt2.primitive}[]{1, 2, 3})); + + <#if pt2.valueType.isFloat > + // verify the NaN short-circuit case + assertEquals(Double.NaN, cov(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{1, 2, ${pt2.boxed}.NaN})); + try { cor(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3,${pt.null},5}), new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5})); @@ -1195,6 +1226,9 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals((${pt.primitive})3, percentile(0.50, new ${pt.primitive}[]{4,2,3,${pt.null},${pt.null}})); assertEquals((${pt.primitive})4, percentile(1.0, new ${pt.primitive}[]{4,2,3,${pt.null},${pt.null},${pt.null}})); + // verify the empty array case + assertEquals(${pt.null}, percentile(0.00, new ${pt.vectorDirect}(new ${pt.primitive}[]{}))); + <#if pt.valueType.isFloat > assertEquals(${pt.boxed}.NaN, percentile(1.0, new ${pt.primitive}[]{4,2,3, ${pt.boxed}.NaN})); @@ -1356,10 +1390,20 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(var(new ${pt.primitive}[]{1,2,3}), wvar(new ${pt.primitive}[]{1,2,3,${pt.null},5}, new ${pt2.primitive}[]{2,2,2,7,${pt2.null}})); // verify the all-null cases return null - assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); - assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wvar(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + // verify size==1 + assertEquals(Double.NaN, wvar(new ${pt.primitive}[]{1}, new ${pt2.primitive}[]{4})); + + <#if pt2.valueType.isFloat > + // verify NaN poisoning + assertEquals(Double.NaN, wvar(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3,${pt.null},5}), new ${pt2.vectorDirect}(new ${pt2.primitive}[]{4,5,6,Float.NaN,${pt2.null}}))); + + + // verify the zero-weight case returns null + assertEquals(Double.NaN, wvar(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{0, 0, 0})); } @@ -1391,10 +1435,9 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, wstd(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3}), (${pt2.vector}) null)); // verify the all-null cases return null - assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); - assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - + assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wstd(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); } @@ -1435,9 +1478,9 @@ public class TestNumeric extends BaseArrayTestCase { } // verify the all-null cases return null - assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); - assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wste(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); @@ -1466,9 +1509,9 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(NULL_DOUBLE, wtstat(new ${pt.vectorDirect}(new ${pt.primitive}[]{1,2,3}), (${pt2.vector}) null)); // verify the all-null cases return null - assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{1, 2, 3}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); - assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{1, 2, 3})); - assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}})); + assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); + assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{1, 2, 3})); + assertEquals(NULL_DOUBLE, wtstat(new ${pt.primitive}[]{${pt.null}, ${pt.null}, ${pt.null}}, new ${pt2.primitive}[]{${pt2.null}, ${pt2.null}, ${pt2.null}})); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java index d57e45941a9..a2f8f971ba9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ByteChunkedVarOperator.java @@ -140,7 +140,8 @@ private boolean removeChunk(ByteChunk values, long destination if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (totalNormalCount == 1) { + } + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java index baee0905939..51599f8a9fd 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/CharChunkedVarOperator.java @@ -136,7 +136,8 @@ private boolean removeChunk(CharChunk values, long destination if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (totalNormalCount == 1) { + } + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java index 68d447dbad2..42916045fc0 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/DoubleChunkedVarOperator.java @@ -117,18 +117,18 @@ private boolean addChunk(DoubleChunk values, long destination, resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; - } else if (forceNanResult) { + } + if (forceNanResult) { + resultColumn.set(destination, Double.NaN); + return true; + } + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; - } else { - final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); - if (totalNormalCount == 0) { - resultColumn.set(destination, NULL_DOUBLE); - return true; - } else if (totalNormalCount == 1) { - resultColumn.set(destination, Double.NaN); - return true; - } } return false; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java index b58b274a162..3d843d604f5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FloatChunkedVarOperator.java @@ -113,18 +113,18 @@ private boolean addChunk(FloatChunk values, long destination, resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; - } else if (forceNanResult) { + } + if (forceNanResult) { + resultColumn.set(destination, Double.NaN); + return true; + } + final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); + if (totalNormalCount == 0) { + resultColumn.set(destination, NULL_DOUBLE); + return true; + } else if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; - } else { - final long totalNormalCount = nonNullCounter.getCountUnsafe(destination); - if (totalNormalCount == 0) { - resultColumn.set(destination, NULL_DOUBLE); - return true; - } else if (totalNormalCount == 1) { - resultColumn.set(destination, Double.NaN); - return true; - } } return false; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java index af090a16201..482c9af19d5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/IntChunkedVarOperator.java @@ -140,7 +140,8 @@ private boolean removeChunk(IntChunk values, long destination, if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (totalNormalCount == 1) { + } + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java index c7fe18af606..d5d4929a7ab 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/LongChunkedVarOperator.java @@ -140,7 +140,8 @@ private boolean removeChunk(LongChunk values, long destination if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (totalNormalCount == 1) { + } + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java index 36a3aea1cbd..b8a77cfefc7 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ShortChunkedVarOperator.java @@ -140,7 +140,8 @@ private boolean removeChunk(ShortChunk values, long destinatio if (totalNormalCount == 0) { resultColumn.set(destination, NULL_DOUBLE); return true; - } else if (totalNormalCount == 1) { + } + if (totalNormalCount == 1) { resultColumn.set(destination, Double.NaN); return true; } From 2ea64184bf4e84ea0f5b4e875560239a99777991 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 18 Jun 2024 14:18:10 -0700 Subject: [PATCH 15/18] Test correction. --- engine/function/src/templates/TestNumeric.ftl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/function/src/templates/TestNumeric.ftl b/engine/function/src/templates/TestNumeric.ftl index fc1dc658875..5e51d5445cd 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -613,7 +613,7 @@ public class TestNumeric extends BaseArrayTestCase { <#if pt2.valueType.isFloat > // verify the NaN short-circuit case - assertEquals(Double.NaN, cov(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{1, 2, ${pt2.boxed}.NaN})); + assertEquals(Double.NaN, cor(new ${pt.primitive}[]{1, 2, 3}, new ${pt2.primitive}[]{1, 2, ${pt2.boxed}.NaN})); try { From e00e49a5a983c9eb0f3037be2564dee097736cca Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 18 Jun 2024 15:16:47 -0700 Subject: [PATCH 16/18] Removed dead code. --- engine/function/src/templates/Numeric.ftl | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 40079299290..efc990d8f16 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1311,11 +1311,6 @@ public class Numeric { int nullCount = 0; for (int i = 0; i < n && isNull(sorted[i]); i++) { nullCount++; - <#if pt.valueType.isFloat > - if (nullStart == -1) { - nullStart = i; - } - } @@ -1410,11 +1405,6 @@ public class Numeric { int nullCount = 0; for (int i = 0; i < n && isNull(sorted[i]); i++) { nullCount++; - <#if pt.valueType.isFloat > - if (nullStart == -1) { - nullStart = i; - } - } From 6bf8bb9485ad89e84daf8367e937b9567dc0eb32 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 18 Jun 2024 16:26:24 -0700 Subject: [PATCH 17/18] Update engine/function/src/templates/Numeric.ftl Accepting PR suggestions Co-authored-by: Ryan Caudy --- engine/function/src/templates/Numeric.ftl | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index efc990d8f16..e5692385f2a 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1344,7 +1344,6 @@ public class Numeric { return sorted[medianIndex]; } - /** * Returns the percentile. {@code null} input values are ignored but {@code NaN} values will poison the computation, * and {@code NaN} will be returned From cf1f22e973acf02d645f1013653fbd527ebd5bb9 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 18 Jun 2024 16:26:37 -0700 Subject: [PATCH 18/18] Update engine/function/src/templates/Numeric.ftl Accepting PR suggestions Co-authored-by: Ryan Caudy --- engine/function/src/templates/Numeric.ftl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index e5692385f2a..8710a195b83 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1414,7 +1414,7 @@ public class Numeric { n -= nullCount; <#if pt.valueType.isFloat > final int idx = (int) Math.round(percentile * (n - 1)); - final int adjustedIndex = idx >= nullStart ? idx + nullCount : idx; + final int adjustedIndex = idx < nullStart ? idx : idx + nullCount; return sorted[adjustedIndex]; <#else> int idx = (int) Math.round(percentile * (n - 1));