You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "ani5rudh (via GitHub)" <gi...@apache.org> on 2023/07/20 16:17:37 UTC
[GitHub] [commons-statistics] ani5rudh opened a new pull request, #50: STATISTICS-77: Sum Implementation
ani5rudh opened a new pull request, #50:
URL: https://github.com/apache/commons-statistics/pull/50
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-statistics] aherbert merged pull request #50: STATISTICS-77: Sum Implementation
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert merged PR #50:
URL: https://github.com/apache/commons-statistics/pull/50
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-statistics] ani5rudh commented on a diff in pull request #50: STATISTICS-77: Sum Implementation
Posted by "ani5rudh (via GitHub)" <gi...@apache.org>.
ani5rudh commented on code in PR #50:
URL: https://github.com/apache/commons-statistics/pull/50#discussion_r1271838639
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.commons.rng.UniformRandomProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Test for {@link Sum}.
+ */
+final class SumTest {
+
+ @Test
+ void testEmpty() {
+ Sum sum = Sum.create();
+ Assertions.assertEquals(0.0, sum.getAsDouble());
+ }
+
+ @Test
+ void testNan() {
+ Sum sum = Sum.create();
+ double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ for (double value : testArray) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(Double.NaN, sum.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSum(double[] values, double expected) {
+ Sum sum = Sum.create();
+ for (double value : values) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(expected, sum.getAsDouble(), "sum");
+ Assertions.assertEquals(expected, Sum.of(values).getAsDouble(), "of (values)");
+ }
+
+ static Stream<Arguments> testSum() {
Review Comment:
I had included a similar test case as the one above in my [PR](https://github.com/ani5rudh/commons-statistics/blob/846c6185330439a4bb7b1df25611dd3ddcd696ed/commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java#L105).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-statistics] aherbert commented on a diff in pull request #50: STATISTICS-77: Sum Implementation
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #50:
URL: https://github.com/apache/commons-statistics/pull/50#discussion_r1271396439
##########
commons-statistics-descriptive/pom.xml:
##########
@@ -56,6 +56,12 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-numbers-core</artifactId>
+ <version>1.2-SNAPSHOT</version>
Review Comment:
This module alone should not use a different version from the entire project. If you have different modules using different versions then a downstream project which transitively includes multiple modules from here may not correctly resolve to the one that this module requires. The change should be made to the property in the parent pom `<statistics.commons.numbers.version>`.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Sum.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+
+/**
+ * Returns the sum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.
+ *
+ * <p>The result is zero if no values are added.
+ *
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.
+ *
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.
+ *
+ * @since 1.1
+ */
+public abstract class Sum implements DoubleStatistic, DoubleStatisticAccumulator<Sum> {
+
+ /**
+ * Create a Sum instance.
+ */
+ Sum() {
+ //No-op
+ }
+
+ /**
+ * Creates a {@code Sum} implementation which does not store the input value(s) it consumes.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code> or the sum
+ * at any point is a <code>NaN</code>.
+ *
+ * <p>The result is zero if no values have been added.
+ *
+ * <p>Uses a {@link org.apache.commons.numbers.core.Sum Sum} delegate under the hood
Review Comment:
If you override the javadoc link (which drops the o.a.c.numbers text) then we should make it clear that the `Sum` is from the Numbers project.
`Uses the {@link org.apache.commons.numbers.core.Sum Commons Numbers Sum} implementation.`
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.commons.rng.UniformRandomProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Test for {@link Sum}.
+ */
+final class SumTest {
+
+ @Test
+ void testEmpty() {
+ Sum sum = Sum.create();
+ Assertions.assertEquals(0.0, sum.getAsDouble());
+ }
+
+ @Test
+ void testNan() {
+ Sum sum = Sum.create();
+ double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ for (double value : testArray) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(Double.NaN, sum.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSum(double[] values, double expected) {
+ Sum sum = Sum.create();
+ for (double value : values) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(expected, sum.getAsDouble(), "sum");
+ Assertions.assertEquals(expected, Sum.of(values).getAsDouble(), "of (values)");
+ }
+
+ static Stream<Arguments> testSum() {
Review Comment:
This could use some extended precision examples. This [one](https://en.wikipedia.org/wiki/Kahan_summation_algorithm#Further_enhancements) is useful:
```
1, 10e100, 1, -10e100 == 2
```
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.commons.rng.UniformRandomProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Test for {@link Sum}.
+ */
+final class SumTest {
+
+ @Test
+ void testEmpty() {
+ Sum sum = Sum.create();
+ Assertions.assertEquals(0.0, sum.getAsDouble());
+ }
+
+ @Test
+ void testNan() {
+ Sum sum = Sum.create();
+ double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ for (double value : testArray) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(Double.NaN, sum.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSum(double[] values, double expected) {
+ Sum sum = Sum.create();
+ for (double value : values) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(expected, sum.getAsDouble(), "sum");
+ Assertions.assertEquals(expected, Sum.of(values).getAsDouble(), "of (values)");
+ }
+
+ static Stream<Arguments> testSum() {
+ return Stream.of(
+ Arguments.of(new double[] {}, 0.0),
+ Arguments.of(new double[] {0, 0, 0.0}, 0.0),
+ Arguments.of(new double[] {1, -7, 6}, 0),
+ Arguments.of(new double[] {1, 7, -15, 3}, -4),
+ Arguments.of(new double[] {2, 2, 2, 2}, 8.0),
+ Arguments.of(new double[] {2.3}, 2.3),
+ Arguments.of(new double[] {3.14, 2.718, 1.414}, 7.272),
+ Arguments.of(new double[] {12.5, 12.0, 11.8, 14.2, 14.9, 14.5, 21.0,
+ 8.2, 10.3, 11.3, 14.1, 9.9, 12.2, 12.0, 12.1, 11.0, 19.8, 11.0, 10.0, 8.8,
+ 9.0, 12.3}, 272.9),
+ Arguments.of(new double[] {-0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.0, -0.0}, 0.0),
+ Arguments.of(new double[] {0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.001, 0.0002, 0.00003, 10000.11, 0.000004}, 10000.111234),
+ Arguments.of(new double[] {10E-50, 5E-100, 25E-200, 35.345E-50}, 45.345E-50),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NaN),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, -Double.MIN_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NaN, 34.56, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, Double.NaN, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, 89.74, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, 1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {Double.MAX_VALUE, -1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, -1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {1, 2, 3, Double.MAX_VALUE, -Double.MAX_VALUE, -7}, -1)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testParallelStream(double[] values, double expected) {
+ double ans = Arrays.stream(values)
+ .parallel()
+ .collect(Sum::create, Sum::accept, Sum::combine)
+ .getAsDouble();
+ Assertions.assertEquals(expected, ans);
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSumRandomOrder(double[] values, double expected) {
+ UniformRandomProvider rng = TestHelper.createRNG();
+ for (int i = 0; i < 10; i++) {
+ testSum(TestHelper.shuffle(rng, values), expected);
+ testParallelStream(TestHelper.shuffle(rng, values), expected);
+ testCombine(TestHelper.shuffle(rng, values), expected);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testCombine(double[] values, double expected) {
+ Sum sum1 = Sum.create();
+ Sum sum2 = Sum.create();
+ Arrays.stream(values).forEach(sum1);
+ Arrays.stream(values).forEach(sum2);
+ double sum2BeforeCombine = sum2.getAsDouble();
+ sum1.combine(sum2);
+ Assertions.assertEquals(expected * 2, sum1.getAsDouble(), 10E-12);
Review Comment:
The delta here may not always be suitable since it is absolute and not relative. For a quick relative test you can use:
```java
double eps = 1e-15; // for example
assertEquals(expected, actual, Math.abs(expected) * eps)
```
If you only expect a tiny difference then we can use Precision.equals with a ulp argument. This may be more relevant here.
Since you lose the error message containing the values from the assertion with doubles you have to provide it. As such this is useful to put into a method in the TestHelper class:
```java
static void assertEquals(double expected, double actual, int ulps, Supplier<String> msg) {
Assertions.assertTrue(Precision.equals(expected, actual, ulps),
() -> equals + " != " + actual + " within " + ulps + " ulps: " + msg.get());
}
```
Note: This is untested and could be improved to create an error message more consistent with JUnit (fixing the error message is not a priority).
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Sum.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+
+/**
+ * Returns the sum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.
+ *
+ * <p>The result is zero if no values are added.
+ *
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.
+ *
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.
+ *
+ * @since 1.1
+ */
+public abstract class Sum implements DoubleStatistic, DoubleStatisticAccumulator<Sum> {
+
+ /**
+ * Create a Sum instance.
+ */
+ Sum() {
+ //No-op
+ }
+
+ /**
+ * Creates a {@code Sum} implementation which does not store the input value(s) it consumes.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code> or the sum
+ * at any point is a <code>NaN</code>.
+ *
+ * <p>The result is zero if no values have been added.
+ *
+ * <p>Uses a {@link org.apache.commons.numbers.core.Sum Sum} delegate under the hood
+ * to update and compute the {@code sum}.
+ *
+ * @return {@code Sum} implementation.
+ */
+ public static Sum create() {
+ return new WrappedSum();
+ }
+
+ /**
+ * Returns a {@code Sum} instance that has the sum of all input value(s).
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>
+ * or the sum at any point is a <code>NaN</code>.
+ *
+ * <p>When the input is an empty array, the result is zero.
+ *
+ * @param values Values.
+ * @return {@code Sum} instance.
+ */
+ public static Sum of(double... values) {
+ final WrappedSum sum = new WrappedSum();
+ Arrays.stream(values).forEach(sum);
+ return sum;
+ }
+
+ /**
+ * Updates the state of the statistic to reflect the addition of {@code value}.
+ * @param value Value.
+ */
+ @Override
+ public abstract void accept(double value);
+
+ /**
+ * Gets the sum of all input values.
+ *
+ * <p>When no values have been added, the result is zero.
+ *
+ * @return {@code Sum} of all values seen so far.
+ */
+ @Override
+ public abstract double getAsDouble();
+
+ /** {@inheritDoc} */
+ @Override
+ public abstract Sum combine(Sum other);
+
+ /**
+ * {@code Sum} implementation that does not store the input value(s) processed so far.
+ *
+ * <p>Uses a {@link org.apache.commons.numbers.core.Sum Sum} delegate under the hood
Review Comment:
Simplify to `Delegates to the {@link org.apache.commons.numbers.core.Sum} implementation`.
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.commons.rng.UniformRandomProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Test for {@link Sum}.
+ */
+final class SumTest {
+
+ @Test
+ void testEmpty() {
+ Sum sum = Sum.create();
+ Assertions.assertEquals(0.0, sum.getAsDouble());
+ }
+
+ @Test
+ void testNan() {
+ Sum sum = Sum.create();
+ double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ for (double value : testArray) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(Double.NaN, sum.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSum(double[] values, double expected) {
+ Sum sum = Sum.create();
+ for (double value : values) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(expected, sum.getAsDouble(), "sum");
+ Assertions.assertEquals(expected, Sum.of(values).getAsDouble(), "of (values)");
+ }
+
+ static Stream<Arguments> testSum() {
+ return Stream.of(
+ Arguments.of(new double[] {}, 0.0),
+ Arguments.of(new double[] {0, 0, 0.0}, 0.0),
+ Arguments.of(new double[] {1, -7, 6}, 0),
+ Arguments.of(new double[] {1, 7, -15, 3}, -4),
+ Arguments.of(new double[] {2, 2, 2, 2}, 8.0),
+ Arguments.of(new double[] {2.3}, 2.3),
+ Arguments.of(new double[] {3.14, 2.718, 1.414}, 7.272),
+ Arguments.of(new double[] {12.5, 12.0, 11.8, 14.2, 14.9, 14.5, 21.0,
+ 8.2, 10.3, 11.3, 14.1, 9.9, 12.2, 12.0, 12.1, 11.0, 19.8, 11.0, 10.0, 8.8,
+ 9.0, 12.3}, 272.9),
+ Arguments.of(new double[] {-0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.0, -0.0}, 0.0),
+ Arguments.of(new double[] {0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.001, 0.0002, 0.00003, 10000.11, 0.000004}, 10000.111234),
+ Arguments.of(new double[] {10E-50, 5E-100, 25E-200, 35.345E-50}, 45.345E-50),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NaN),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, -Double.MIN_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NaN, 34.56, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, Double.NaN, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, 89.74, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, 1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {Double.MAX_VALUE, -1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, -1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {1, 2, 3, Double.MAX_VALUE, -Double.MAX_VALUE, -7}, -1)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testParallelStream(double[] values, double expected) {
+ double ans = Arrays.stream(values)
+ .parallel()
+ .collect(Sum::create, Sum::accept, Sum::combine)
+ .getAsDouble();
+ Assertions.assertEquals(expected, ans);
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSumRandomOrder(double[] values, double expected) {
+ UniformRandomProvider rng = TestHelper.createRNG();
+ for (int i = 0; i < 10; i++) {
+ testSum(TestHelper.shuffle(rng, values), expected);
+ testParallelStream(TestHelper.shuffle(rng, values), expected);
+ testCombine(TestHelper.shuffle(rng, values), expected);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testCombine(double[] values, double expected) {
+ Sum sum1 = Sum.create();
+ Sum sum2 = Sum.create();
+ Arrays.stream(values).forEach(sum1);
+ Arrays.stream(values).forEach(sum2);
+ double sum2BeforeCombine = sum2.getAsDouble();
+ sum1.combine(sum2);
+ Assertions.assertEquals(expected * 2, sum1.getAsDouble(), 10E-12);
+ Assertions.assertEquals(sum2BeforeCombine, sum2.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ void testArrayOfArrays(double[][] arr, double expected) {
+ double actual = Arrays.stream(arr)
+ .map(Sum::of)
+ .reduce(Sum::combine)
+ .map(Sum::getAsDouble)
+ .orElseThrow(RuntimeException::new);
+ Assertions.assertEquals(expected, actual, 10E-12);
Review Comment:
Reuse the TestHelper assertEquals
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.commons.rng.UniformRandomProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Test for {@link Sum}.
+ */
+final class SumTest {
+
+ @Test
+ void testEmpty() {
+ Sum sum = Sum.create();
+ Assertions.assertEquals(0.0, sum.getAsDouble());
+ }
+
+ @Test
+ void testNan() {
+ Sum sum = Sum.create();
+ double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ for (double value : testArray) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(Double.NaN, sum.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSum(double[] values, double expected) {
+ Sum sum = Sum.create();
+ for (double value : values) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(expected, sum.getAsDouble(), "sum");
+ Assertions.assertEquals(expected, Sum.of(values).getAsDouble(), "of (values)");
+ }
+
+ static Stream<Arguments> testSum() {
+ return Stream.of(
+ Arguments.of(new double[] {}, 0.0),
+ Arguments.of(new double[] {0, 0, 0.0}, 0.0),
+ Arguments.of(new double[] {1, -7, 6}, 0),
+ Arguments.of(new double[] {1, 7, -15, 3}, -4),
+ Arguments.of(new double[] {2, 2, 2, 2}, 8.0),
+ Arguments.of(new double[] {2.3}, 2.3),
+ Arguments.of(new double[] {3.14, 2.718, 1.414}, 7.272),
+ Arguments.of(new double[] {12.5, 12.0, 11.8, 14.2, 14.9, 14.5, 21.0,
+ 8.2, 10.3, 11.3, 14.1, 9.9, 12.2, 12.0, 12.1, 11.0, 19.8, 11.0, 10.0, 8.8,
+ 9.0, 12.3}, 272.9),
+ Arguments.of(new double[] {-0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.0, -0.0}, 0.0),
+ Arguments.of(new double[] {0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.001, 0.0002, 0.00003, 10000.11, 0.000004}, 10000.111234),
+ Arguments.of(new double[] {10E-50, 5E-100, 25E-200, 35.345E-50}, 45.345E-50),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NaN),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, -Double.MIN_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NaN, 34.56, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, Double.NaN, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, 89.74, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, 1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {Double.MAX_VALUE, -1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, -1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {1, 2, 3, Double.MAX_VALUE, -Double.MAX_VALUE, -7}, -1)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testParallelStream(double[] values, double expected) {
+ double ans = Arrays.stream(values)
+ .parallel()
+ .collect(Sum::create, Sum::accept, Sum::combine)
+ .getAsDouble();
+ Assertions.assertEquals(expected, ans);
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSumRandomOrder(double[] values, double expected) {
+ UniformRandomProvider rng = TestHelper.createRNG();
+ for (int i = 0; i < 10; i++) {
+ testSum(TestHelper.shuffle(rng, values), expected);
+ testParallelStream(TestHelper.shuffle(rng, values), expected);
+ testCombine(TestHelper.shuffle(rng, values), expected);
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testCombine(double[] values, double expected) {
+ Sum sum1 = Sum.create();
+ Sum sum2 = Sum.create();
+ Arrays.stream(values).forEach(sum1);
+ Arrays.stream(values).forEach(sum2);
+ double sum2BeforeCombine = sum2.getAsDouble();
+ sum1.combine(sum2);
+ Assertions.assertEquals(expected * 2, sum1.getAsDouble(), 10E-12);
+ Assertions.assertEquals(sum2BeforeCombine, sum2.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ void testArrayOfArrays(double[][] arr, double expected) {
+ double actual = Arrays.stream(arr)
+ .map(Sum::of)
+ .reduce(Sum::combine)
+ .map(Sum::getAsDouble)
+ .orElseThrow(RuntimeException::new);
+ Assertions.assertEquals(expected, actual, 10E-12);
+ }
+
+ static Stream<Arguments> testArrayOfArrays() {
Review Comment:
Add the {1, 10e100, 1, -10e100} example split into two arrays.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-statistics] codecov-commenter commented on pull request #50: STATISTICS-77: Sum Implementation
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #50:
URL: https://github.com/apache/commons-statistics/pull/50#issuecomment-1646767909
## [Codecov](https://app.codecov.io/gh/apache/commons-statistics/pull/50?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#50](https://app.codecov.io/gh/apache/commons-statistics/pull/50?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (846c618) into [master](https://app.codecov.io/gh/apache/commons-statistics/commit/b69d7ad0f2429395fed3971603479855450e4a7e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b69d7ad) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #50 +/- ##
=========================================
Coverage 99.82% 99.82%
Complexity 1688 1688
=========================================
Files 65 65
Lines 4472 4472
Branches 756 756
=========================================
Hits 4464 4464
Misses 4 4
Partials 4 4
```
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-statistics] aherbert commented on a diff in pull request #50: STATISTICS-77: Sum Implementation
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on code in PR #50:
URL: https://github.com/apache/commons-statistics/pull/50#discussion_r1272520892
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -136,7 +141,7 @@ void testCombine(double[] values, double expected) {
Arrays.stream(values).forEach(sum2);
double sum2BeforeCombine = sum2.getAsDouble();
sum1.combine(sum2);
- Assertions.assertEquals(expected * 2, sum1.getAsDouble(), 10E-12);
+ TestHelper.assertEquals(expected * 2, sum1.getAsDouble(), Math.abs(expected) * EPS, () -> EPS_STRING);
Review Comment:
This is not using a ulp tolerance. It should read something like:
```Java
// Relative equality using JUnit assert. This could be moved to a TestHelper method accepting eps for reuse.
Assertions.assertEquals(expected * 2, sum1.getAsDouble(), Math.abs(expected) * EPS, () -> EPS_STRING);
// Or using a ULP threshold
TestHelper.assertEquals(expected * 2, sum1.getAsDouble(), 1, () -> "combined sum");
```
I think the ULP test is what we wish to use here. 1 ULP may be too low. But it should pass with a few (e.g. 1 ULP is relative eps 2^-52 ~ 2.2e-16).
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Sum.java:
##########
@@ -109,8 +108,7 @@ public static Sum of(double... values) {
/**
* {@code Sum} implementation that does not store the input value(s) processed so far.
*
- * <p>Uses a {@link org.apache.commons.numbers.core.Sum Sum} delegate under the hood
- * to update and compute the {@code sum}.
+ * <p>Delegates to the {@link org.apache.commons.numbers.core.Sum} implementation.
Review Comment:
You have an extra space before `{@link`
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/SumTest.java:
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import java.util.Arrays;
+import java.util.stream.Stream;
+import org.apache.commons.rng.UniformRandomProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Test for {@link Sum}.
+ */
+final class SumTest {
+
+ private static final double EPS = 10E-12;
+
+ private static final String EPS_STRING = String.valueOf(EPS);
+
+ @Test
+ void testEmpty() {
+ Sum sum = Sum.create();
+ Assertions.assertEquals(0.0, sum.getAsDouble());
+ }
+
+ @Test
+ void testNan() {
+ Sum sum = Sum.create();
+ double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ for (double value : testArray) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(Double.NaN, sum.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource(value = "testSum")
+ void testSum(double[] values, double expected) {
+ Sum sum = Sum.create();
+ for (double value : values) {
+ sum.accept(value);
+ }
+ Assertions.assertEquals(expected, sum.getAsDouble(), "sum");
+ Assertions.assertEquals(expected, Sum.of(values).getAsDouble(), "of (values)");
+ }
+
+ static Stream<Arguments> testSum() {
+ return Stream.of(
+ Arguments.of(new double[] {}, 0.0),
+ Arguments.of(new double[] {0, 0, 0.0}, 0.0),
+ Arguments.of(new double[] {1, -7, 6}, 0),
+ Arguments.of(new double[] {1, 7, -15, 3}, -4),
+ Arguments.of(new double[] {2, 2, 2, 2}, 8.0),
+ Arguments.of(new double[] {2.3}, 2.3),
+ Arguments.of(new double[] {3.14, 2.718, 1.414}, 7.272),
+ Arguments.of(new double[] {12.5, 12.0, 11.8, 14.2, 14.9, 14.5, 21.0,
+ 8.2, 10.3, 11.3, 14.1, 9.9, 12.2, 12.0, 12.1, 11.0, 19.8, 11.0, 10.0, 8.8,
+ 9.0, 12.3}, 272.9),
+ Arguments.of(new double[] {-0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.0, -0.0}, 0.0),
+ Arguments.of(new double[] {0.0, +0.0}, 0.0),
+ Arguments.of(new double[] {0.001, 0.0002, 0.00003, 10000.11, 0.000004}, 10000.111234),
+ Arguments.of(new double[] {10E-50, 5E-100, 25E-200, 35.345E-50}, 45.345E-50),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NaN),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.NEGATIVE_INFINITY},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, Double.MAX_VALUE},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, Double.POSITIVE_INFINITY},
+ Double.POSITIVE_INFINITY),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, -Double.MIN_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.NaN, 34.56, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, Double.NaN, 89.74}, Double.NaN),
+ Arguments.of(new double[] {34.56, 89.74, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NaN, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {Double.NEGATIVE_INFINITY, Double.MAX_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {Double.MAX_VALUE, 1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, -1, 1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {Double.MAX_VALUE, -1}, Double.MAX_VALUE),
+ Arguments.of(new double[] {-Double.MAX_VALUE, 1, -1}, -Double.MAX_VALUE),
+ Arguments.of(new double[] {1, 2, 3, Double.MAX_VALUE, -Double.MAX_VALUE, -7}, -1),
Review Comment:
Can you put this large cancellation test case in the array-of-arrays test as well, making sure the +/-MAX_VALUE is separated into the two arrays.
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/TestHelper.java:
##########
@@ -68,4 +71,9 @@ private static void swap(double[] array, int i, int j) {
array[i] = array[j];
array[j] = tmp;
}
+
+ static void assertEquals(double expected, double actual, double ulps, Supplier<String> msg) {
Review Comment:
`double ulps` should be `int ulps`.
Here you are comparing the two double within an absolute floating-point delta. We require to compare the two doubles within units-of-least precision. This is how many possible double values there are between them. It is used for very close numbers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-statistics] kinow commented on a diff in pull request #50: STATISTICS-77: Sum Implementation
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #50:
URL: https://github.com/apache/commons-statistics/pull/50#discussion_r1271398064
##########
commons-statistics-descriptive/pom.xml:
##########
@@ -56,6 +56,12 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-numbers-core</artifactId>
+ <version>1.2-SNAPSHOT</version>
Review Comment:
Add it as a `<property>` (above), perhaps?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org