From 25e0cb1ff86be7d7814085f8e096e9db4661f4bd Mon Sep 17 00:00:00 2001 From: Larry Booker Date: Wed, 8 May 2024 09:54:17 -0700 Subject: [PATCH] Widen returned types for `UpdateBy` floating point operations. (#5371) * Initial commit of changes for widening update_by returned types (Float -> Double) * Fixed NULL_FLOAT related bugs, adjusted tests to use correct datatypes. * Added all NULL tests to UpdateBy cumulative operations. --- .../updateby/prod/DoubleCumProdOperator.java | 1 + .../updateby/prod/FloatCumProdOperator.java | 9 +- .../rollingsum/DoubleRollingSumOperator.java | 7 +- .../rollingsum/FloatRollingSumOperator.java | 33 ++--- .../updateby/sum/DoubleCumSumOperator.java | 9 +- .../updateby/sum/FloatCumSumOperator.java | 15 ++- .../table/impl/updateby/BaseUpdateByTest.java | 48 ++++++-- .../table/impl/updateby/TestCumMinMax.java | 25 ++++ .../table/impl/updateby/TestCumProd.java | 17 +++ .../table/impl/updateby/TestCumSum.java | 114 ++++-------------- .../table/impl/updateby/TestRollingSum.java | 66 ++++++++-- 11 files changed, 197 insertions(+), 147 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/DoubleCumProdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/DoubleCumProdOperator.java index 56cd5ba585b..32b76c10ba9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/DoubleCumProdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/DoubleCumProdOperator.java @@ -16,6 +16,7 @@ import io.deephaven.engine.table.impl.updateby.internal.BaseDoubleUpdateByOperator; import org.jetbrains.annotations.NotNull; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; import static io.deephaven.util.QueryConstants.NULL_DOUBLE; public class DoubleCumProdOperator extends BaseDoubleUpdateByOperator { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/FloatCumProdOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/FloatCumProdOperator.java index af36f5c9539..5072c54f293 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/FloatCumProdOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/prod/FloatCumProdOperator.java @@ -9,16 +9,17 @@ import io.deephaven.chunk.attributes.Values; import io.deephaven.engine.table.impl.MatchPair; import io.deephaven.engine.table.impl.updateby.UpdateByOperator; -import io.deephaven.engine.table.impl.updateby.internal.BaseFloatUpdateByOperator; +import io.deephaven.engine.table.impl.updateby.internal.BaseDoubleUpdateByOperator; import org.jetbrains.annotations.NotNull; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; import static io.deephaven.util.QueryConstants.NULL_FLOAT; -public class FloatCumProdOperator extends BaseFloatUpdateByOperator { +public class FloatCumProdOperator extends BaseDoubleUpdateByOperator { // region extra-fields // endregion extra-fields - protected class Context extends BaseFloatUpdateByOperator.Context { + protected class Context extends BaseDoubleUpdateByOperator.Context { public FloatChunk floatValueChunk; protected Context(final int chunkSize) { @@ -37,7 +38,7 @@ public void push(int pos, int count) { final float val = floatValueChunk.get(pos); if (val != NULL_FLOAT) { - curVal = curVal == NULL_FLOAT ? val : curVal * val; + curVal = curVal == NULL_DOUBLE ? val : curVal * val; } } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/DoubleRollingSumOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/DoubleRollingSumOperator.java index 5959e8e3fe4..36f2c5beb7a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/DoubleRollingSumOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/DoubleRollingSumOperator.java @@ -18,6 +18,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; import static io.deephaven.util.QueryConstants.NULL_DOUBLE; public class DoubleRollingSumOperator extends BaseDoubleUpdateByOperator { @@ -60,11 +61,13 @@ public void push(int pos, int count) { aggSum.ensureRemaining(count); for (int ii = 0; ii < count; ii++) { - double val = doubleInfluencerValuesChunk.get(pos + ii); - aggSum.addUnsafe(val); + final double val = doubleInfluencerValuesChunk.get(pos + ii); if (val == NULL_DOUBLE) { nullCount++; + aggSum.addUnsafe(NULL_DOUBLE); + } else { + aggSum.addUnsafe(val); } } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/FloatRollingSumOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/FloatRollingSumOperator.java index 56bbad7f7dc..a5c26d7a4d5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/FloatRollingSumOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingsum/FloatRollingSumOperator.java @@ -3,37 +3,38 @@ // package io.deephaven.engine.table.impl.updateby.rollingsum; -import io.deephaven.base.ringbuffer.AggregatingFloatRingBuffer; +import io.deephaven.base.ringbuffer.AggregatingDoubleRingBuffer; import io.deephaven.base.verify.Assert; import io.deephaven.chunk.Chunk; import io.deephaven.chunk.FloatChunk; import io.deephaven.chunk.attributes.Values; import io.deephaven.engine.table.impl.MatchPair; import io.deephaven.engine.table.impl.updateby.UpdateByOperator; -import io.deephaven.engine.table.impl.updateby.internal.BaseFloatUpdateByOperator; +import io.deephaven.engine.table.impl.updateby.internal.BaseDoubleUpdateByOperator; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; import static io.deephaven.util.QueryConstants.NULL_FLOAT; -public class FloatRollingSumOperator extends BaseFloatUpdateByOperator { +public class FloatRollingSumOperator extends BaseDoubleUpdateByOperator { private static final int BUFFER_INITIAL_SIZE = 64; - protected class Context extends BaseFloatUpdateByOperator.Context { + protected class Context extends BaseDoubleUpdateByOperator.Context { protected FloatChunk floatInfluencerValuesChunk; - protected AggregatingFloatRingBuffer aggSum; + protected AggregatingDoubleRingBuffer aggSum; protected Context(final int chunkSize) { super(chunkSize); - aggSum = new AggregatingFloatRingBuffer(BUFFER_INITIAL_SIZE, + aggSum = new AggregatingDoubleRingBuffer(BUFFER_INITIAL_SIZE, 0, - Float::sum, // tree function + Double::sum, // tree function (a, b) -> { // value function - if (a == NULL_FLOAT && b == NULL_FLOAT) { + if (a == NULL_DOUBLE && b == NULL_DOUBLE) { return 0; // identity val - } else if (a == NULL_FLOAT) { + } else if (a == NULL_DOUBLE) { return b; - } else if (b == NULL_FLOAT) { + } else if (b == NULL_DOUBLE) { return a; } return a + b; @@ -56,11 +57,13 @@ public void push(int pos, int count) { aggSum.ensureRemaining(count); for (int ii = 0; ii < count; ii++) { - float val = floatInfluencerValuesChunk.get(pos + ii); - aggSum.addUnsafe(val); + final float val = floatInfluencerValuesChunk.get(pos + ii); if (val == NULL_FLOAT) { nullCount++; + aggSum.addUnsafe(NULL_DOUBLE); + } else { + aggSum.addUnsafe(val); } } } @@ -70,9 +73,9 @@ public void pop(int count) { Assert.geq(aggSum.size(), "aggSum.size()", count); for (int ii = 0; ii < count; ii++) { - float val = aggSum.removeUnsafe(); + double val = aggSum.removeUnsafe(); - if (val == NULL_FLOAT) { + if (val == NULL_DOUBLE) { nullCount--; } } @@ -81,7 +84,7 @@ public void pop(int count) { @Override public void writeToOutputChunk(int outIdx) { if (aggSum.size() == nullCount) { - outputValues.set(outIdx, NULL_FLOAT); + outputValues.set(outIdx, NULL_DOUBLE); } else { outputValues.set(outIdx, aggSum.evaluate()); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/DoubleCumSumOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/DoubleCumSumOperator.java index 9c06cc12136..dbd5aa1fa62 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/DoubleCumSumOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/DoubleCumSumOperator.java @@ -16,6 +16,7 @@ import io.deephaven.engine.table.impl.updateby.internal.BaseDoubleUpdateByOperator; import org.jetbrains.annotations.NotNull; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; import static io.deephaven.util.QueryConstants.NULL_DOUBLE; public class DoubleCumSumOperator extends BaseDoubleUpdateByOperator { @@ -37,12 +38,10 @@ public void push(int pos, int count) { Assert.eq(count, "push count", 1); // read the value from the values chunk - final double currentVal = doubleValueChunk.get(pos); + final double val = doubleValueChunk.get(pos); - if (curVal == NULL_DOUBLE) { - curVal = currentVal; - } else if (currentVal != NULL_DOUBLE) { - curVal += currentVal; + if (val != NULL_DOUBLE) { + curVal = curVal == NULL_DOUBLE ? val : curVal + val; } } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/FloatCumSumOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/FloatCumSumOperator.java index 08f7ffce1f3..883306a6502 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/FloatCumSumOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/sum/FloatCumSumOperator.java @@ -9,14 +9,15 @@ import io.deephaven.chunk.attributes.Values; import io.deephaven.engine.table.impl.MatchPair; import io.deephaven.engine.table.impl.updateby.UpdateByOperator; -import io.deephaven.engine.table.impl.updateby.internal.BaseFloatUpdateByOperator; +import io.deephaven.engine.table.impl.updateby.internal.BaseDoubleUpdateByOperator; import org.jetbrains.annotations.NotNull; +import static io.deephaven.util.QueryConstants.NULL_DOUBLE; import static io.deephaven.util.QueryConstants.NULL_FLOAT; -public class FloatCumSumOperator extends BaseFloatUpdateByOperator { +public class FloatCumSumOperator extends BaseDoubleUpdateByOperator { - protected class Context extends BaseFloatUpdateByOperator.Context { + protected class Context extends BaseDoubleUpdateByOperator.Context { public FloatChunk floatValueChunk; protected Context(final int chunkSize) { @@ -33,12 +34,10 @@ public void push(int pos, int count) { Assert.eq(count, "push count", 1); // read the value from the values chunk - final float currentVal = floatValueChunk.get(pos); + final float val = floatValueChunk.get(pos); - if (curVal == NULL_FLOAT) { - curVal = currentVal; - } else if (currentVal != NULL_FLOAT) { - curVal += currentVal; + if (val != NULL_FLOAT) { + curVal = curVal == NULL_DOUBLE ? val : curVal + val; } } } 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 8f8054f8925..dc34435515a 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 @@ -42,14 +42,42 @@ static CreateResult createTestTable(int tableSize, boolean includeSym, boolean i CollectionUtil.ZERO_LENGTH_STRING_ARRAY, new TestDataGenerator[0]); } + static CreateResult createTestTable( + int tableSize, + boolean includeSym, + boolean includeGroups, + boolean isRefreshing, + int seed, + String[] extraNames, + TestDataGenerator[] extraGenerators) { + return createTestTable(tableSize, includeSym, includeGroups, isRefreshing, seed, extraNames, extraGenerators, + 0.1); + } + @SuppressWarnings({"rawtypes"}) - static CreateResult createTestTable(int tableSize, + static CreateResult createTestTableAllNull( + int tableSize, boolean includeSym, boolean includeGroups, boolean isRefreshing, int seed, String[] extraNames, TestDataGenerator[] extraGenerators) { + + return createTestTable(tableSize, includeSym, includeGroups, isRefreshing, seed, extraNames, extraGenerators, + 1.0); + } + + @SuppressWarnings({"rawtypes"}) + static CreateResult createTestTable( + int tableSize, + boolean includeSym, + boolean includeGroups, + boolean isRefreshing, + int seed, + String[] extraNames, + TestDataGenerator[] extraGenerators, + double nullFraction) { if (includeGroups && !includeSym) { throw new IllegalArgumentException(); } @@ -68,15 +96,15 @@ static CreateResult createTestTable(int tableSize, colsList.addAll(Arrays.asList("byteCol", "shortCol", "intCol", "longCol", "floatCol", "doubleCol", "boolCol", "bigIntCol", "bigDecimalCol")); - generators.addAll(Arrays.asList(new ByteGenerator((byte) -127, (byte) 127, .1), - new ShortGenerator((short) -6000, (short) 65535, .1), - new IntGenerator(10, 100, .1), - new LongGenerator(10, 100, .1), - new FloatGenerator(10.1F, 20.1F, .1), - new DoubleGenerator(10.1, 20.1, .1), - new BooleanGenerator(.5, .1), - new BigIntegerGenerator(new BigInteger("-10"), new BigInteger("10"), .1), - new BigDecimalGenerator(new BigInteger("1"), new BigInteger("2"), 5, .1))); + generators.addAll(Arrays.asList(new ByteGenerator((byte) -127, (byte) 127, nullFraction), + new ShortGenerator((short) -6000, (short) 65535, nullFraction), + new IntGenerator(10, 100, nullFraction), + new LongGenerator(10, 100, nullFraction), + new FloatGenerator(10.1F, 20.1F, nullFraction), + new DoubleGenerator(10.1, 20.1, nullFraction), + new BooleanGenerator(.5, nullFraction), + new BigIntegerGenerator(new BigInteger("-10"), new BigInteger("10"), nullFraction), + new BigDecimalGenerator(new BigInteger("1"), new BigInteger("2"), 5, nullFraction))); final Random random = new Random(seed); final ColumnInfo[] columnInfos = initColumnInfos(colsList.toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY), diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumMinMax.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumMinMax.java index f6a59ca2167..684d08f6ead 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumMinMax.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumMinMax.java @@ -4,6 +4,7 @@ package io.deephaven.engine.table.impl.updateby; import io.deephaven.api.updateby.UpdateByOperation; +import io.deephaven.datastructures.util.CollectionUtil; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.table.PartitionedTable; import io.deephaven.engine.table.Table; @@ -12,6 +13,7 @@ import io.deephaven.engine.testutil.EvalNugget; import io.deephaven.engine.table.impl.QueryTable; import io.deephaven.engine.testutil.TstUtils; +import io.deephaven.engine.testutil.generator.TestDataGenerator; import io.deephaven.function.Numeric; import io.deephaven.test.types.OutOfBandTest; import org.jetbrains.annotations.NotNull; @@ -52,6 +54,29 @@ public void testStaticZeroKey() { } } + @Test + public void testStaticZeroKeyAllNulls() { + final QueryTable t = createTestTableAllNull(100000, false, false, false, 0x31313131, + CollectionUtil.ZERO_LENGTH_STRING_ARRAY, new TestDataGenerator[0]).t; + + final Table result = t.updateBy(List.of( + UpdateByOperation.CumMin("byteColMin=byteCol", "shortColMin=shortCol", "intColMin=intCol", + "longColMin=longCol", "floatColMin=floatCol", "doubleColMin=doubleCol", + "bigIntColMin=bigIntCol", "bigDecimalColMin=bigDecimalCol"), + UpdateByOperation.CumMax("byteColMax=byteCol", "shortColMax=shortCol", "intColMax=intCol", + "longColMax=longCol", "floatColMax=floatCol", "doubleColMax=doubleCol", + "bigIntColMax=bigIntCol", "bigDecimalColMax=bigDecimalCol"))); + for (String col : t.getDefinition().getColumnNamesArray()) { + if ("boolCol".equals(col)) { + continue; + } + assertWithCumMin(DataAccessHelpers.getColumn(t, col).getDirect(), + DataAccessHelpers.getColumn(result, col + "Min").getDirect()); + assertWithCumMax(DataAccessHelpers.getColumn(t, col).getDirect(), + DataAccessHelpers.getColumn(result, col + "Max").getDirect()); + } + } + // endregion // region Bucketed Tests diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumProd.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumProd.java index e43c2997cf0..0f75fe5b85e 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumProd.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumProd.java @@ -4,6 +4,7 @@ package io.deephaven.engine.table.impl.updateby; import io.deephaven.api.updateby.UpdateByControl; +import io.deephaven.datastructures.util.CollectionUtil; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.table.PartitionedTable; import io.deephaven.engine.table.Table; @@ -14,6 +15,7 @@ import io.deephaven.engine.testutil.GenerateTableUpdates; import io.deephaven.engine.testutil.EvalNugget; import io.deephaven.engine.testutil.TstUtils; +import io.deephaven.engine.testutil.generator.TestDataGenerator; import io.deephaven.function.Numeric; import io.deephaven.test.types.OutOfBandTest; import org.jetbrains.annotations.NotNull; @@ -53,6 +55,21 @@ public void testStaticZeroKey() { } } + @Test + public void testStaticZeroKeyAllNulls() { + final QueryTable t = createTestTableAllNull(100000, false, false, false, 0x31313131, + CollectionUtil.ZERO_LENGTH_STRING_ARRAY, new TestDataGenerator[0]).t; + final Table result = t.updateBy(UpdateByOperation.CumProd()); + for (String col : t.getDefinition().getColumnNamesArray()) { + if ("boolCol".equals(col)) { + continue; + } + assertWithCumProd(DataAccessHelpers.getColumn(t, col).getDirect(), + DataAccessHelpers.getColumn(result, col).getDirect(), + DataAccessHelpers.getColumn(result, col).getType()); + } + } + // endregion // region Bucketed Tests diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumSum.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumSum.java index 488ff43ec68..9ef95d01e31 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumSum.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumSum.java @@ -5,6 +5,7 @@ import io.deephaven.api.updateby.UpdateByControl; import io.deephaven.api.updateby.UpdateByOperation; +import io.deephaven.datastructures.util.CollectionUtil; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.table.PartitionedTable; import io.deephaven.engine.table.Table; @@ -56,6 +57,21 @@ public void testStaticZeroKey() { } } + @Test + public void testStaticZeroKeyAllNulls() { + final QueryTable t = createTestTableAllNull(100000, false, false, false, 0x31313131, + CollectionUtil.ZERO_LENGTH_STRING_ARRAY, new TestDataGenerator[0]).t; + + t.setRefreshing(false); + + final Table summed = t.updateBy(UpdateByOperation.CumSum()); + for (String col : t.getDefinition().getColumnNamesArray()) { + assertWithCumSum(DataAccessHelpers.getColumn(t, col).getDirect(), + DataAccessHelpers.getColumn(summed, col).getDirect(), + DataAccessHelpers.getColumn(summed, col).getType()); + } + } + // endregion // region Bucketed Tests @@ -211,91 +227,7 @@ protected Table e() { */ // endregion - private long[] cumsum(byte[] values) { - if (values == null) { - return null; - } - - if (values.length == 0) { - return new long[0]; - } - - long[] result = new long[values.length]; - result[0] = isNull(values[0]) ? NULL_LONG : values[0]; - - for (int i = 1; i < values.length; i++) { - final boolean curValNull = isNull(values[i]); - if (isNull(result[i - 1])) { - result[i] = curValNull ? NULL_LONG : values[i]; - } else { - if (curValNull) { - result[i] = result[i - 1]; - } else { - result[i] = result[i - 1] + values[i]; - } - } - } - - return result; - } - - private long[] cumsum(short[] values) { - if (values == null) { - return null; - } - - if (values.length == 0) { - return new long[0]; - } - - long[] result = new long[values.length]; - result[0] = isNull(values[0]) ? NULL_LONG : values[0]; - - for (int i = 1; i < values.length; i++) { - final boolean curValNull = isNull(values[i]); - if (isNull(result[i - 1])) { - result[i] = curValNull ? NULL_LONG : values[i]; - } else { - if (curValNull) { - result[i] = result[i - 1]; - } else { - result[i] = result[i - 1] + values[i]; - } - } - } - - return result; - } - - private long[] cumsum(int[] values) { - if (values == null) { - return null; - } - - if (values.length == 0) { - return new long[0]; - } - - long[] result = new long[values.length]; - result[0] = isNull(values[0]) ? NULL_LONG : values[0]; - - for (int i = 1; i < values.length; i++) { - final boolean curValNull = isNull(values[i]); - if (isNull(result[i - 1])) { - result[i] = curValNull ? NULL_LONG : values[i]; - } else { - if (curValNull) { - result[i] = result[i - 1]; - } else { - result[i] = result[i - 1] + values[i]; - } - } - } - - return result; - } - - private long[] cumsum(Boolean[] values) { + private long[] boolean_cumsum(Boolean[] values) { if (values == null) { return null; } @@ -323,7 +255,7 @@ private long[] cumsum(Boolean[] values) { return result; } - public static Object[] cumSum(Object[] values, final boolean isBD) { + public static Object[] big_cumSum(Object[] values, final boolean isBD) { if (values == null) { return null; } @@ -353,11 +285,11 @@ public static Object[] cumSum(Object[] values, final boolean isBD) { final void assertWithCumSum(@NotNull final Object expected, @NotNull final Object actual, Class type) { if (expected instanceof byte[]) { - assertArrayEquals(cumsum((byte[]) expected), (long[]) actual); + assertArrayEquals(Numeric.cumsum((byte[]) expected), (long[]) actual); } else if (expected instanceof short[]) { - assertArrayEquals(cumsum((short[]) expected), (long[]) actual); + assertArrayEquals(Numeric.cumsum((short[]) expected), (long[]) actual); } else if (expected instanceof int[]) { - assertArrayEquals(cumsum((int[]) expected), (long[]) actual); + assertArrayEquals(Numeric.cumsum((int[]) expected), (long[]) actual); } else if (expected instanceof long[]) { assertArrayEquals(Numeric.cumsum((long[]) expected), (long[]) actual); } else if (expected instanceof float[]) { @@ -365,9 +297,9 @@ final void assertWithCumSum(@NotNull final Object expected, @NotNull final Objec } else if (expected instanceof double[]) { assertArrayEquals(Numeric.cumsum((double[]) expected), (double[]) actual, .001d); } else if (expected instanceof Boolean[]) { - assertArrayEquals(cumsum((Boolean[]) expected), (long[]) actual); + assertArrayEquals(boolean_cumsum((Boolean[]) expected), (long[]) actual); } else { - assertArrayEquals(cumSum((Object[]) expected, type == BigDecimal.class), (Object[]) actual); + assertArrayEquals(big_cumSum((Object[]) expected, type == BigDecimal.class), (Object[]) actual); } } } 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 a5397d9c45d..c92fe6f46d8 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 @@ -61,6 +61,24 @@ public EnumSet diffItems() { // region Static Zero Key Tests + @Test + public void testStaticZeroKeyWithAllNullWindows() { + final QueryTable t = createTestTable(10000, false, false, false, 0x31313131).t; + t.setRefreshing(false); + + // With a window size of 1 and 10% null generation, guaranteed to cover the all NULL case. + final int prevTicks = 1; + final int postTicks = 0; + + final Table summed = t.updateBy(UpdateByOperation.RollingSum(prevTicks, postTicks)); + + for (String col : t.getDefinition().getColumnNamesArray()) { + assertWithRollingSumTicks(DataAccessHelpers.getColumn(t, col).getDirect(), + DataAccessHelpers.getColumn(summed, col).getDirect(), + DataAccessHelpers.getColumn(summed, col).getType(), prevTicks, postTicks); + } + } + @Test public void testStaticZeroKeyRev() { final QueryTable t = createTestTable(10000, false, false, false, 0x31313131).t; @@ -313,6 +331,14 @@ public void testNullOnBucketChange() { assertTableEquals(expected, r); } + @Test + public void testStaticBucketedAllNull() { + // With a window size of 1 and 10% null generation, guaranteed to cover the all NULL case. + final int prevTicks = 1; + final int postTicks = 0; + doTestStaticBucketed(false, prevTicks, postTicks); + } + @Test public void testStaticBucketedRev() { final int prevTicks = 100; @@ -445,6 +471,14 @@ private void doTestStaticBucketedTimed(boolean grouped, Duration prevTime, Durat // region Live Tests + @Test + public void testZeroKeyAppendOnlyAllNull() { + // With a window size of 1 and 10% null generation, guaranteed to cover the all NULL case. + final int prevTicks = 1; + final int postTicks = 0; + doTestAppendOnly(false, prevTicks, postTicks); + } + @Test public void testZeroKeyAppendOnlyRev() { final int prevTicks = 100; @@ -480,6 +514,14 @@ public void testZeroKeyAppendOnlyFwdRev() { doTestAppendOnly(false, prevTicks, postTicks); } + @Test + public void testBucketedAppendOnlyAllNull() { + // With a window size of 1 and 10% null generation, guaranteed to cover the all NULL case. + final int prevTicks = 1; + final int postTicks = 0; + doTestAppendOnly(true, prevTicks, postTicks); + } + @Test public void testBucketedAppendOnlyRev() { final int prevTicks = 100; @@ -1193,20 +1235,20 @@ private long[] rollingSum(long[] values, int prevTicks, int postTicks) { return result; } - private float[] rollingSum(float[] values, int prevTicks, int postTicks) { + private double[] rollingSum(float[] values, int prevTicks, int postTicks) { if (values == null) { return null; } if (values.length == 0) { - return new float[0]; + return new double[0]; } - float[] result = new float[values.length]; + double[] result = new double[values.length]; for (int i = 0; i < values.length; i++) { - result[i] = NULL_FLOAT; + result[i] = NULL_DOUBLE; // set the head and the tail final int head = Math.max(0, i - prevTicks + 1); @@ -1215,7 +1257,7 @@ private float[] rollingSum(float[] values, int prevTicks, int postTicks) { // compute everything in this window for (int computeIdx = head; computeIdx <= tail; computeIdx++) { if (!isNull(values[computeIdx])) { - if (result[i] == NULL_FLOAT) { + if (result[i] == NULL_DOUBLE) { result[i] = values[computeIdx]; } else { result[i] += values[computeIdx]; @@ -1534,22 +1576,22 @@ private long[] rollingSumTime(long[] values, long[] timestamps, long prevNanos, return result; } - private float[] rollingSumTime(float[] values, long[] timestamps, long prevNanos, long postNanos) { + private double[] rollingSumTime(float[] values, long[] timestamps, long prevNanos, long postNanos) { if (values == null) { return null; } if (values.length == 0) { - return new float[0]; + return new double[0]; } - float[] result = new float[values.length]; + double[] result = new double[values.length]; int head = 0; int tail = 0; for (int i = 0; i < values.length; i++) { - result[i] = NULL_FLOAT; + result[i] = NULL_DOUBLE; // check the current timestamp. skip if NULL if (timestamps[i] == NULL_LONG) { @@ -1572,7 +1614,7 @@ private float[] rollingSumTime(float[] values, long[] timestamps, long prevNanos // compute everything in this window for (int computeIdx = head; computeIdx < tail; computeIdx++) { if (!isNull(values[computeIdx])) { - if (result[i] == NULL_FLOAT) { + if (result[i] == NULL_DOUBLE) { result[i] = values[computeIdx]; } else { result[i] += values[computeIdx]; @@ -1756,7 +1798,7 @@ final void assertWithRollingSumTicks(@NotNull final Object expected, @NotNull fi } else if (expected instanceof long[]) { assertArrayEquals(rollingSum((long[]) expected, prevTicks, postTicks), (long[]) actual); } else if (expected instanceof float[]) { - assertArrayEquals(rollingSum((float[]) expected, prevTicks, postTicks), (float[]) actual, deltaF); + assertArrayEquals(rollingSum((float[]) expected, prevTicks, postTicks), (double[]) actual, deltaF); } else if (expected instanceof double[]) { assertArrayEquals(rollingSum((double[]) expected, prevTicks, postTicks), (double[]) actual, deltaD); } else if (expected instanceof Boolean[]) { @@ -1785,7 +1827,7 @@ final void assertWithRollingSumTime(@NotNull final Object expected, @NotNull fin } else if (expected instanceof long[]) { assertArrayEquals(rollingSumTime((long[]) expected, timestamps, prevTime, postTime), (long[]) actual); } else if (expected instanceof float[]) { - assertArrayEquals(rollingSumTime((float[]) expected, timestamps, prevTime, postTime), (float[]) actual, + assertArrayEquals(rollingSumTime((float[]) expected, timestamps, prevTime, postTime), (double[]) actual, deltaF); } else if (expected instanceof double[]) { assertArrayEquals(rollingSumTime((double[]) expected, timestamps, prevTime, postTime), (double[]) actual,