From 16c72881259a67494ce3a87bd0be4587522a2756 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 7 Nov 2023 13:29:25 -0800 Subject: [PATCH 1/9] Initial commit of emstd bug fix and test. --- .../emstd/BasePrimitiveEmStdOperator.java | 10 +- .../engine/table/impl/updateby/TestEmStd.java | 129 +++++++++++++++++- 2 files changed, 134 insertions(+), 5 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/emstd/BasePrimitiveEmStdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/emstd/BasePrimitiveEmStdOperator.java index 8df10acd3e3..4def5813764 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/emstd/BasePrimitiveEmStdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/emstd/BasePrimitiveEmStdOperator.java @@ -135,8 +135,14 @@ public void initializeCumulative(@NotNull final UpdateByOperator.Context updateC if (firstUnmodifiedKey != NULL_ROW_KEY) { ctx.curVal = outputSource.getDouble(firstUnmodifiedKey); ctx.curEma = emaSource.getDouble(firstUnmodifiedKey); - if (ctx.curVal == Double.NaN) { - ctx.curVariance = outputSource.getDouble(firstUnmodifiedKey); + if (ctx.curEma != NULL_DOUBLE + && !Double.isNaN(ctx.curEma) + && Double.isNaN(ctx.curVal)) { + // When we have a valid EMA, but the previous em_std value is NaN, we need to un-poison variance + // (by setting to 0.0) to allow the new variance to be computed properly. + // NOTE: this case can only exist between the first and second rows after initialization or a RESET + // caused by the {@link OperationControl control}. + ctx.curVariance = 0.0; } else if (ctx.curVal == NULL_DOUBLE) { ctx.curVariance = 0.0; } else { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestEmStd.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestEmStd.java index d50552d266d..3addd32b4c2 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestEmStd.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestEmStd.java @@ -92,7 +92,7 @@ public class TestEmStd extends BaseUpdateByTest { private BigDecimal[] convert(double[] input) { final BigDecimal[] output = new BigDecimal[input.length]; for (int ii = 0; ii < input.length; ii++) { - output[ii] = BigDecimal.valueOf(input[ii]); + output[ii] = Double.isNaN(input[ii]) ? BigDecimal.ZERO : BigDecimal.valueOf(input[ii]); } return output; } @@ -125,8 +125,9 @@ public void testVerifyVsPandas() { // generated from python pandas: // one_minus_alpha = 1.0 - math.exp(-1.0 / 10) // data["input"].ewm(alpha=one_minus_alpha, adjust=False).std(bias=True) + // NOTE: manually converted the first value to NaN to match Deephaven (from 0.0 in pandas) final double[] externalStd10 = new double[] { - 0.0, 1829.0154392759346, 1758.8960442013629, 2076.3748022882937, 2553.5576361856415, + Double.NaN, 1829.0154392759346, 1758.8960442013629, 2076.3748022882937, 2553.5576361856415, 2433.2971958076046, 2314.9508862196717, 2330.690949555806, 2296.9168425011217, 2291.9729309301392, 2312.255890924336, 2207.8214715377408, 2525.894583487406, 2531.3289503882397, 2413.990404017413, 2587.5388769864812, 2531.6706405602604, 2890.7631052604515, 2753.5309285046505, 2734.616641196746, @@ -151,8 +152,9 @@ public void testVerifyVsPandas() { // generated from python pandas: // one_minus_alpha = 1.0 - math.exp(-1.0 / 50) // data["input"].ewm(alpha=one_minus_alpha, adjust=False).std(bias=True) + // NOTE: manually converted the first value to NaN to match Deephaven (from 0.0 in pandas) final double[] externalStd50 = new double[] { - 0.0, 868.3667520669262, 861.6294154053817, 1065.6661593432248, 1365.1098539430147, + Double.NaN, 868.3667520669262, 861.6294154053817, 1065.6661593432248, 1365.1098539430147, 1369.6797998341608, 1367.2161208721454, 1448.8924859183394, 1507.5259703546187, 1498.3774979527218, 1496.6149141177552, 1482.433499352465, 1668.504887524693, 1742.0801310749232, 1728.8129551576424, 1752.4940689267764, 1740.522951793199, 1934.2905671973408, 1917.9800735018696, 1973.8732379079147, @@ -880,6 +882,127 @@ protected Table e() { } // endregion + // region Special Tests + @Test + public void testInitialEmptySingleRowIncrement() { + final CreateResult tickResult = createTestTable(0, true, false, true, 0x31313131, + new String[] {"charCol"}, new TestDataGenerator[] { + new CharGenerator('A', 'z', 0.1)}); + final CreateResult timeResult = createTestTable(0, true, false, true, 0x31313131, + new String[] {"ts", "charCol"}, new TestDataGenerator[] { + new SortedInstantGenerator( + parseInstant("2022-03-09T09:00:00.000 NY"), + parseInstant("2022-03-09T16:30:00.000 NY")), + new CharGenerator('A', 'z', 0.1)}); + + tickResult.t.setAttribute(Table.APPEND_ONLY_TABLE_ATTRIBUTE, Boolean.TRUE); + timeResult.t.setAttribute(Table.APPEND_ONLY_TABLE_ATTRIBUTE, Boolean.TRUE); + + final UpdateByControl control = UpdateByControl.builder().useRedirection(false).build(); + + final OperationControl skipControl = OperationControl.builder() + .onNullValue(BadDataBehavior.SKIP) + .onNanValue(BadDataBehavior.SKIP).build(); + + final OperationControl resetControl = OperationControl.builder() + .onNullValue(BadDataBehavior.RESET) + .onNanValue(BadDataBehavior.RESET).build(); + + final EvalNugget[] nuggets = new EvalNugget[] { + new EvalNugget() { + @Override + protected Table e() { + return tickResult.t.updateBy(control, + UpdateByOperation.EmStd(skipControl, 100, primitiveColumns), + "Sym"); + } + }, + new EvalNugget() { + @Override + protected Table e() { + return tickResult.t.updateBy(control, + UpdateByOperation.EmStd(skipControl, 100, primitiveColumns)); + } + }, + new EvalNugget() { + @Override + protected Table e() { + return tickResult.t.updateBy(control, + UpdateByOperation.EmStd(resetControl, 100, primitiveColumns), + "Sym"); + } + }, + new EvalNugget() { + @Override + protected Table e() { + return tickResult.t.updateBy(control, + UpdateByOperation.EmStd(resetControl, 100, primitiveColumns)); + } + } + }; + + final EvalNugget[] timeNuggets = new EvalNugget[] { + new EvalNugget() { + @Override + protected Table e() { + TableDefaults base = timeResult.t; + return base.updateBy(control, + UpdateByOperation.EmStd(skipControl, "ts", 10 * MINUTE, primitiveColumns), + "Sym"); + } + }, + new EvalNugget() { + @Override + protected Table e() { + TableDefaults base = timeResult.t; + return base.updateBy(control, + UpdateByOperation.EmStd(skipControl, "ts", 10 * MINUTE, primitiveColumns)); + } + }, + new EvalNugget() { + @Override + protected Table e() { + TableDefaults base = timeResult.t; + return base.updateBy(control, + UpdateByOperation.EmStd(resetControl, "ts", 10 * MINUTE, primitiveColumns), + "Sym"); + } + }, + new EvalNugget() { + @Override + protected Table e() { + TableDefaults base = timeResult.t; + return base.updateBy(control, + UpdateByOperation.EmStd(resetControl, "ts", 10 * MINUTE, primitiveColumns)); + } + } + + }; + + final Random billy = new Random(0xB177B177); + for (int ii = 0; ii < 500; ii++) { + if (ii % 100 == 0) { + // Force nulls into the table for all the primitive columns + + } else { + try { + final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); + updateGraph.runWithinUnitTestCycle(() -> { + generateAppends(1, billy, tickResult.t, tickResult.infos); + generateAppends(1, billy, timeResult.t, timeResult.infos); + }); + validate("Table", nuggets); + validate("Table", timeNuggets); + } catch (Throwable t) { + System.out.println("Crapped out on step " + ii); + throw t; + } + } + } + } + + // endregion + // region Manual Verification functions public static double[] compute_emstd_ticks(OperationControl control, long ticks, double[] values) { if (values == null) { From f871ff8706489439f40d537d1688ed3796d7aeb9 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Tue, 7 Nov 2023 20:44:46 -0800 Subject: [PATCH 2/9] Initial commit of NaN test tooling bug-fix. --- .../impl/by/DoubleChunkedVarOperator.java | 32 +++++++- .../impl/by/FloatChunkedVarOperator.java | 32 +++++++- .../rollingavg/ByteRollingAvgOperator.java | 6 +- .../rollingavg/CharRollingAvgOperator.java | 6 +- .../rollingavg/DoubleRollingAvgOperator.java | 6 +- .../rollingavg/FloatRollingAvgOperator.java | 6 +- .../rollingavg/IntRollingAvgOperator.java | 6 +- .../rollingavg/LongRollingAvgOperator.java | 6 +- .../rollingavg/ShortRollingAvgOperator.java | 6 +- .../DoubleRollingProductOperator.java | 15 +++- .../FloatRollingProductOperator.java | 18 +++-- .../io/deephaven/engine/util/TableDiff.java | 27 +++++-- .../impl/updateby/TestRollingProduct.java | 77 +++++++++++++++---- .../replicators/ReplicateUpdateBy.java | 10 ++- 14 files changed, 208 insertions(+), 45 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 711ba70ab0a..31f3b929c6d 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,8 +97,20 @@ private boolean addChunk(DoubleChunk values, long destination, if (forceNanResult || nonNullCount <= 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - newSum * newSum / nonNullCount) / (nonNullCount - 1); - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + // Perform the calculation in a way that minimizes the impact of FP error. + final double eps = Math.ulp(newSum2); + final double vs2bar = newSum * (newSum / nonNullCount); + final double delta = newSum2 - vs2bar; + final double rel_eps = delta / eps; + + // Return zero when the variance is leq the FP error. + final double variance = Math.abs(rel_eps) > 1.0 ? delta / (nonNullCount - 1) : 0.0; + if (variance < 0.0) { + // Negative variance can only be due to FP error. + resultColumn.set(destination, 0.0); + } else { + resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + } } return true; } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) <= 1)) { @@ -150,8 +162,20 @@ private boolean removeChunk(DoubleChunk values, long destinati resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - newSum * newSum / totalNormalCount) / (totalNormalCount - 1); - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + // Perform the calculation in a way that minimizes the impact of FP error. + final double eps = Math.ulp(newSum2); + final double vs2bar = newSum * (newSum / totalNormalCount); + final double delta = newSum2 - vs2bar; + final double rel_eps = delta / eps; + + // Return zero when the variance is leq the FP error. + final double variance = Math.abs(rel_eps) > 1.0 ? delta / (totalNormalCount - 1) : 0.0; + if (variance < 0.0) { + // Negative variance can only be due to FP error. + resultColumn.set(destination, 0.0); + } else { + 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 5e525b07e2a..43750b8fcd9 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 @@ -92,8 +92,20 @@ private boolean addChunk(FloatChunk values, long destination, if (forceNanResult || nonNullCount <= 1) { resultColumn.set(destination, Double.NaN); } else { - final double variance = (newSum2 - newSum * newSum / nonNullCount) / (nonNullCount - 1); - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + // Perform the calculation in a way that minimizes the impact of FP error. + final double eps = Math.ulp(newSum2); + final double vs2bar = newSum * (newSum / nonNullCount); + final double delta = newSum2 - vs2bar; + final double rel_eps = delta / eps; + + // Return zero when the variance is leq the FP error. + final double variance = Math.abs(rel_eps) > 1.0 ? delta / (nonNullCount - 1) : 0.0; + if (variance < 0.0) { + // Negative variance can only be due to FP error. + resultColumn.set(destination, 0.0); + } else { + resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + } } return true; } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) <= 1)) { @@ -145,8 +157,20 @@ private boolean removeChunk(FloatChunk values, long destinatio resultColumn.set(destination, Double.NaN); return true; } - final double variance = (newSum2 - newSum * newSum / totalNormalCount) / (totalNormalCount - 1); - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + // Perform the calculation in a way that minimizes the impact of FP error. + final double eps = Math.ulp(newSum2); + final double vs2bar = newSum * (newSum / totalNormalCount); + final double delta = newSum2 - vs2bar; + final double rel_eps = delta / eps; + + // Return zero when the variance is leq the FP error. + final double variance = Math.abs(rel_eps) > 1.0 ? delta / (totalNormalCount - 1) : 0.0; + if (variance < 0.0) { + // Negative variance can only be due to FP error. + resultColumn.set(destination, 0.0); + } else { + resultColumn.set(destination, std ? Math.sqrt(variance) : variance); + } return true; } 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 91d128411cb..dc354836853 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 @@ -88,7 +88,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = byteWindowValues.size() - nullCount; - outputValues.set(outIdx, curVal / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } 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 babdae78e29..6f5c7610c74 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,7 +82,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = charWindowValues.size() - nullCount; - outputValues.set(outIdx, curVal / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } 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 3748d4cd90c..3c3e405368d 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 @@ -84,7 +84,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = aggSum.size() - nullCount; - outputValues.set(outIdx, aggSum.evaluate() / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } 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 d8e7431c071..220f3df01e9 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 @@ -79,7 +79,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = aggSum.size() - nullCount; - outputValues.set(outIdx, aggSum.evaluate() / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } 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 57b8ef4ff5e..3fc6e88bf80 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 @@ -87,7 +87,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = intWindowValues.size() - nullCount; - outputValues.set(outIdx, curVal / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } 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 7fae62c0f5d..3fcc9ff5006 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 @@ -87,7 +87,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = longWindowValues.size() - nullCount; - outputValues.set(outIdx, curVal / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } 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 7c81f008a00..70920192b0c 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 @@ -87,7 +87,11 @@ public void writeToOutputChunk(int outIdx) { outputValues.set(outIdx, NULL_DOUBLE); } else { final int count = shortWindowValues.size() - nullCount; - outputValues.set(outIdx, curVal / (double)count); + if (count == 0) { + outputValues.set(outIdx, Double.NaN); + } else { + outputValues.set(outIdx, curVal / (double)count); + } } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java index 23385c171c0..9e5a297bfc8 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java @@ -1,6 +1,6 @@ /* * --------------------------------------------------------------------------------------------------------------------- - * AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY - for any changes edit CharRollingProductOperator and regenerate + * AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY - for any changes edit FloatRollingProductOperator and regenerate * --------------------------------------------------------------------------------------------------------------------- */ package io.deephaven.engine.table.impl.updateby.rollingproduct; @@ -30,6 +30,7 @@ protected class Context extends BaseDoubleUpdateByOperator.Context { protected AggregatingDoubleRingBuffer buffer; private int zeroCount; + private int nanCount; protected Context(final int affectedChunkSize, final int influencerChunkSize) { super(affectedChunkSize); @@ -48,6 +49,7 @@ protected Context(final int affectedChunkSize, final int influencerChunkSize) { }, true); zeroCount = 0; + nanCount = 0; } @Override @@ -76,6 +78,8 @@ public void push(int pos, int count) { buffer.addUnsafe(val); if (val == 0) { zeroCount++; + } else if (Double.isNaN(val)) { + nanCount++; } } } @@ -90,6 +94,8 @@ public void pop(int count) { if (val == NULL_DOUBLE) { nullCount--; + } else if (Double.isNaN(val)) { + --nanCount; } else if (val == 0) { --zeroCount; } @@ -101,7 +107,11 @@ public void writeToOutputChunk(int outIdx) { if (buffer.size() == nullCount) { outputValues.set(outIdx, NULL_DOUBLE); } else { - outputValues.set(outIdx, zeroCount > 0 ? 0.0 : buffer.evaluate()); + if (nanCount > 0) { + outputValues.set(outIdx, Double.NaN); + } else { + outputValues.set(outIdx, zeroCount > 0 ? 0.0 : buffer.evaluate()); + } } } @@ -109,6 +119,7 @@ public void writeToOutputChunk(int outIdx) { public void reset() { super.reset(); zeroCount = 0; + nanCount = 0; buffer.clear(); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java index 44e4d737d14..45285089c56 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java @@ -1,8 +1,3 @@ -/* - * --------------------------------------------------------------------------------------------------------------------- - * AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY - for any changes edit CharRollingProductOperator and regenerate - * --------------------------------------------------------------------------------------------------------------------- - */ package io.deephaven.engine.table.impl.updateby.rollingproduct; import io.deephaven.base.ringbuffer.AggregatingDoubleRingBuffer; @@ -30,6 +25,7 @@ protected class Context extends BaseDoubleUpdateByOperator.Context { protected AggregatingDoubleRingBuffer buffer; private int zeroCount; + private int nanCount; protected Context(final int affectedChunkSize, final int influencerChunkSize) { super(affectedChunkSize); @@ -48,6 +44,7 @@ protected Context(final int affectedChunkSize, final int influencerChunkSize) { }, true); zeroCount = 0; + nanCount = 0; } @Override @@ -76,6 +73,8 @@ public void push(int pos, int count) { buffer.addUnsafe(val); if (val == 0) { zeroCount++; + } else if (Double.isNaN(val)) { + nanCount++; } } } @@ -90,6 +89,8 @@ public void pop(int count) { if (val == NULL_DOUBLE) { nullCount--; + } else if (Double.isNaN(val)) { + --nanCount; } else if (val == 0) { --zeroCount; } @@ -101,7 +102,11 @@ public void writeToOutputChunk(int outIdx) { if (buffer.size() == nullCount) { outputValues.set(outIdx, NULL_DOUBLE); } else { - outputValues.set(outIdx, zeroCount > 0 ? 0.0 : buffer.evaluate()); + if (nanCount > 0) { + outputValues.set(outIdx, Double.NaN); + } else { + outputValues.set(outIdx, zeroCount > 0 ? 0.0 : buffer.evaluate()); + } } } @@ -109,6 +114,7 @@ public void writeToOutputChunk(int outIdx) { public void reset() { super.reset(); zeroCount = 0; + nanCount = 0; buffer.clear(); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java b/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java index aa1f18a75c1..cdc01bb862a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/TableDiff.java @@ -59,8 +59,8 @@ static Pair diffInternal(Table actualResult, Table expectedResult, } } - final Map actualNameToColumnSource = actualResult.getColumnSourceMap(); - final Map expectedNameToColumnSource = expectedResult.getColumnSourceMap(); + final Map> actualNameToColumnSource = actualResult.getColumnSourceMap(); + final Map> expectedNameToColumnSource = expectedResult.getColumnSourceMap(); final String[] actualColumnNames = actualResult.getDefinition().getColumnNames().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); final String[] expectedColumnNames = @@ -78,8 +78,8 @@ static Pair diffInternal(Table actualResult, Table expectedResult, final Set columnNamesForDiff = new LinkedHashSet<>(); for (int ci = 0; ci < expectedColumnNames.length; ci++) { final String expectedColumnName = expectedColumnNames[ci]; - final ColumnSource expectedColumnSource = expectedNameToColumnSource.get(expectedColumnName); - final ColumnSource actualColumnSource = actualNameToColumnSource.get(expectedColumnName); + final ColumnSource expectedColumnSource = expectedNameToColumnSource.get(expectedColumnName); + final ColumnSource actualColumnSource = actualNameToColumnSource.get(expectedColumnName); if (actualColumnSource == null) { issues.add("Expected column " + expectedColumnName + " not found"); } else { @@ -114,7 +114,7 @@ static Pair diffInternal(Table actualResult, Table expectedResult, try (final SafeCloseableList safeCloseables = new SafeCloseableList(); final SharedContext expectedSharedContext = SharedContext.makeSharedContext(); final SharedContext actualSharedContext = SharedContext.makeSharedContext(); - final WritableBooleanChunk equalValues = WritableBooleanChunk.makeWritableChunk(chunkSize)) { + final WritableBooleanChunk equalValues = WritableBooleanChunk.makeWritableChunk(chunkSize)) { final ColumnDiffContext[] columnContexts = columnNamesForDiff.stream() .map(name -> safeCloseables.add(new ColumnDiffContext(name, expectedNameToColumnSource.get(name), @@ -231,7 +231,7 @@ private ColumnDiffContext(@NotNull final String name, */ private long diffChunk(@NotNull final RowSequence expectedChunkOk, @NotNull final RowSequence actualChunkOk, - @NotNull final WritableBooleanChunk equalValues, + @NotNull final WritableBooleanChunk equalValues, @NotNull final Set itemsToSkip, @NotNull final List issues, long position) { @@ -267,6 +267,13 @@ private long diffChunk(@NotNull final RowSequence expectedChunkOk, } else if (chunkType == ChunkType.Float) { final float expectedValue = expectedValues.asFloatChunk().get(ii); final float actualValue = actualValues.asFloatChunk().get(ii); + if (Float.isNaN(expectedValue) || Float.isNaN(actualValue)) { + final String actualString = Float.isNaN(actualValue) ? "NaN" : Float.toString(actualValue); + final String expectString = Float.isNaN(expectedValue) ? "NaN" : Float.toString(expectedValue); + issues.add("Column " + name + " different from the expected set, first difference at row " + + position + " encountered " + actualString + " expected " + expectString); + return position; + } if (expectedValue == io.deephaven.util.QueryConstants.NULL_FLOAT || actualValue == io.deephaven.util.QueryConstants.NULL_FLOAT) { final String actualString = actualValue == io.deephaven.util.QueryConstants.NULL_FLOAT ? "null" @@ -297,6 +304,14 @@ private long diffChunk(@NotNull final RowSequence expectedChunkOk, } else if (chunkType == ChunkType.Double) { final double expectedValue = expectedValues.asDoubleChunk().get(ii); final double actualValue = actualValues.asDoubleChunk().get(ii); + if (Double.isNaN(expectedValue) || Double.isNaN(actualValue)) { + final String actualString = Double.isNaN(actualValue) ? "NaN" : Double.toString(actualValue); + final String expectString = + Double.isNaN(expectedValue) ? "NaN" : Double.toString(expectedValue); + issues.add("Column " + name + " different from the expected set, first difference at row " + + position + " encountered " + actualString + " expected " + expectString); + return position; + } if (expectedValue == io.deephaven.util.QueryConstants.NULL_DOUBLE || actualValue == io.deephaven.util.QueryConstants.NULL_DOUBLE) { final String actualString = actualValue == io.deephaven.util.QueryConstants.NULL_DOUBLE ? "null" 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 e3b24089b7a..e2756f5e9f3 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 @@ -4,18 +4,14 @@ import io.deephaven.api.updateby.UpdateByControl; import io.deephaven.api.updateby.UpdateByOperation; import io.deephaven.base.verify.Assert; +import io.deephaven.datastructures.util.CollectionUtil; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.impl.DataAccessHelpers; import io.deephaven.engine.table.impl.QueryTable; -import io.deephaven.engine.testutil.ControlledUpdateGraph; -import io.deephaven.engine.testutil.EvalNugget; -import io.deephaven.engine.testutil.GenerateTableUpdates; -import io.deephaven.engine.testutil.TstUtils; -import io.deephaven.engine.testutil.generator.CharGenerator; -import io.deephaven.engine.testutil.generator.SortedInstantGenerator; -import io.deephaven.engine.testutil.generator.TestDataGenerator; +import io.deephaven.engine.testutil.*; +import io.deephaven.engine.testutil.generator.*; import io.deephaven.engine.util.TableDiff; import io.deephaven.test.types.OutOfBandTest; import io.deephaven.time.DateTimeUtils; @@ -28,13 +24,12 @@ import java.math.BigInteger; import java.math.MathContext; import java.time.Duration; -import java.util.Arrays; -import java.util.EnumSet; -import java.util.List; -import java.util.Random; +import java.util.*; import java.util.function.Function; import static io.deephaven.engine.testutil.GenerateTableUpdates.generateAppends; +import static io.deephaven.engine.testutil.TstUtils.getTable; +import static io.deephaven.engine.testutil.TstUtils.initColumnInfos; import static io.deephaven.engine.testutil.testcase.RefreshingTableTestCase.simulateShiftAwareStep; import static io.deephaven.function.Basic.isNull; @@ -102,6 +97,62 @@ private String[] getCastingFormulas(String[] columns) { .toArray(String[]::new); } + /** + * Create a custom test table where the values are small enough they won't overflow Double.MAX_VALUE when + * multiplied. This will allow the results to be verified with the Numeric#product() function. + */ + @SuppressWarnings({"rawtypes"}) + static CreateResult createSmallTestTable(int tableSize, + boolean includeSym, + boolean includeGroups, + boolean isRefreshing, + int seed, + String[] extraNames, + TestDataGenerator[] extraGenerators) { + if (includeGroups && !includeSym) { + throw new IllegalArgumentException(); + } + + final List colsList = new ArrayList<>(); + final List generators = new ArrayList<>(); + if (includeSym) { + colsList.add("Sym"); + generators.add(new SetGenerator<>("a", "b", "c", "d", null)); + } + + if (extraNames.length > 0) { + colsList.addAll(Arrays.asList(extraNames)); + generators.addAll(Arrays.asList(extraGenerators)); + } + + colsList.addAll(Arrays.asList("byteCol", "shortCol", "intCol", "longCol", "floatCol", "doubleCol", "boolCol", + "bigIntCol", "bigDecimalCol")); + generators.addAll(Arrays.asList(new ByteGenerator((byte) -1, (byte) 5, .1), + new ShortGenerator((short) -1, (short) 5, .1), + new IntGenerator(-1, 5, .1), + new LongGenerator(-1, 5, .1), + new FloatGenerator(-1, 5, .1), + new DoubleGenerator(-1, 5, .1), + new BooleanGenerator(.5, .1), + new BigIntegerGenerator(new BigInteger("-1"), new BigInteger("5"), .1), + new BigDecimalGenerator(new BigInteger("1"), new BigInteger("2"), 5, .1))); + + final Random random = new Random(seed); + final ColumnInfo[] columnInfos = initColumnInfos(colsList.toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY), + generators.toArray(new TestDataGenerator[0])); + final QueryTable t = getTable(tableSize, random, columnInfos); + + + // if (!isRefreshing && includeGroups) { + // final ColumnSource groupingSource = t.getColumnSource("Sym"); + // groupingSource.setGroupingProvider(StaticGroupingProvider.buildFrom(groupingSource, t.getRowSet())); + // } + + t.setRefreshing(isRefreshing); + + return new CreateResult(t, columnInfos, random); + } + // region Object Helper functions final Function, BigInteger> prodBigInt = bigIntegerObjectVector -> { @@ -396,7 +447,7 @@ public void testStaticZeroKeyTimedFwdRev() { } private void doTestStaticZeroKey(final int prevTicks, final int postTicks) { - final QueryTable t = createTestTable(STATIC_TABLE_SIZE, true, false, false, 0x31313131, + final QueryTable t = createSmallTestTable(STATIC_TABLE_SIZE, true, false, false, 0x31313131, new String[] {"charCol"}, new TestDataGenerator[] {new CharGenerator('A', 'z', 0.1)}).t; @@ -411,7 +462,7 @@ private void doTestStaticZeroKey(final int prevTicks, final int postTicks) { } private void doTestStaticZeroKeyTimed(final Duration prevTime, final Duration postTime) { - final QueryTable t = createTestTable(STATIC_TABLE_SIZE, false, false, false, 0xFFFABBBC, + final QueryTable t = createSmallTestTable(STATIC_TABLE_SIZE, false, false, false, 0xFFFABBBC, new String[] {"ts", "charCol"}, new TestDataGenerator[] {new SortedInstantGenerator( DateTimeUtils.parseInstant("2022-03-09T09:00:00.000 NY"), DateTimeUtils.parseInstant("2022-03-09T16:30:00.000 NY")), diff --git a/replication/static/src/main/java/io/deephaven/replicators/ReplicateUpdateBy.java b/replication/static/src/main/java/io/deephaven/replicators/ReplicateUpdateBy.java index c5fb4853029..da86b6cb406 100644 --- a/replication/static/src/main/java/io/deephaven/replicators/ReplicateUpdateBy.java +++ b/replication/static/src/main/java/io/deephaven/replicators/ReplicateUpdateBy.java @@ -134,13 +134,17 @@ public static void main(String[] args) throws IOException { } } - files = ReplicatePrimitiveCode.charToAllButBoolean( - "engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/CharRollingProductOperator.java"); + files = ReplicatePrimitiveCode.charToIntegers( + "engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/CharRollingProductOperator.java", + exemptions); for (final String f : files) { - if (f.contains("Integer")) { + if (f.contains("Int")) { fixupInteger(f); } } + ReplicatePrimitiveCode.floatToAllFloatingPoints( + "engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java"); + files = ReplicatePrimitiveCode.charToAllButBoolean( "engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/delta/CharDeltaOperator.java", From 916946b9053b60de77215a5211178715f11143a9 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Wed, 8 Nov 2023 10:08:26 -0800 Subject: [PATCH 3/9] Correct post-merge test failure. --- .../table/impl/by/DoubleChunkedVarOperator.java | 13 +++++++++++++ .../table/impl/by/FloatChunkedVarOperator.java | 13 +++++++++++++ .../updateby/rollingstd/ByteRollingStdOperator.java | 5 ++++- .../updateby/rollingstd/CharRollingStdOperator.java | 5 ++++- .../rollingstd/DoubleRollingStdOperator.java | 5 ++++- .../rollingstd/FloatRollingStdOperator.java | 5 ++++- .../updateby/rollingstd/IntRollingStdOperator.java | 5 ++++- .../updateby/rollingstd/LongRollingStdOperator.java | 5 ++++- .../rollingstd/ShortRollingStdOperator.java | 5 ++++- 9 files changed, 54 insertions(+), 7 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 31f3b929c6d..c2f33d1b60d 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,6 +97,12 @@ private boolean addChunk(DoubleChunk values, long destination, if (forceNanResult || nonNullCount <= 1) { resultColumn.set(destination, Double.NaN); } else { + // If the sum has reach +/-Infinity, we are stuck with NaN forever. + if (Double.isInfinite(newSum) || Double.isInfinite(newSum2)) { + resultColumn.set(destination, Double.NaN); + return true; + } + // Perform the calculation in a way that minimizes the impact of FP error. final double eps = Math.ulp(newSum2); final double vs2bar = newSum * (newSum / nonNullCount); @@ -162,6 +168,13 @@ private boolean removeChunk(DoubleChunk values, long destinati 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)) { + resultColumn.set(destination, Double.NaN); + return true; + } + // Perform the calculation in a way that minimizes the impact of FP error. final double eps = Math.ulp(newSum2); final double vs2bar = newSum * (newSum / totalNormalCount); 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 43750b8fcd9..58670f7777c 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 @@ -92,6 +92,12 @@ private boolean addChunk(FloatChunk values, long destination, if (forceNanResult || nonNullCount <= 1) { resultColumn.set(destination, Double.NaN); } else { + // If the sum has reach +/-Infinity, we are stuck with NaN forever. + if (Double.isInfinite(newSum) || Double.isInfinite(newSum2)) { + resultColumn.set(destination, Double.NaN); + return true; + } + // Perform the calculation in a way that minimizes the impact of FP error. final double eps = Math.ulp(newSum2); final double vs2bar = newSum * (newSum / nonNullCount); @@ -157,6 +163,13 @@ private boolean removeChunk(FloatChunk values, long destinatio 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)) { + resultColumn.set(destination, Double.NaN); + return true; + } + // Perform the calculation in a way that minimizes the impact of FP error. final double eps = Math.ulp(newSum2); final double vs2bar = newSum * (newSum / totalNormalCount); 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 1a5c3b42770..1707f555603 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 @@ -121,7 +121,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } 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 b965e373639..4e2893a8120 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 @@ -115,7 +115,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } 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 b9320231ba0..3adbc7e04d3 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 @@ -120,7 +120,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } 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 e1c90523f14..082fa4f46e7 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 @@ -120,7 +120,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } 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 d35df8d2423..3aa840cbf17 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 @@ -120,7 +120,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } 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 87cb06edca2..793d441e497 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 @@ -120,7 +120,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } 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 e4335953489..84ca25415a8 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 @@ -120,7 +120,10 @@ public void writeToOutputChunk(int outIdx) { final double valueSquareSum = valueSquareBuffer.evaluate(); final double valueSum = valueBuffer.evaluate(); - if (Double.isNaN(valueSquareSum) || Double.isNaN(valueSum)) { + if (Double.isNaN(valueSquareSum) + || Double.isNaN(valueSum) + || Double.isInfinite(valueSquareSum) + || Double.isInfinite(valueSum)) { outputValues.set(outIdx, Double.NaN); return; } From 53a0a643d864de1ff3a8f2f16136c45035d8f83c Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 9 Nov 2023 12:55:27 -0800 Subject: [PATCH 4/9] Some PR comments addressed --- .../impl/by/DoubleChunkedVarOperator.java | 45 +++++++------------ .../impl/by/FloatChunkedVarOperator.java | 45 +++++++------------ .../DoubleRollingProductOperator.java | 12 ++++- .../FloatRollingProductOperator.java | 12 ++++- .../table/impl/updateby/BaseUpdateByTest.java | 17 ++++--- .../impl/updateby/TestRollingProduct.java | 13 +++--- 6 files changed, 70 insertions(+), 74 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 c2f33d1b60d..19b4ef31f6f 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,26 +97,13 @@ private boolean addChunk(DoubleChunk values, long destination, if (forceNanResult || nonNullCount <= 1) { resultColumn.set(destination, Double.NaN); } else { - // If the sum has reach +/-Infinity, we are stuck with NaN forever. + // If the sum or sumSquared has reached +/-Infinity, we are stuck with NaN forever. if (Double.isInfinite(newSum) || Double.isInfinite(newSum2)) { resultColumn.set(destination, Double.NaN); return true; } - - // Perform the calculation in a way that minimizes the impact of FP error. - final double eps = Math.ulp(newSum2); - final double vs2bar = newSum * (newSum / nonNullCount); - final double delta = newSum2 - vs2bar; - final double rel_eps = delta / eps; - - // Return zero when the variance is leq the FP error. - final double variance = Math.abs(rel_eps) > 1.0 ? delta / (nonNullCount - 1) : 0.0; - if (variance < 0.0) { - // Negative variance can only be due to FP error. - resultColumn.set(destination, 0.0); - } else { - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); - } + final double variance = computeVariance(nonNullCount, newSum, newSum2); + resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) <= 1)) { @@ -127,6 +114,17 @@ private boolean addChunk(DoubleChunk values, long destination, } } + private static double computeVariance(long nonNullCount, double newSum, double newSum2) { + // Perform the calculation in a way that minimizes the impact of FP error. + final double eps = Math.ulp(newSum2); + final double vs2bar = newSum * (newSum / nonNullCount); + final double delta = newSum2 - vs2bar; + final double rel_eps = delta / eps; + + // Return zero when the variance is leq the FP error or when variance becomes negative + final double variance = Math.abs(rel_eps) > 1.0 ? delta / (nonNullCount - 1) : 0.0; + return Math.max(variance, 0.0); + } private boolean removeChunk(DoubleChunk values, long destination, int chunkStart, int chunkSize) { final MutableDouble sum2 = new MutableDouble(); @@ -176,19 +174,8 @@ private boolean removeChunk(DoubleChunk values, long destinati } // Perform the calculation in a way that minimizes the impact of FP error. - final double eps = Math.ulp(newSum2); - final double vs2bar = newSum * (newSum / totalNormalCount); - final double delta = newSum2 - vs2bar; - final double rel_eps = delta / eps; - - // Return zero when the variance is leq the FP error. - final double variance = Math.abs(rel_eps) > 1.0 ? delta / (totalNormalCount - 1) : 0.0; - if (variance < 0.0) { - // Negative variance can only be due to FP error. - resultColumn.set(destination, 0.0); - } else { - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); - } + 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 58670f7777c..641e5e95ec7 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 @@ -92,26 +92,13 @@ private boolean addChunk(FloatChunk values, long destination, if (forceNanResult || nonNullCount <= 1) { resultColumn.set(destination, Double.NaN); } else { - // If the sum has reach +/-Infinity, we are stuck with NaN forever. + // If the sum or sumSquared has reached +/-Infinity, we are stuck with NaN forever. if (Double.isInfinite(newSum) || Double.isInfinite(newSum2)) { resultColumn.set(destination, Double.NaN); return true; } - - // Perform the calculation in a way that minimizes the impact of FP error. - final double eps = Math.ulp(newSum2); - final double vs2bar = newSum * (newSum / nonNullCount); - final double delta = newSum2 - vs2bar; - final double rel_eps = delta / eps; - - // Return zero when the variance is leq the FP error. - final double variance = Math.abs(rel_eps) > 1.0 ? delta / (nonNullCount - 1) : 0.0; - if (variance < 0.0) { - // Negative variance can only be due to FP error. - resultColumn.set(destination, 0.0); - } else { - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); - } + final double variance = computeVariance(nonNullCount, newSum, newSum2); + resultColumn.set(destination, std ? Math.sqrt(variance) : variance); } return true; } else if (forceNanResult || (nonNullCounter.getCountUnsafe(destination) <= 1)) { @@ -122,6 +109,17 @@ private boolean addChunk(FloatChunk values, long destination, } } + private static double computeVariance(long nonNullCount, double newSum, double newSum2) { + // Perform the calculation in a way that minimizes the impact of FP error. + final double eps = Math.ulp(newSum2); + final double vs2bar = newSum * (newSum / nonNullCount); + final double delta = newSum2 - vs2bar; + final double rel_eps = delta / eps; + + // Return zero when the variance is leq the FP error or when variance becomes negative + final double variance = Math.abs(rel_eps) > 1.0 ? delta / (nonNullCount - 1) : 0.0; + return Math.max(variance, 0.0); + } private boolean removeChunk(FloatChunk values, long destination, int chunkStart, int chunkSize) { final MutableDouble sum2 = new MutableDouble(); @@ -171,19 +169,8 @@ private boolean removeChunk(FloatChunk values, long destinatio } // Perform the calculation in a way that minimizes the impact of FP error. - final double eps = Math.ulp(newSum2); - final double vs2bar = newSum * (newSum / totalNormalCount); - final double delta = newSum2 - vs2bar; - final double rel_eps = delta / eps; - - // Return zero when the variance is leq the FP error. - final double variance = Math.abs(rel_eps) > 1.0 ? delta / (totalNormalCount - 1) : 0.0; - if (variance < 0.0) { - // Negative variance can only be due to FP error. - resultColumn.set(destination, 0.0); - } else { - resultColumn.set(destination, std ? Math.sqrt(variance) : variance); - } + 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/updateby/rollingproduct/DoubleRollingProductOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java index 9e5a297bfc8..6d0d3cef925 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/DoubleRollingProductOperator.java @@ -31,6 +31,7 @@ protected class Context extends BaseDoubleUpdateByOperator.Context { private int zeroCount; private int nanCount; + private int infCount; protected Context(final int affectedChunkSize, final int influencerChunkSize) { super(affectedChunkSize); @@ -50,6 +51,7 @@ protected Context(final int affectedChunkSize, final int influencerChunkSize) { true); zeroCount = 0; nanCount = 0; + infCount = 0; } @Override @@ -80,6 +82,8 @@ public void push(int pos, int count) { zeroCount++; } else if (Double.isNaN(val)) { nanCount++; + } else if (Double.isInfinite(val)) { + infCount++; } } } @@ -98,6 +102,8 @@ public void pop(int count) { --nanCount; } else if (val == 0) { --zeroCount; + } else if (Double.isInfinite(val)) { + --infCount; } } } @@ -107,9 +113,12 @@ public void writeToOutputChunk(int outIdx) { if (buffer.size() == nullCount) { outputValues.set(outIdx, NULL_DOUBLE); } else { - if (nanCount > 0) { + if (nanCount > 0 || (infCount > 0 && zeroCount > 0)) { + // Output NaN without evaluating the buffer when the buffer is poisoned with NaNs or when we + // have an Inf * 0 case outputValues.set(outIdx, Double.NaN); } else { + // When zeros are present, we can skip evaluating the buffer. outputValues.set(outIdx, zeroCount > 0 ? 0.0 : buffer.evaluate()); } } @@ -120,6 +129,7 @@ public void reset() { super.reset(); zeroCount = 0; nanCount = 0; + infCount = 0; buffer.clear(); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java index 45285089c56..f7868c2c0ae 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingproduct/FloatRollingProductOperator.java @@ -26,6 +26,7 @@ protected class Context extends BaseDoubleUpdateByOperator.Context { private int zeroCount; private int nanCount; + private int infCount; protected Context(final int affectedChunkSize, final int influencerChunkSize) { super(affectedChunkSize); @@ -45,6 +46,7 @@ protected Context(final int affectedChunkSize, final int influencerChunkSize) { true); zeroCount = 0; nanCount = 0; + infCount = 0; } @Override @@ -75,6 +77,8 @@ public void push(int pos, int count) { zeroCount++; } else if (Double.isNaN(val)) { nanCount++; + } else if (Double.isInfinite(val)) { + infCount++; } } } @@ -93,6 +97,8 @@ public void pop(int count) { --nanCount; } else if (val == 0) { --zeroCount; + } else if (Double.isInfinite(val)) { + --infCount; } } } @@ -102,9 +108,12 @@ public void writeToOutputChunk(int outIdx) { if (buffer.size() == nullCount) { outputValues.set(outIdx, NULL_DOUBLE); } else { - if (nanCount > 0) { + if (nanCount > 0 || (infCount > 0 && zeroCount > 0)) { + // Output NaN without evaluating the buffer when the buffer is poisoned with NaNs or when we + // have an Inf * 0 case outputValues.set(outIdx, Double.NaN); } else { + // When zeros are present, we can skip evaluating the buffer. outputValues.set(outIdx, zeroCount > 0 ? 0.0 : buffer.evaluate()); } } @@ -115,6 +124,7 @@ public void reset() { super.reset(); zeroCount = 0; nanCount = 0; + infCount = 0; buffer.clear(); } } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/BaseUpdateByTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/BaseUpdateByTest.java index 9dd2cb2b4cc..fcb3daec101 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/BaseUpdateByTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/BaseUpdateByTest.java @@ -1,6 +1,8 @@ package io.deephaven.engine.table.impl.updateby; import io.deephaven.datastructures.util.CollectionUtil; +import io.deephaven.engine.rowset.RowSet; +import io.deephaven.engine.table.impl.AbstractColumnSource; import io.deephaven.engine.table.impl.QueryTable; import io.deephaven.engine.testutil.ColumnInfo; import io.deephaven.engine.testutil.generator.*; @@ -9,10 +11,7 @@ import org.junit.Rule; import java.math.BigInteger; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Random; +import java.util.*; import static io.deephaven.engine.testutil.TstUtils.getTable; import static io.deephaven.engine.testutil.TstUtils.initColumnInfos; @@ -81,11 +80,11 @@ static CreateResult createTestTable(int tableSize, generators.toArray(new TestDataGenerator[0])); final QueryTable t = getTable(tableSize, random, columnInfos); - - // if (!isRefreshing && includeGroups) { - // final ColumnSource groupingSource = t.getColumnSource("Sym"); - // groupingSource.setGroupingProvider(StaticGroupingProvider.buildFrom(groupingSource, t.getRowSet())); - // } + if (!isRefreshing && includeGroups) { + final AbstractColumnSource groupingSource = (AbstractColumnSource) t.getColumnSource("Sym"); + final Map gtr = groupingSource.getValuesMapping(t.getRowSet()); + groupingSource.setGroupToRange(gtr); + } t.setRefreshing(isRefreshing); 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 e2756f5e9f3..d01a1749111 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 @@ -7,7 +7,10 @@ import io.deephaven.datastructures.util.CollectionUtil; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; +import io.deephaven.engine.rowset.RowSet; +import io.deephaven.engine.table.ColumnSource; import io.deephaven.engine.table.Table; +import io.deephaven.engine.table.impl.AbstractColumnSource; import io.deephaven.engine.table.impl.DataAccessHelpers; import io.deephaven.engine.table.impl.QueryTable; import io.deephaven.engine.testutil.*; @@ -142,11 +145,11 @@ static CreateResult createSmallTestTable(int tableSize, generators.toArray(new TestDataGenerator[0])); final QueryTable t = getTable(tableSize, random, columnInfos); - - // if (!isRefreshing && includeGroups) { - // final ColumnSource groupingSource = t.getColumnSource("Sym"); - // groupingSource.setGroupingProvider(StaticGroupingProvider.buildFrom(groupingSource, t.getRowSet())); - // } + if (!isRefreshing && includeGroups) { + final AbstractColumnSource groupingSource = (AbstractColumnSource) t.getColumnSource("Sym"); + final Map gtr = groupingSource.getValuesMapping(t.getRowSet()); + groupingSource.setGroupToRange(gtr); + } t.setRefreshing(isRefreshing); From 43ddf917d79748e27611748705fc6208358a0321 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Fri, 10 Nov 2023 10:27:10 -0800 Subject: [PATCH 5/9] Numeric class modified to better handle NaN and Inf values, new tests to verify standard behavior. --- engine/function/src/templates/Numeric.ftl | 183 ++++++++++-------- engine/function/src/templates/TestNumeric.ftl | 48 +++++ 2 files changed, 153 insertions(+), 78 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 49df0a3f4af..641a38ecd50 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -365,6 +365,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++; @@ -416,6 +421,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 += Math.abs(c); count++; @@ -464,10 +474,14 @@ public class Numeric { double sum = 0; double sum2 = 0; double count = 0; - 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 += (double)c; sum2 += (double)c * (double)c; @@ -476,8 +490,8 @@ public class Numeric { } } - // Return NaN if poisoned or too few values to compute variance. - if (count <= 1 || Double.isNaN(sum) || Double.isNaN(sum2)) { + // Return NaN if overflow or too few values to compute variance. + if (count <= 1 || Double.isInfinite(sum) || Double.isInfinite(sum2)) { return Double.NaN; } @@ -569,7 +583,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)) { sum += w * c; sum2 += w * c * c; @@ -1291,6 +1314,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)) { + return Double.NaN; + } + + <#if pt2.valueType.isFloat > + if (isNaN(v1)) { + return Double.NaN; + } + if (!isNull(v0) && !isNull(v1)) { sum0 += v0; @@ -1379,6 +1412,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)) { + return Double.NaN; + } + + <#if pt2.valueType.isFloat > + if (isNaN(v1)) { + return Double.NaN; + } + if (!isNull(v0) && !isNull(v1)) { sum0 += v0; @@ -1418,6 +1461,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 ${pt.boxed}.NaN; + } + if (!isNull(c)) { sum += c; } @@ -1454,10 +1502,33 @@ public class Numeric { ${pt.primitive} prod = 1; int count = 0; + <#if pt.valueType.isFloat > + double zeroCount = 0; + double infCount = 0; + 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 ${pt.boxed}.NaN; + } else if (Double.isInfinite(c)) { + if (zeroCount > 0) { + return ${pt.boxed}.NaN; + } + infCount++; + } else if (c == 0) { + if (infCount > 0) { + return ${pt.boxed}.NaN; + } + zeroCount++; + } + <#else> + if (c == 0) { + return 0; + } + if (!isNull(c)) { count++; prod *= c; @@ -1469,7 +1540,11 @@ public class Numeric { return ${pt.null}; } + <#if pt.valueType.isFloat > + return zeroCount > 0 ? 0 : (${pt.primitive}) (prod); + <#else> return (${pt.primitive}) (prod); + } /** @@ -1507,24 +1582,7 @@ public class Numeric { return null; } - if (values.length == 0) { - return new ${pt.primitive}[0]; - } - - ${pt.primitive}[] result = new ${pt.primitive}[values.length]; - result[0] = values[0]; - - for (int i = 1; i < values.length; i++) { - if (isNull(result[i - 1])) { - result[i] = values[i]; - } else if (isNull(values[i])) { - result[i] = result[i - 1]; - } else { - result[i] = (${pt.primitive})Math.min(result[i - 1], values[i]); - } - } - - return result; + return cummin(new ${pt.vectorDirect}(values)); } /** @@ -1588,24 +1646,7 @@ public class Numeric { return null; } - if (values.length == 0) { - return new ${pt.primitive}[0]; - } - - ${pt.primitive}[] result = new ${pt.primitive}[values.length]; - result[0] = values[0]; - - for (int i = 1; i < values.length; i++) { - if (isNull(result[i - 1])) { - result[i] = values[i]; - } else if (isNull(values[i])) { - result[i] = result[i - 1]; - } else { - result[i] = (${pt.primitive})Math.max(result[i - 1], values[i]); - } - } - - return result; + return cummax(new ${pt.vectorDirect}(values)); } /** @@ -1669,24 +1710,7 @@ public class Numeric { return null; } - if (values.length == 0) { - return new ${pt.primitive}[0]; - } - - ${pt.primitive}[] result = new ${pt.primitive}[values.length]; - result[0] = values[0]; - - for (int i = 1; i < values.length; i++) { - if (isNull(result[i - 1])) { - result[i] = values[i]; - } else if (isNull(values[i])) { - result[i] = result[i - 1]; - } else { - result[i] = (${pt.primitive}) (result[i - 1] + values[i]); - } - } - - return result; + return cumsum(new ${pt.vectorDirect}(values)); } /** @@ -1750,24 +1774,7 @@ public class Numeric { return null; } - if (values.length == 0) { - return new ${pt.primitive}[0]; - } - - ${pt.primitive}[] result = new ${pt.primitive}[values.length]; - result[0] = values[0]; - - for (int i = 1; i < values.length; i++) { - if (isNull(result[i - 1])) { - result[i] = values[i]; - } else if (isNull(values[i])) { - result[i] = result[i - 1]; - } else { - result[i] = (${pt.primitive}) (result[i - 1] * values[i]); - } - } - - return result; + return cumprod(new ${pt.vectorDirect}(values)); } /** @@ -2280,7 +2287,17 @@ 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; } @@ -2363,7 +2380,17 @@ 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 24226383f07..d4e38cbaa25 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -367,6 +367,28 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(tstat(v), tstat((${pt.primitive})0, (${pt.primitive})40, ${pt.null}, (${pt.primitive})50, (${pt.primitive})60, (${pt.primitive}) -1, (${pt.primitive})0)); } +<#if pt.valueType.isFloat > + public void test${pt.boxed}NanAndInfHandling() { + final ${pt.primitive}[] normalWithNaN = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NaN, 4, 5}; + assertTrue(Double.isNaN(var(normalWithNaN))); + assertTrue(Double.isNaN(std(normalWithNaN))); + assertTrue(Double.isNaN(ste(normalWithNaN))); + assertTrue(Double.isNaN(tstat(normalWithNaN))); + + final ${pt.primitive}[] normalWithInf = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.POSITIVE_INFINITY, 4, 5}; + assertTrue(Double.isNaN(var(normalWithNaN))); + assertTrue(Double.isNaN(std(normalWithNaN))); + assertTrue(Double.isNaN(ste(normalWithNaN))); + assertTrue(Double.isNaN(tstat(normalWithNaN))); + + final ${pt.primitive}[] normalWithNegInf = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NEGATIVE_INFINITY, 4, 5}; + assertTrue(Double.isNaN(var(normalWithNaN))); + assertTrue(Double.isNaN(std(normalWithNaN))); + assertTrue(Double.isNaN(ste(normalWithNaN))); + assertTrue(Double.isNaN(tstat(normalWithNaN))); + } + + <#list primitiveTypes as pt2> <#if pt2.valueType.isNumber > @@ -512,6 +534,32 @@ public class TestNumeric extends BaseArrayTestCase { assertEquals(${pt.null}, product((${pt.vector}) null)); } +<#if pt.valueType.isFloat > + public void test${pt.boxed}ProductOverflowAndNaN() { + final ${pt.primitive} LARGE_VALUE = Math.nextDown(${pt.boxed}.MAX_VALUE); + + final ${pt.primitive}[] overflow = new ${pt.primitive}[]{1, LARGE_VALUE, LARGE_VALUE}; + final ${pt.primitive} overflowProduct = product(overflow); + assertTrue(${pt.boxed}.isInfinite(overflowProduct) && overflowProduct > 0); + + final ${pt.primitive}[] negOverflow = new ${pt.primitive}[]{1, LARGE_VALUE, -LARGE_VALUE}; + final ${pt.primitive} negOverflowProduct = product(negOverflow); + assertTrue(${pt.boxed}.isInfinite(negOverflowProduct) && negOverflowProduct < 0); + + final ${pt.primitive}[] overflowWithZero = new ${pt.primitive}[]{1, LARGE_VALUE, LARGE_VALUE, 0}; + assertTrue(Math.abs(product(overflowWithZero)) == 0.0); + + final ${pt.primitive}[] normalWithNaN = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NaN, 4, 5}; + assertTrue(${pt.boxed}.isNaN(product(normalWithNaN))); + + final ${pt.primitive}[] posInfAndZero = new ${pt.primitive}[]{1, ${pt.boxed}.POSITIVE_INFINITY, 0}; + assertTrue(${pt.boxed}.isNaN(product(posInfAndZero))); + + final ${pt.primitive}[] negInfAndZero = new ${pt.primitive}[]{1, ${pt.boxed}.NEGATIVE_INFINITY, 0}; + assertTrue(${pt.boxed}.isNaN(product(negInfAndZero))); + } + + // public void test${pt.boxed}ProdObjectVector() { // assertEquals(new ${pt.primitive}[]{-30, 120}, product(new ObjectVectorDirect<>(new ${pt.primitive}[][]{{5, 4}, {-3, 5}, {2, 6}}))); // assertEquals(new ${pt.primitive}[]{-30, ${pt.null}}, product(new ObjectVectorDirect<>(new ${pt.primitive}[][]{{5, ${pt.null}}, {-3, 5}, {2, 6}}))); From d547db33ac5171350bf18fe71640f4b087037b0e Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Fri, 10 Nov 2023 12:09:27 -0800 Subject: [PATCH 6/9] Expanded the NaN and Inf test code to document our expectations. --- engine/function/src/templates/Numeric.ftl | 4 +- engine/function/src/templates/TestNumeric.ftl | 100 ++++++++++++++++-- 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 641a38ecd50..4c086f5b9d9 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -602,8 +602,8 @@ public class Numeric { } } - // Return NaN if poisoned or too few values to compute variance. - if (count <= 1 || Double.isNaN(sum) || Double.isNaN(sum2) || Double.isNaN(count) || Double.isNaN(count2)) { + // Return NaN if overflow or too few values to compute variance. + if (count <= 1 || Double.isInfinite(sum) || Double.isInfinite(sum2)) { return Double.NaN; } diff --git a/engine/function/src/templates/TestNumeric.ftl b/engine/function/src/templates/TestNumeric.ftl index d4e38cbaa25..c08e411e56d 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -368,24 +368,99 @@ public class TestNumeric extends BaseArrayTestCase { } <#if pt.valueType.isFloat > - public void test${pt.boxed}NanAndInfHandling() { + public void test${pt.boxed}NaNAndInfHandling() { + double result; + + final ${pt.primitive}[] normal = new ${pt.primitive}[]{1, 2, 3, 4, 5, 6}; + final ${pt.primitive}[] normalWithNaN = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NaN, 4, 5}; + assertTrue(Double.isNaN(avg(normalWithNaN))); assertTrue(Double.isNaN(var(normalWithNaN))); assertTrue(Double.isNaN(std(normalWithNaN))); assertTrue(Double.isNaN(ste(normalWithNaN))); assertTrue(Double.isNaN(tstat(normalWithNaN))); + assertTrue(Double.isNaN(wavg(normalWithNaN, normal))); + assertTrue(Double.isNaN(wvar(normalWithNaN, normal))); + assertTrue(Double.isNaN(wstd(normalWithNaN, normal))); + assertTrue(Double.isNaN(wste(normalWithNaN, normal))); + assertTrue(Double.isNaN(wtstat(normalWithNaN, normal))); + + assertTrue(Double.isNaN(wavg(normal, normalWithNaN))); + assertTrue(Double.isNaN(wvar(normal, normalWithNaN))); + assertTrue(Double.isNaN(wstd(normal, normalWithNaN))); + assertTrue(Double.isNaN(wste(normal, normalWithNaN))); + assertTrue(Double.isNaN(wtstat(normal, normalWithNaN))); + final ${pt.primitive}[] normalWithInf = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.POSITIVE_INFINITY, 4, 5}; - assertTrue(Double.isNaN(var(normalWithNaN))); - assertTrue(Double.isNaN(std(normalWithNaN))); - assertTrue(Double.isNaN(ste(normalWithNaN))); - assertTrue(Double.isNaN(tstat(normalWithNaN))); + result = avg(normalWithInf); + assertTrue(Double.isInfinite(result) && result > 0); // positive infinity + + assertTrue(Double.isNaN(var(normalWithInf))); + assertTrue(Double.isNaN(std(normalWithInf))); + assertTrue(Double.isNaN(ste(normalWithInf))); + assertTrue(Double.isNaN(tstat(normalWithInf))); + + result = wavg(normalWithInf, normal); + assertTrue(Double.isInfinite(result) && result > 0); // positive infinity + + assertTrue(Double.isNaN(wvar(normalWithInf, normal))); + assertTrue(Double.isNaN(wstd(normalWithInf, normal))); + assertTrue(Double.isNaN(wste(normalWithInf, normal))); + assertTrue(Double.isNaN(wtstat(normalWithInf, normal))); + + assertTrue(Double.isNaN(wavg(normal, normalWithInf))); // is NaN because of inf/inf division + assertTrue(Double.isNaN(wvar(normal, normalWithInf))); + assertTrue(Double.isNaN(wstd(normal, normalWithInf))); + assertTrue(Double.isNaN(wste(normal, normalWithInf))); + assertTrue(Double.isNaN(wtstat(normal, normalWithInf))); final ${pt.primitive}[] normalWithNegInf = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NEGATIVE_INFINITY, 4, 5}; - assertTrue(Double.isNaN(var(normalWithNaN))); - assertTrue(Double.isNaN(std(normalWithNaN))); - assertTrue(Double.isNaN(ste(normalWithNaN))); - assertTrue(Double.isNaN(tstat(normalWithNaN))); + result = avg(normalWithNegInf); + assertTrue(Double.isInfinite(result) && result < 0); // negative infinity + + assertTrue(Double.isNaN(var(normalWithNegInf))); + assertTrue(Double.isNaN(std(normalWithNegInf))); + assertTrue(Double.isNaN(ste(normalWithNegInf))); + assertTrue(Double.isNaN(tstat(normalWithNegInf))); + + result = wavg(normalWithNegInf, normal); + assertTrue(Double.isInfinite(result) && result < 0); // negative infinity + + assertTrue(Double.isNaN(wvar(normalWithNegInf, normal))); + assertTrue(Double.isNaN(wstd(normalWithNegInf, normal))); + assertTrue(Double.isNaN(wste(normalWithNegInf, normal))); + assertTrue(Double.isNaN(wtstat(normalWithNegInf, normal))); + + assertTrue(Double.isNaN(wavg(normal, normalWithNegInf))); // is NaN because of -inf/-inf division + assertTrue(Double.isNaN(wvar(normal, normalWithNegInf))); + assertTrue(Double.isNaN(wstd(normal, normalWithNegInf))); + assertTrue(Double.isNaN(wste(normal, normalWithNegInf))); + assertTrue(Double.isNaN(wtstat(normal, normalWithNegInf))); + + <#if pt.primitive == "double" > + // testing normal value overflow. NOTE: this is testing for doubles only, since overflowing a double using + // smaller types is quite difficult + final double LARGE_VALUE = Math.nextDown(Double.MAX_VALUE); + + final double[] overflow = new double[]{1, LARGE_VALUE, LARGE_VALUE}; + assertTrue(Double.isInfinite(avg(overflow))); + + assertTrue(Double.isNaN(var(overflow))); + assertTrue(Double.isNaN(std(overflow))); + assertTrue(Double.isNaN(ste(overflow))); + assertTrue(Double.isNaN(tstat(overflow))); + + final double[] negOverflow = new double[]{1, LARGE_VALUE, -LARGE_VALUE}; + assertTrue(Double.isNaN(var(negOverflow))); + assertTrue(Double.isNaN(std(negOverflow))); + assertTrue(Double.isNaN(ste(negOverflow))); + assertTrue(Double.isNaN(tstat(negOverflow))); + + final double[] negAdditionOverflow = new double[]{1, -LARGE_VALUE, -LARGE_VALUE}; + result = avg(negAdditionOverflow); + assertTrue(Double.isInfinite(result) && result < 0); // negative infinity + } @@ -557,6 +632,13 @@ public class TestNumeric extends BaseArrayTestCase { final ${pt.primitive}[] negInfAndZero = new ${pt.primitive}[]{1, ${pt.boxed}.NEGATIVE_INFINITY, 0}; assertTrue(${pt.boxed}.isNaN(product(negInfAndZero))); + + final ${pt.primitive}[] zeroAndPosInf = new ${pt.primitive}[]{1, 0, ${pt.boxed}.POSITIVE_INFINITY}; + assertTrue(${pt.boxed}.isNaN(product(zeroAndPosInf))); + + final ${pt.primitive}[] zeroAndNegInf = new ${pt.primitive}[]{1, 0, ${pt.boxed}.NEGATIVE_INFINITY}; + assertTrue(${pt.boxed}.isNaN(product(zeroAndNegInf))); + } From 47d30ff5cc9c82a0055ea0bcff0bf6ebb80f9dab Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 16 Nov 2023 10:00:45 -0800 Subject: [PATCH 7/9] Added new tests to cover some short circuit behavior. --- engine/function/src/templates/Numeric.ftl | 9 -------- engine/function/src/templates/TestNumeric.ftl | 21 ++++++++++++++++--- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index b8997351b0b..83205c23aff 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -2314,16 +2314,12 @@ 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; @@ -2407,17 +2403,12 @@ 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 61d14218eab..e8dd31f5fe8 100644 --- a/engine/function/src/templates/TestNumeric.ftl +++ b/engine/function/src/templates/TestNumeric.ftl @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2021 Deephaven Data Labs and Patent Pending + * Copyright (c) 2016-2023 Deephaven Data Labs and Patent Pending */ package io.deephaven.function; @@ -372,17 +372,22 @@ public class TestNumeric extends BaseArrayTestCase { final ${pt.primitive}[] normalWithNaN = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NaN, 4, 5}; assertTrue(Double.isNaN(avg(normalWithNaN))); + assertTrue(Double.isNaN(absAvg(normalWithNaN))); assertTrue(Double.isNaN(var(normalWithNaN))); assertTrue(Double.isNaN(std(normalWithNaN))); assertTrue(Double.isNaN(ste(normalWithNaN))); assertTrue(Double.isNaN(tstat(normalWithNaN))); + assertTrue(Double.isNaN(cov(normalWithNaN, normal))); + assertTrue(Double.isNaN(cor(normalWithNaN, normal))); assertTrue(Double.isNaN(wavg(normalWithNaN, normal))); assertTrue(Double.isNaN(wvar(normalWithNaN, normal))); assertTrue(Double.isNaN(wstd(normalWithNaN, normal))); assertTrue(Double.isNaN(wste(normalWithNaN, normal))); assertTrue(Double.isNaN(wtstat(normalWithNaN, normal))); + assertTrue(Double.isNaN(cov(normal, normalWithNaN))); + assertTrue(Double.isNaN(cor(normal, normalWithNaN))); assertTrue(Double.isNaN(wavg(normal, normalWithNaN))); assertTrue(Double.isNaN(wvar(normal, normalWithNaN))); assertTrue(Double.isNaN(wstd(normal, normalWithNaN))); @@ -392,20 +397,25 @@ public class TestNumeric extends BaseArrayTestCase { final ${pt.primitive}[] normalWithInf = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.POSITIVE_INFINITY, 4, 5}; result = avg(normalWithInf); assertTrue(Double.isInfinite(result) && result > 0); // positive infinity + result = absAvg(normalWithInf); + assertTrue(Double.isInfinite(result) && result > 0); // positive infinity assertTrue(Double.isNaN(var(normalWithInf))); assertTrue(Double.isNaN(std(normalWithInf))); assertTrue(Double.isNaN(ste(normalWithInf))); assertTrue(Double.isNaN(tstat(normalWithInf))); + assertTrue(Double.isNaN(cov(normalWithInf, normal))); + assertTrue(Double.isNaN(cor(normalWithInf, normal))); result = wavg(normalWithInf, normal); assertTrue(Double.isInfinite(result) && result > 0); // positive infinity - assertTrue(Double.isNaN(wvar(normalWithInf, normal))); assertTrue(Double.isNaN(wstd(normalWithInf, normal))); assertTrue(Double.isNaN(wste(normalWithInf, normal))); assertTrue(Double.isNaN(wtstat(normalWithInf, normal))); + assertTrue(Double.isNaN(cov(normal, normalWithInf))); + assertTrue(Double.isNaN(cor(normal, normalWithInf))); assertTrue(Double.isNaN(wavg(normal, normalWithInf))); // is NaN because of inf/inf division assertTrue(Double.isNaN(wvar(normal, normalWithInf))); assertTrue(Double.isNaN(wstd(normal, normalWithInf))); @@ -415,20 +425,25 @@ public class TestNumeric extends BaseArrayTestCase { final ${pt.primitive}[] normalWithNegInf = new ${pt.primitive}[]{1, 2, 3, ${pt.boxed}.NEGATIVE_INFINITY, 4, 5}; result = avg(normalWithNegInf); assertTrue(Double.isInfinite(result) && result < 0); // negative infinity + result = absAvg(normalWithNegInf); + assertTrue(Double.isInfinite(result) && result > 0); // positive infinity assertTrue(Double.isNaN(var(normalWithNegInf))); assertTrue(Double.isNaN(std(normalWithNegInf))); assertTrue(Double.isNaN(ste(normalWithNegInf))); assertTrue(Double.isNaN(tstat(normalWithNegInf))); + assertTrue(Double.isNaN(cov(normalWithNegInf, normal))); + assertTrue(Double.isNaN(cor(normalWithNegInf, normal))); result = wavg(normalWithNegInf, normal); assertTrue(Double.isInfinite(result) && result < 0); // negative infinity - assertTrue(Double.isNaN(wvar(normalWithNegInf, normal))); assertTrue(Double.isNaN(wstd(normalWithNegInf, normal))); assertTrue(Double.isNaN(wste(normalWithNegInf, normal))); assertTrue(Double.isNaN(wtstat(normalWithNegInf, normal))); + assertTrue(Double.isNaN(cov(normal, normalWithNegInf))); + assertTrue(Double.isNaN(cor(normal, normalWithNegInf))); assertTrue(Double.isNaN(wavg(normal, normalWithNegInf))); // is NaN because of -inf/-inf division assertTrue(Double.isNaN(wvar(normal, normalWithNegInf))); assertTrue(Double.isNaN(wstd(normal, normalWithNegInf))); From e2b1f18fdb44657de5182a1d787702cf70bff1b4 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 16 Nov 2023 11:28:56 -0800 Subject: [PATCH 8/9] Update engine/function/src/templates/Numeric.ftl Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- 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 83205c23aff..9b8dd097948 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -481,7 +481,7 @@ public class Numeric { double sum = 0; double sum2 = 0; - int count = 0; + long count = 0; try ( final ${pt.vectorIterator} vi = values.iterator() ) { while ( vi.hasNext() ) { final ${pt.primitive} c = vi.${pt.iteratorNext}(); From 7bb5e686379be1c0b21716c823edca47a78614a4 Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Thu, 16 Nov 2023 11:29:03 -0800 Subject: [PATCH 9/9] Update engine/function/src/templates/Numeric.ftl Co-authored-by: Chip Kent <5250374+chipkent@users.noreply.github.com> --- engine/function/src/templates/Numeric.ftl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/function/src/templates/Numeric.ftl b/engine/function/src/templates/Numeric.ftl index 9b8dd097948..d9d5404d3ca 100644 --- a/engine/function/src/templates/Numeric.ftl +++ b/engine/function/src/templates/Numeric.ftl @@ -1530,8 +1530,8 @@ public class Numeric { ${pt.primitive} prod = 1; int count = 0; <#if pt.valueType.isFloat > - int zeroCount = 0; - int infCount = 0; + long zeroCount = 0; + long infCount = 0; try ( final ${pt.vectorIterator} vi = values.iterator() ) {