aherbert commented on code in PR #51: URL: https://github.com/apache/commons-statistics/pull/51#discussion_r1283180578
########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/FirstMoment.java: ########## @@ -0,0 +1,168 @@ +/* + * 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; + +/** + * Computes the first moment (arithmetic mean) using the definitional formula: + * + * <p> mean = sum(x_i) / n + * + * <p> To limit numeric errors, the value of the statistic is computed using the + * following recursive updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>Returns <code>Double.NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes NaN and / or infinite + * values of opposite sign. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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. + */ +class FirstMoment implements DoubleStatistic, DoubleStatisticAccumulator<FirstMoment> { + /** Count of values that have been added. */ + private long n; + + /** First moment of values that have been added. */ + private double m1; + + /** Running count of <code>Double.POSITIVE_INFINITY</code> values. */ + private long posInfCount; + + /** Running count of <code>Double.NEGATIVE_INFINITY</code> values. */ + private long negInfCount; + + /** + * Deviation of most recently added value from the previous first moment. + * Retained to prevent repeated computation in higher order moments. + */ + private double dev; + + /** + * Deviation of most recently added value from the previous first moment, + * normalized by current sample size. Retained to prevent repeated + * computation in higher order moments + */ + private double nDev; + + /** + * Create a FirstMoment instance. + */ + FirstMoment() { + n = 0; + posInfCount = 0; + negInfCount = 0; + dev = 0.0; + nDev = 0.0; + m1 = Double.NaN; + } + + /** + * Create a FirstMoment instance with the given first moment and number of values. + * @param m1 First moment. + * @param n Number of values. + */ + FirstMoment(final double m1, final long n) { + this.m1 = m1; + this.n = n; + } + + /** + * Updates the state of the statistic to reflect the addition of {@code value}. + * @param value Value. + */ + @Override + public void accept(double value) { + if (n == 0) { Review Comment: I think we should remove this branch. Simply initialise m1 as 0.0. The logic to return NaN when n=0 can be added in the getAsDouble method. ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Mean.java: ########## @@ -0,0 +1,195 @@ +/* + * 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; + +/** + * Computes the arithmetic mean of a set of values. Uses the following recursive + * updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>If {@link #of(double...)} is used to compute the mean of variable number Review Comment: `of a variable` ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/FirstMoment.java: ########## @@ -0,0 +1,168 @@ +/* + * 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; + +/** + * Computes the first moment (arithmetic mean) using the definitional formula: + * + * <p> mean = sum(x_i) / n + * + * <p> To limit numeric errors, the value of the statistic is computed using the + * following recursive updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>Returns <code>Double.NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes NaN and / or infinite + * values of opposite sign. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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. + */ +class FirstMoment implements DoubleStatistic, DoubleStatisticAccumulator<FirstMoment> { + /** Count of values that have been added. */ + private long n; + + /** First moment of values that have been added. */ + private double m1; + + /** Running count of <code>Double.POSITIVE_INFINITY</code> values. */ + private long posInfCount; + + /** Running count of <code>Double.NEGATIVE_INFINITY</code> values. */ + private long negInfCount; + + /** + * Deviation of most recently added value from the previous first moment. + * Retained to prevent repeated computation in higher order moments. + */ + private double dev; + + /** + * Deviation of most recently added value from the previous first moment, + * normalized by current sample size. Retained to prevent repeated + * computation in higher order moments + */ + private double nDev; + + /** + * Create a FirstMoment instance. + */ + FirstMoment() { + n = 0; + posInfCount = 0; + negInfCount = 0; + dev = 0.0; + nDev = 0.0; + m1 = Double.NaN; + } + + /** + * Create a FirstMoment instance with the given first moment and number of values. + * @param m1 First moment. + * @param n Number of values. + */ + FirstMoment(final double m1, final long n) { Review Comment: This will require the `double sum` argument as well to support my modification. It may not matter as the sum is only used a flag for non-finite input values and this constructor is used when the two-pass algorithm created a finite result. ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Mean.java: ########## @@ -0,0 +1,195 @@ +/* + * 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; + +/** + * Computes the arithmetic mean of a set of values. Uses the following recursive + * updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>If {@link #of(double...)} is used to compute the mean of variable number + * of values, a two-pass, corrected algorithm is used, starting with + * the recursive updating algorithm mentioned above, which protects the mean from overflow, + * and then correcting this by adding the mean deviation of the data values from the + * arithmetic mean. See, e.g. "Comparison of Several Algorithms for Computing + * Sample Means and Variances," Robert F. Ling, Journal of the American + * Statistical Association, Vol. 69, No. 348 (Dec., 1974), pp. 859-866. + * + * <p>Returns <code>NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes <code>NaN</code> and / or infinite + * values of opposite sign. + * + * <p>This class is designed to work with (though does not require) + * {@linkplain java.util.stream streams}. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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 Mean implements DoubleStatistic, DoubleStatisticAccumulator<Mean> { + + /** + * Create a Mean instance. + */ + Mean() { + // No-op + } + + /** + * Creates a {@code Mean} 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 + * if no values have been added. + * + * @return {@code Mean} implementation. + */ + public static Mean create() { + return new StorelessMean(); + } + + /** + * Returns a {@code Mean} instance that has the arithmetic mean of all input values, or <code>NaN</code> + * if the input array is empty. + * + * <p>Note: {@code Mean} computed using {@link Mean#accept Mean.accept()} may be different + * from this mean. + * + * <p>See {@link Mean} for details on the computing algorithm. + * + * @param values Values. + * @return {@code Mean} instance. + */ + public static Mean of(double... values) { + final Mean mean = Statistics.add(new StorelessMean(), values); + + if (!Double.isFinite(mean.getAsDouble())) { + return mean; + } + + final double xbar = mean.getAsDouble(); + double correction = 0; + + for (final double value : values) { + correction += value - xbar; + } + + return StorelessMean.create(xbar + (correction / values.length), mean.getN()); + } + + /** + * 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 mean of all input values. + * + * <p>When no values have been added, the result is <code>NaN</code>. + * + * @return {@code Mean} of all values seen so far. + */ + @Override + public abstract double getAsDouble(); + + /** {@inheritDoc} */ + @Override + public abstract Mean combine(Mean other); + + /** + * Returns the number of values that have been added. + * @return Number of values. + */ + public abstract long getN(); + + /** + * {@code Mean} implementation that does not store the input value(s) processed so far. + */ + private static class StorelessMean extends Mean { + + /** + * External Moment used to compute the mean. + */ + private FirstMoment firstMoment; + + /** + * Creates a StorelessMean instance with an External Moment. + * + * @param m1 First moment. + * @param n Number of values. + */ + private StorelessMean(final double m1, final long n) { + firstMoment = new FirstMoment(m1, n); + } + + /** + * Create a Mean instance. + */ + StorelessMean() { + firstMoment = new FirstMoment(); + } + + + /** + * Creates a StorelessMean instance with an External Moment. + * + * @param m1 First moment. + * @param n Number of values. + * @return A StorelessMean instance. + */ + static StorelessMean create(final double m1, final long n) { + return new StorelessMean(m1, n); + } + + @Override + public void accept(double value) { + firstMoment.accept(value); + } + + @Override + public double getAsDouble() { + return firstMoment.getAsDouble(); + } + + @Override + public Mean combine(Mean other) { + final StorelessMean that = (StorelessMean) other; + firstMoment = firstMoment.combine(that.firstMoment); Review Comment: Since we have created firstMomemt we know that the combine here will return a reference to the left-hand argument. As such we do not have to assign the result and the firstMoment can be final. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MeanTest.java: ########## @@ -0,0 +1,229 @@ +/* + * 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 Mean}. + */ +final class MeanTest { + + @Test + void testEmpty() { + Mean mean = Mean.create(); + Assertions.assertEquals(Double.NaN, mean.getAsDouble()); + } + + @Test + void testNan() { + Mean mean = Mean.create(); + double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + for (double value : testArray) { + mean.accept(value); + } + Assertions.assertEquals(Double.NaN, mean.getAsDouble()); + } + + @ParameterizedTest + @MethodSource(value = {"testMean"}) + void testMean(double[] values, double expected) { + Mean mean = Mean.create(); + for (double value : values) { + mean.accept(value); + } + TestHelper.assertEquals(expected, mean.getAsDouble(), 5, () -> "mean"); + TestHelper.assertEquals(expected, Mean.of(values).getAsDouble(), 5, () -> "of (values)"); + } + + static Stream<Arguments> testMean() { + return Stream.of( + Arguments.of(new double[] {10, 8, 13, 9, 11, 14, 6, 4, 12, 7, 5}, 9), + Arguments.of(new double[] {8.04, 6.95, 7.58, 8.81, 8.33, 9.96, 7.24, 4.26, 10.84, 4.82, 5.68}, 7.500909090909092), + Arguments.of(new double[] {9.14, 8.14, 8.74, 8.77, 9.26, 8.10, 6.13, 3.10, 9.13, 7.26, 4.74}, 7.500909090909092), + Arguments.of(new double[] {7.46, 6.77, 12.74, 7.11, 7.81, 8.84, 6.08, 5.39, 8.15, 6.42, 5.73}, 7.5), + Arguments.of(new double[] {8, 8, 8, 8, 8, 8, 8, 19, 8, 8, 8}, 9), + Arguments.of(new double[] {6.58, 5.76, 7.71, 8.84, 8.47, 7.04, 5.25, 12.50, 5.56, 7.91, 6.89}, 7.500909090909092), + Arguments.of(new double[]{}, Double.NaN), + 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}, -1), + Arguments.of(new double[] {2, 2, 2, 2}, 2.0), + Arguments.of(new double[] {2.3}, 2.3), + Arguments.of(new double[] {3.14, 2.718, 1.414}, 2.424), + 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}, 12.404545454545454), + 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}, + 2000.0222468), + Arguments.of(new double[] {10E-50, 5E-100, 25E-200, 35.345E-50}, + 1.133625E-49), + 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.MAX_VALUE), + Arguments.of(new double[] {-Double.MAX_VALUE, Double.POSITIVE_INFINITY}, + Double.POSITIVE_INFINITY), + Arguments.of(new double[] {-Double.MAX_VALUE, -Double.MAX_VALUE}, + -Double.MAX_VALUE), + 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 / 2), + Arguments.of(new double[] {-Double.MAX_VALUE, 1, 1}, -Double.MAX_VALUE / 3), + Arguments.of(new double[] {-Double.MAX_VALUE, -1, 1}, -Double.MAX_VALUE / 3), + Arguments.of(new double[] {Double.MAX_VALUE, -1}, Double.MAX_VALUE / 2), + Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY, + Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY}, Double.POSITIVE_INFINITY) + ); + } + + @ParameterizedTest + @MethodSource(value = "testMean") + void testParallelStream(double[] values, double expected) { + double ans = Arrays.stream(values) + .parallel() + .collect(Mean::create, Mean::accept, Mean::combine) + .getAsDouble(); + + TestHelper.assertEquals(expected, ans, 5, () -> "parallel stream"); + } + + @ParameterizedTest + @MethodSource(value = "testMean") + void testMeanRandomOrder(double[] values, double expected) { + UniformRandomProvider rng = TestHelper.createRNG(); + for (int i = 1; i <= 10; i++) { + testMean(TestHelper.shuffle(rng, values), expected); + testParallelStream(TestHelper.shuffle(rng, values), expected); + } + } + + @ParameterizedTest + @MethodSource(value = {"testCombine"}) + void testCombine(double[][] values, double expected) { + Mean mean1 = Mean.create(); + Mean mean2 = Mean.create(); + Arrays.stream(values[0]).forEach(mean1); + Arrays.stream(values[1]).forEach(mean2); + double mean2BeforeCombine = mean2.getAsDouble(); + mean1.combine(mean2); + TestHelper.assertEquals(expected, mean1.getAsDouble(), 10, () -> "combine"); + Assertions.assertEquals(mean2BeforeCombine, mean2.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[][] {{}, {}}, Double.NaN), + Arguments.of(new double[][] {{}, {1}}, 1), + Arguments.of(new double[][] {{1}, {}}, 1), + Arguments.of(new double[][] {{}, {1, 7, -15, 3}}, -1), + Arguments.of(new double[][] {{0}, {0, 0.0}}, 0.0), + Arguments.of(new double[][] {{4, 8, -6, 3, 18}, {1, -7, 6}}, 3.375), + Arguments.of(new double[][] {{10, 8, 13, 9, 11, 14, 6, 4, 12, 7, 5}, {8, 8, 8, 8, 8, 8, 8, 19, 8, 8, 8}}, 9), + Arguments.of(new double[][] {{10, 8, 13, 9, 11, 14, 6, 4, 12, 7, 5}, {7.46, 6.77, 12.74, 7.11, 7.81, 8.84, 6.08, 5.39, 8.15, 6.42, 5.73}}, 8.25), + Arguments.of(new double[][] {{6.0, -1.32, -5.78, 8.967, 13.32, -9.67, 0.14, 7.321, 11.456, -3.111}, {2, 2, 2, 2}}, 2.52307142857142857), Review Comment: These high accuracy values should have a note on how they were computed. E.g. state that the values were computed using an extended precision library (Matlab vpa; Maxima; Python mpmath; Java BigDecimal). In the case of BigDecimal you could just code use the definitional formula and compute the value dynamically. ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/FirstMoment.java: ########## @@ -0,0 +1,168 @@ +/* + * 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; + +/** + * Computes the first moment (arithmetic mean) using the definitional formula: + * + * <p> mean = sum(x_i) / n + * + * <p> To limit numeric errors, the value of the statistic is computed using the + * following recursive updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>Returns <code>Double.NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes NaN and / or infinite + * values of opposite sign. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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. + */ +class FirstMoment implements DoubleStatistic, DoubleStatisticAccumulator<FirstMoment> { + /** Count of values that have been added. */ + private long n; + + /** First moment of values that have been added. */ + private double m1; + + /** Running count of <code>Double.POSITIVE_INFINITY</code> values. */ + private long posInfCount; + + /** Running count of <code>Double.NEGATIVE_INFINITY</code> values. */ + private long negInfCount; + + /** + * Deviation of most recently added value from the previous first moment. + * Retained to prevent repeated computation in higher order moments. + */ + private double dev; + + /** + * Deviation of most recently added value from the previous first moment, + * normalized by current sample size. Retained to prevent repeated + * computation in higher order moments + */ + private double nDev; + + /** + * Create a FirstMoment instance. + */ + FirstMoment() { + n = 0; + posInfCount = 0; + negInfCount = 0; + dev = 0.0; + nDev = 0.0; + m1 = Double.NaN; + } + + /** + * Create a FirstMoment instance with the given first moment and number of values. + * @param m1 First moment. + * @param n Number of values. + */ + FirstMoment(final double m1, final long n) { + this.m1 = m1; + this.n = n; + } + + /** + * Updates the state of the statistic to reflect the addition of {@code value}. + * @param value Value. + */ + @Override + public void accept(double value) { + if (n == 0) { + m1 = 0.0; + } + n++; + if (value == Double.POSITIVE_INFINITY) { + posInfCount++; + } else if (value == Double.NEGATIVE_INFINITY) { + negInfCount++; + } else { + //nDev is computed in this manner to prevent overflow. + nDev = (value / n) - (m1 / n); + dev = nDev * n; + m1 += nDev; + } + } + + /** + * Gets the first moment of all input values. + * + * <p>When no values have been added, the result is <code>NaN</code>. + * + * @return {@code First moment} of all values seen so far. + */ + @Override + public double getAsDouble() { + if (posInfCount == 0 && negInfCount == 0) { + return m1; + } else if (posInfCount == 0) { + return Double.NEGATIVE_INFINITY; + } else if (negInfCount == 0) { + return Double.POSITIVE_INFINITY; + } else { + return Double.NaN; + } + } + + /** {@inheritDoc} */ + @Override + public FirstMoment combine(FirstMoment other) { + if (n == 0) { + dev = other.dev; + nDev = other.nDev; + m1 = other.m1; + } else if (other.n == 0) { + return this; + } + + n += other.n; + if (other.posInfCount > 0) { + posInfCount += other.posInfCount; + } else if (other.negInfCount > 0) { + negInfCount += other.negInfCount; + } else { + dev = other.m1 - m1; + nDev = dev * ((double) other.n / n); + m1 += nDev; + } + return this; + } + + /** + * @return Number of values seen so far. + */ + public long getN() { Review Comment: This need not be public ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Mean.java: ########## @@ -0,0 +1,195 @@ +/* + * 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; + +/** + * Computes the arithmetic mean of a set of values. Uses the following recursive + * updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>If {@link #of(double...)} is used to compute the mean of variable number + * of values, a two-pass, corrected algorithm is used, starting with + * the recursive updating algorithm mentioned above, which protects the mean from overflow, + * and then correcting this by adding the mean deviation of the data values from the + * arithmetic mean. See, e.g. "Comparison of Several Algorithms for Computing + * Sample Means and Variances," Robert F. Ling, Journal of the American + * Statistical Association, Vol. 69, No. 348 (Dec., 1974), pp. 859-866. + * + * <p>Returns <code>NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes <code>NaN</code> and / or infinite + * values of opposite sign. + * + * <p>This class is designed to work with (though does not require) + * {@linkplain java.util.stream streams}. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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 Mean implements DoubleStatistic, DoubleStatisticAccumulator<Mean> { + + /** + * Create a Mean instance. + */ + Mean() { + // No-op + } + + /** + * Creates a {@code Mean} 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 + * if no values have been added. + * + * @return {@code Mean} implementation. + */ + public static Mean create() { + return new StorelessMean(); + } + + /** + * Returns a {@code Mean} instance that has the arithmetic mean of all input values, or <code>NaN</code> + * if the input array is empty. + * + * <p>Note: {@code Mean} computed using {@link Mean#accept Mean.accept()} may be different + * from this mean. + * + * <p>See {@link Mean} for details on the computing algorithm. + * + * @param values Values. + * @return {@code Mean} instance. + */ + public static Mean of(double... values) { + final Mean mean = Statistics.add(new StorelessMean(), values); + + if (!Double.isFinite(mean.getAsDouble())) { + return mean; + } + + final double xbar = mean.getAsDouble(); + double correction = 0; + + for (final double value : values) { + correction += value - xbar; + } + + return StorelessMean.create(xbar + (correction / values.length), mean.getN()); + } + + /** + * 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 mean of all input values. + * + * <p>When no values have been added, the result is <code>NaN</code>. + * + * @return {@code Mean} of all values seen so far. + */ + @Override + public abstract double getAsDouble(); + + /** {@inheritDoc} */ + @Override + public abstract Mean combine(Mean other); + + /** + * Returns the number of values that have been added. + * @return Number of values. + */ + public abstract long getN(); + + /** + * {@code Mean} implementation that does not store the input value(s) processed so far. + */ + private static class StorelessMean extends Mean { + + /** + * External Moment used to compute the mean. + */ + private FirstMoment firstMoment; Review Comment: I think this should be final. It allows JVM optimisation. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MeanTest.java: ########## @@ -0,0 +1,229 @@ +/* + * 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 Mean}. + */ +final class MeanTest { + + @Test + void testEmpty() { + Mean mean = Mean.create(); + Assertions.assertEquals(Double.NaN, mean.getAsDouble()); + } + + @Test + void testNan() { + Mean mean = Mean.create(); + double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + for (double value : testArray) { + mean.accept(value); + } + Assertions.assertEquals(Double.NaN, mean.getAsDouble()); + } + + @ParameterizedTest + @MethodSource(value = {"testMean"}) + void testMean(double[] values, double expected) { + Mean mean = Mean.create(); + for (double value : values) { + mean.accept(value); + } + TestHelper.assertEquals(expected, mean.getAsDouble(), 5, () -> "mean"); + TestHelper.assertEquals(expected, Mean.of(values).getAsDouble(), 5, () -> "of (values)"); + } + + static Stream<Arguments> testMean() { + return Stream.of( + Arguments.of(new double[] {10, 8, 13, 9, 11, 14, 6, 4, 12, 7, 5}, 9), + Arguments.of(new double[] {8.04, 6.95, 7.58, 8.81, 8.33, 9.96, 7.24, 4.26, 10.84, 4.82, 5.68}, 7.500909090909092), + Arguments.of(new double[] {9.14, 8.14, 8.74, 8.77, 9.26, 8.10, 6.13, 3.10, 9.13, 7.26, 4.74}, 7.500909090909092), + Arguments.of(new double[] {7.46, 6.77, 12.74, 7.11, 7.81, 8.84, 6.08, 5.39, 8.15, 6.42, 5.73}, 7.5), + Arguments.of(new double[] {8, 8, 8, 8, 8, 8, 8, 19, 8, 8, 8}, 9), + Arguments.of(new double[] {6.58, 5.76, 7.71, 8.84, 8.47, 7.04, 5.25, 12.50, 5.56, 7.91, 6.89}, 7.500909090909092), + Arguments.of(new double[]{}, Double.NaN), + 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}, -1), + Arguments.of(new double[] {2, 2, 2, 2}, 2.0), + Arguments.of(new double[] {2.3}, 2.3), + Arguments.of(new double[] {3.14, 2.718, 1.414}, 2.424), + 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}, 12.404545454545454), + 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}, + 2000.0222468), + Arguments.of(new double[] {10E-50, 5E-100, 25E-200, 35.345E-50}, + 1.133625E-49), + 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.MAX_VALUE), + Arguments.of(new double[] {-Double.MAX_VALUE, Double.POSITIVE_INFINITY}, + Double.POSITIVE_INFINITY), + Arguments.of(new double[] {-Double.MAX_VALUE, -Double.MAX_VALUE}, + -Double.MAX_VALUE), + 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 / 2), + Arguments.of(new double[] {-Double.MAX_VALUE, 1, 1}, -Double.MAX_VALUE / 3), + Arguments.of(new double[] {-Double.MAX_VALUE, -1, 1}, -Double.MAX_VALUE / 3), + Arguments.of(new double[] {Double.MAX_VALUE, -1}, Double.MAX_VALUE / 2), + Arguments.of(new double[] {Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY, + Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY}, Double.POSITIVE_INFINITY) + ); + } + + @ParameterizedTest + @MethodSource(value = "testMean") + void testParallelStream(double[] values, double expected) { + double ans = Arrays.stream(values) + .parallel() + .collect(Mean::create, Mean::accept, Mean::combine) + .getAsDouble(); + + TestHelper.assertEquals(expected, ans, 5, () -> "parallel stream"); + } + + @ParameterizedTest + @MethodSource(value = "testMean") + void testMeanRandomOrder(double[] values, double expected) { + UniformRandomProvider rng = TestHelper.createRNG(); + for (int i = 1; i <= 10; i++) { + testMean(TestHelper.shuffle(rng, values), expected); + testParallelStream(TestHelper.shuffle(rng, values), expected); + } + } + + @ParameterizedTest + @MethodSource(value = {"testCombine"}) + void testCombine(double[][] values, double expected) { + Mean mean1 = Mean.create(); + Mean mean2 = Mean.create(); + Arrays.stream(values[0]).forEach(mean1); + Arrays.stream(values[1]).forEach(mean2); + double mean2BeforeCombine = mean2.getAsDouble(); + mean1.combine(mean2); + TestHelper.assertEquals(expected, mean1.getAsDouble(), 10, () -> "combine"); + Assertions.assertEquals(mean2BeforeCombine, mean2.getAsDouble()); + } + + static Stream<Arguments> testCombine() { Review Comment: I think this case is missing where you can get overflow in the combine when computing the deviation from the mean: ```java Arguments.of(new double[][] {{Double.MAX_VALUE}, {-Double.MAX_VALUE}}, 0), Arguments.of(new double[][] {{1}, {-Double.MAX_VALUE}}, -Double.MAX_VALUE / 2), Arguments.of(new double[][] {{1, 1, 1}, {-Double.MAX_VALUE}}, -Double.MAX_VALUE / 4), ``` ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/FirstMoment.java: ########## @@ -0,0 +1,168 @@ +/* + * 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; + +/** + * Computes the first moment (arithmetic mean) using the definitional formula: + * + * <p> mean = sum(x_i) / n + * + * <p> To limit numeric errors, the value of the statistic is computed using the + * following recursive updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>Returns <code>Double.NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes NaN and / or infinite + * values of opposite sign. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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. + */ +class FirstMoment implements DoubleStatistic, DoubleStatisticAccumulator<FirstMoment> { + /** Count of values that have been added. */ + private long n; + + /** First moment of values that have been added. */ + private double m1; + + /** Running count of <code>Double.POSITIVE_INFINITY</code> values. */ + private long posInfCount; + + /** Running count of <code>Double.NEGATIVE_INFINITY</code> values. */ + private long negInfCount; + + /** + * Deviation of most recently added value from the previous first moment. + * Retained to prevent repeated computation in higher order moments. + */ + private double dev; + + /** + * Deviation of most recently added value from the previous first moment, + * normalized by current sample size. Retained to prevent repeated + * computation in higher order moments + */ + private double nDev; + + /** + * Create a FirstMoment instance. + */ + FirstMoment() { + n = 0; + posInfCount = 0; + negInfCount = 0; + dev = 0.0; + nDev = 0.0; + m1 = Double.NaN; + } + + /** + * Create a FirstMoment instance with the given first moment and number of values. + * @param m1 First moment. + * @param n Number of values. + */ + FirstMoment(final double m1, final long n) { + this.m1 = m1; + this.n = n; + } + + /** + * Updates the state of the statistic to reflect the addition of {@code value}. + * @param value Value. + */ + @Override + public void accept(double value) { + if (n == 0) { + m1 = 0.0; + } + n++; + if (value == Double.POSITIVE_INFINITY) { + posInfCount++; + } else if (value == Double.NEGATIVE_INFINITY) { + negInfCount++; + } else { + //nDev is computed in this manner to prevent overflow. + nDev = (value / n) - (m1 / n); Review Comment: The division by n here is inexact and will compromise the accuracy for the main use case. It only prevents overflow if the value to be added is greater than Double.MAX_VALUE from the mean. This is an extreme case. If you wish to prevent overflow then multiply each value by 0.5 for exact scaling, then rescale by 2. The mean will lose precision only when the value and m1 are sub-normal. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MeanTest.java: ########## @@ -0,0 +1,229 @@ +/* + * 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 Mean}. + */ +final class MeanTest { + + @Test + void testEmpty() { + Mean mean = Mean.create(); + Assertions.assertEquals(Double.NaN, mean.getAsDouble()); + } + + @Test + void testNan() { + Mean mean = Mean.create(); + double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + for (double value : testArray) { + mean.accept(value); + } + Assertions.assertEquals(Double.NaN, mean.getAsDouble()); + } + + @ParameterizedTest + @MethodSource(value = {"testMean"}) + void testMean(double[] values, double expected) { + Mean mean = Mean.create(); + for (double value : values) { + mean.accept(value); + } + TestHelper.assertEquals(expected, mean.getAsDouble(), 5, () -> "mean"); + TestHelper.assertEquals(expected, Mean.of(values).getAsDouble(), 5, () -> "of (values)"); + } + + static Stream<Arguments> testMean() { Review Comment: Missing overflow tests: ```java Arguments.of(new double[] {Double.MAX_VALUE, -Double.MAX_VALUE}, 0), Arguments.of(new double[] {1, -Double.MAX_VALUE}, -Double.MAX_VALUE / 2), Arguments.of(new double[] {1, 1, 1, -Double.MAX_VALUE}, -Double.MAX_VALUE / 4), ``` ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/FirstMoment.java: ########## @@ -0,0 +1,168 @@ +/* + * 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; + +/** + * Computes the first moment (arithmetic mean) using the definitional formula: + * + * <p> mean = sum(x_i) / n + * + * <p> To limit numeric errors, the value of the statistic is computed using the + * following recursive updating algorithm: + * <ol> + * <li>Initialize <code>m = </code> the first value</li> + * <li>For each additional value, update using <br> + * <code>m = m + (new value - m) / (number of observations)</code></li> + * </ol> + * + * <p>Returns <code>Double.NaN</code> if the dataset is empty. Note that + * <code>NaN</code> may also be returned if the input includes NaN and / or infinite + * values of opposite sign. + * + * <p><strong>Note that this implementation is not synchronized.</strong> If + * multiple threads access an instance of this class concurrently, and at least + * one of the threads invokes the <code>increment()</code> or + * <code>clear()</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. + */ +class FirstMoment implements DoubleStatistic, DoubleStatisticAccumulator<FirstMoment> { + /** Count of values that have been added. */ + private long n; + + /** First moment of values that have been added. */ + private double m1; + + /** Running count of <code>Double.POSITIVE_INFINITY</code> values. */ + private long posInfCount; Review Comment: These counts are not required. Having to track these counters adds branch points to the code that are not required. I understand what you are trying to do by tracking if the result has a accumulated opposite infinities. However this can be done by just summing the values up. I added a double to track the sum of the current values. If m1 is not finite then some part of the computation encountered a non-finite value. You can then return the sum as it will not be finite and in this case the non-finite sum result is the same as the mean. All we have to do is ensure that m1 never overflows unless a non-finite value is encountered. So we require a test that combine two instances that are currently +MAX_VALUE and -MIN_VALUE to check this is correctly handled. Here is the implementation that passes all your current tests: ```java class FirstMoment implements DoubleStatistic, DoubleStatisticAccumulator<FirstMoment> { private long n; private double sum; private double m1; private double dev; private double nDev; FirstMoment() {} FirstMoment(final double m1, final long n) { this.m1 = m1; this.n = n; } public void accept(double value) { n++; sum += value; // prevent overflow at the cost of precision on sub-normals dev = (value * 0.5 - m1 * 0.5) * 2; nDev = dev / n; m1 += nDev; } public double getAsDouble() { if (Double.isFinite(m1)) { return n == 0 ? Double.NaN : m1; } return sum; } public FirstMoment combine(FirstMoment other) { if (n == 0) { n = other.n; m1 = other.m1; sum = other.sum; // copy dev and nDev - not currently required return this; } if (other.n == 0) { return this; } n += other.n; sum += other.sum; // prevent overflow at the cost of precision on sub-normals dev = (other.m1 * 0.5 - m1 * 0.5) * 2; nDev = dev * ((double) other.n / n); m1 += nDev; return this; } long getN() { return n; } } ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
