aherbert commented on code in PR #46: URL: https://github.com/apache/commons-statistics/pull/46#discussion_r1262438649
########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * Review Comment: Remove this empty line ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/DoubleStatistic.java: ########## @@ -0,0 +1,29 @@ +/* + * 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.function.DoubleConsumer; +import java.util.function.DoubleSupplier; + + +/** + * Represents a state object for computing a single {@code Statistic} over {@code double} valued input(s). + * + * <p>Base interface implemented by all statistics.</p> Review Comment: Check for and remove `</p>` tags ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values + * @return Min instance + */ + public static Min of(double... values) { + final StorelessMin storelessMin = new StorelessMin(); Review Comment: Change `storelessMin` to `min` ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values + * @return Min instance + */ + public static Min of(double... values) { + final StorelessMin storelessMin = new StorelessMin(); + Arrays.stream(values).forEach(storelessMin); + return storelessMin; + } + + /** + * 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 minimum of all input values. + * + * <p>When no values have been added, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @return The {@code min}. + */ + @Override + public abstract double getAsDouble(); + + /** {@inheritDoc} */ + @Override + public abstract Min combine(Min other); + + /** + * {@code Min} implementation that does not store the input value(s) processed so far. + * + * <p>Uses JDK's {@link Math#min Math.min} as an underlying function + * to compute the {@code minimum} + */ + private static final class StorelessMin extends Min { Review Comment: IIUC this does not need the final keyword as it is private. It may be linting tools from Sonarcloud that highlight this. ########## src/conf/pmd/pmd-ruleset.xml: ########## @@ -90,7 +90,8 @@ <properties> <property name="violationSuppressXPath" value="./ancestor-or-self::ClassOrInterfaceDeclaration[@SimpleName='DD' - or @SimpleName='Two' or @SimpleName='One']"/> + or @SimpleName='Two' or @SimpleName='One' or @SimpleName='Min']"/> + Review Comment: Remove this empty line ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values Review Comment: End param descriptions with a period `.` ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); Review Comment: Here the `Min` is redundant from expected and actual. I would drop it for cleaner code. It makes it simpler to adapt the test to other statistics by copy. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN), + Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d), + Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN) + ); + } + + @Test + void testNanParallelStream() { + DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN); + Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble())); + } + + @Test + void testSpecialValues() { + double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble()); + + Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testNaNs(double[] values, double expectedMin) { + Min stat = Min.create(); + for (final double value: values) { + stat.accept(value); + } + Assertions.assertEquals(expectedMin, stat.getAsDouble()); + } + + static Stream<Arguments> testNaNs() { + return Stream.of( + Arguments.of(new double[] {Double.NaN, 2d, 3d}, Double.NaN), + Arguments.of(new double[] {1d, Double.NaN, 3d}, Double.NaN), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN) + ); + } + + @Test + void testNaN() { + double[] testArray = {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[5]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + Assertions.assertEquals(Double.NaN, Min.of(testArray).getAsDouble()); Review Comment: This `Min.of` test can be covered by moving this array into the cases for `testMin`. Essentially what this test is doing is checking if any special non-nan value _cannot_ change a NaN back to a non-nan value. Perhaps this is easier to read using: ```java @Test void testNaN() { double[] testArray = {Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; Min stat = Min.create(); // Test non-nan values cannot revert a NaN for (final double x : testArray) { stat.accept(x); Assertions.assertEquals(Double.NaN, stat.getAsDouble()); } } ``` ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN), + Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d), + Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN) + ); + } + + @Test + void testNanParallelStream() { + DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN); + Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble())); + } + + @Test + void testSpecialValues() { + double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble()); + + Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testNaNs(double[] values, double expectedMin) { + Min stat = Min.create(); + for (final double value: values) { + stat.accept(value); + } + Assertions.assertEquals(expectedMin, stat.getAsDouble()); + } + + static Stream<Arguments> testNaNs() { + return Stream.of( + Arguments.of(new double[] {Double.NaN, 2d, 3d}, Double.NaN), + Arguments.of(new double[] {1d, Double.NaN, 3d}, Double.NaN), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN) + ); + } + + @Test + void testNaN() { + double[] testArray = {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[5]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + Assertions.assertEquals(Double.NaN, Min.of(testArray).getAsDouble()); + + Assertions.assertTrue(Double.isNaN(Min.of(Double.NaN, Double.NaN, Double.NaN).getAsDouble())); + } + + @ParameterizedTest + @MethodSource + void testArrayOfArrays(double[][] input, double expectedMin) { + + double actualMin = Arrays.stream(input) + .map(Min::of) + .reduce(Min::combine) + .map(DoubleSupplier::getAsDouble) + .orElseThrow(RuntimeException::new); + + Assertions.assertEquals(expectedMin, actualMin); + } + static Stream<Arguments> testArrayOfArrays() { Review Comment: Empty line above the method declaration. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN), + Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d), + Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN) + ); + } + + @Test + void testNanParallelStream() { + DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN); + Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble())); + } + + @Test + void testSpecialValues() { + double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble()); + + Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testNaNs(double[] values, double expectedMin) { + Min stat = Min.create(); + for (final double value: values) { + stat.accept(value); + } + Assertions.assertEquals(expectedMin, stat.getAsDouble()); + } + + static Stream<Arguments> testNaNs() { + return Stream.of( + Arguments.of(new double[] {Double.NaN, 2d, 3d}, Double.NaN), + Arguments.of(new double[] {1d, Double.NaN, 3d}, Double.NaN), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN) + ); + } + + @Test + void testNaN() { + double[] testArray = {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[5]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + Assertions.assertEquals(Double.NaN, Min.of(testArray).getAsDouble()); + + Assertions.assertTrue(Double.isNaN(Min.of(Double.NaN, Double.NaN, Double.NaN).getAsDouble())); + } + + @ParameterizedTest + @MethodSource + void testArrayOfArrays(double[][] input, double expectedMin) { + + double actualMin = Arrays.stream(input) + .map(Min::of) + .reduce(Min::combine) + .map(DoubleSupplier::getAsDouble) + .orElseThrow(RuntimeException::new); + + Assertions.assertEquals(expectedMin, actualMin); + } + static Stream<Arguments> testArrayOfArrays() { + return Stream.of( + Arguments.of(new double[][] {{}, {}, {}}, Double.POSITIVE_INFINITY), Review Comment: Note: You can use 4 or 8 space chars for the trailing line. When the lines are long using 4 chars allows more to fit in a standard text editor. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN), + Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d), + Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN) + ); + } + + @Test + void testNanParallelStream() { + DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN); + Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble())); + } + + @Test + void testSpecialValues() { + double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble()); + + Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble()); Review Comment: I would remove this `Min.of` and add this testArray to the cases for testMin. The incremental nature of the assertions in this test are not easy to follow. The same data with special values is in testParallelStream so there is redundancy here. If you wish to add a test for incremental min then perhaps it is more clear with counters: ```java @Test void testIncrement() { int lo = 0; int hi = 0; final Min stat = Min.of(lo); for (int i = 0; i < 5; i++) { stat.accept(++hi); Assertions.assertEquals(lo, stat.getAsDouble()); stat.accept(--lo); Assertions.assertEquals(lo, stat.getAsDouble()); } } ``` ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values + * @return Min instance + */ + public static Min of(double... values) { + final StorelessMin storelessMin = new StorelessMin(); + Arrays.stream(values).forEach(storelessMin); + return storelessMin; + } + + /** + * 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 minimum of all input values. + * + * <p>When no values have been added, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @return The {@code min}. + */ + @Override + public abstract double getAsDouble(); + + /** {@inheritDoc} */ + @Override + public abstract Min combine(Min other); + + /** + * {@code Min} implementation that does not store the input value(s) processed so far. + * + * <p>Uses JDK's {@link Math#min Math.min} as an underlying function + * to compute the {@code minimum} + */ + private static final class StorelessMin extends Min { + + /** Current min. */ + private double min; + + /** Create a StorelessMin instance. */ + StorelessMin() { + min = Double.POSITIVE_INFINITY; + } + + /** {@inheritDoc} */ Review Comment: I think that internal classes may not require the inheritDoc javadoc lines. Javadoc like this is only required on public classes as the final javadoc report does not process private scope. ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values + * @return Min instance + */ + public static Min of(double... values) { + final StorelessMin storelessMin = new StorelessMin(); + Arrays.stream(values).forEach(storelessMin); Review Comment: It may be interesting to use JMH to compare the use of streams to a simple for loop here, e.g. ```java for (final double x : values) { min.accept(x); } ``` For now I would create a Jira ticket as a task to create JMH benchmarks for statistic initialisation comparing Arrays.stream to a simple for loop. ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values + * @return Min instance + */ + public static Min of(double... values) { + final StorelessMin storelessMin = new StorelessMin(); + Arrays.stream(values).forEach(storelessMin); + return storelessMin; + } + + /** + * 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 minimum of all input values. + * + * <p>When no values have been added, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @return The {@code min}. + */ + @Override + public abstract double getAsDouble(); + + /** {@inheritDoc} */ + @Override + public abstract Min combine(Min other); + + /** + * {@code Min} implementation that does not store the input value(s) processed so far. + * + * <p>Uses JDK's {@link Math#min Math.min} as an underlying function + * to compute the {@code minimum} Review Comment: Period at end of sentence. ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java: ########## @@ -0,0 +1,141 @@ +/* + * 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 minimum of the available values. + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>The result is <code>POSITIVE_INFINITY</code> 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. + */ +public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> { + + /** + * Create a Min instance. + */ + Min() { + // No-op + } + + /** + * Creates a {@code Min} 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>. + * + * <p>The result is {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY} + * if no values have been added. + * + * @return {@code Min} implementation + * + */ + public static Min create() { + return new StorelessMin(); + } + + /** + * Returns a {@code Min} instance that has the minimum of all input value(s). + * + * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>. + * + * <p>When the input is an empty array, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @param values Values + * @return Min instance + */ + public static Min of(double... values) { + final StorelessMin storelessMin = new StorelessMin(); + Arrays.stream(values).forEach(storelessMin); + return storelessMin; + } + + /** + * 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 minimum of all input values. + * + * <p>When no values have been added, the result is + * {@link Double#POSITIVE_INFINITY POSITIVE_INFINITY}. + * + * @return The {@code min}. + */ + @Override + public abstract double getAsDouble(); + + /** {@inheritDoc} */ + @Override + public abstract Min combine(Min other); + + /** + * {@code Min} implementation that does not store the input value(s) processed so far. + * + * <p>Uses JDK's {@link Math#min Math.min} as an underlying function + * to compute the {@code minimum} + */ + private static final class StorelessMin extends Min { + + /** Current min. */ + private double min; + + /** Create a StorelessMin instance. */ Review Comment: In general you can use the shorter "Create an instance" for required but otherwise uninformative constructor docs. Since you do not use this constructor I would drop it and initialise the `min` directly in the declaration: ``` private double min = Double.POSITIVE_INFINITY; ``` ########## commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/DoubleStatisticAccumulator.java: ########## @@ -0,0 +1,33 @@ +/* + * 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; + +/** + * A mutable result container that accumulates a {@code DoubleStatistic}. + * + * @param <T> the {@code DoubleStatistic} being accumulated. + */ +public interface DoubleStatisticAccumulator<T extends DoubleStatistic> { + + /** + * Combines the state of another {@code DoubleStatistic} into this one. + * + * @param other another {@code DoubleStatistic} to be combined Review Comment: This project uses capitalisation for the first word after the param name. It does not use `the` as the first word (unlike the JDK). All sentences should end with a period. So this should change to: ``` @param other Another {@code DoubleStatistic} to be combined. // or @param other {@code DoubleStatistic} to be combined. ``` Strangely the Commons math projects are not so strict on the `@return` tag and allow `the`. However it is good to maintain a consistent style. In this case we could select wording from any builder or other object that returns itself, which typically includes the word `this` for example: ``` @return {@code this} instance ``` ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN), + Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d), + Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN) + ); + } + + @Test + void testNanParallelStream() { + DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN); + Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble())); + } + + @Test + void testSpecialValues() { + double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble()); + + Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testNaNs(double[] values, double expectedMin) { + Min stat = Min.create(); + for (final double value: values) { + stat.accept(value); + } + Assertions.assertEquals(expectedMin, stat.getAsDouble()); + } + + static Stream<Arguments> testNaNs() { Review Comment: These test cases and test fixture can be combined into the cases for testMin. ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { Review Comment: This method contains different cases to testMin. They should be merged to capture all cases. You should add an empty array as a values input. You can then test each method using the same streaming method, e.g.: ```java @ParameterizedTest @MethodSource(value = "testMin") void testParallelStream(double[] values, double expectedMin) { ``` ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { + + @Test + void testEmpty() { + Min min = Min.create(); + Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testMin(double[] values, double expectedMin) { + Min stat = Min.create(); + Arrays.stream(values).forEach(stat); + double actualMin = stat.getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin, "min"); + Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min"); + } + + static Stream<Arguments> testMin() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN), + Arguments.of(new double[] {-0.0, +0.0}, -0.0), + Arguments.of(new double[] {0.0, -0.0}, -0.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747) + ); + } + + @ParameterizedTest + @MethodSource + void testCombine(double[] first, double[] second, double expectedMin) { + Min firstMin = Min.create(); + Min secondMin = Min.create(); + + Arrays.stream(first).forEach(firstMin); + Arrays.stream(second).forEach(secondMin); + + double secondMinBeforeCombine = secondMin.getAsDouble(); + firstMin.combine(secondMin); + Assertions.assertEquals(expectedMin, firstMin.getAsDouble()); + Assertions.assertEquals(secondMinBeforeCombine, secondMin.getAsDouble()); + } + + static Stream<Arguments> testCombine() { + return Stream.of( + Arguments.of(new double[] {}, new double[] {}, Double.POSITIVE_INFINITY), + Arguments.of(new double[] {3.14}, new double[] {}, 3.14), + Arguments.of(new double[] {}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {}, new double[] {Double.NaN}, Double.NaN), + Arguments.of(new double[] {Double.NaN, Double.NaN}, new double[] {}, Double.NaN), + Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718), + Arguments.of(new double[] {-1, 0, 1}, new double[] {1.1, 2.2, 3.3}, -1), + Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1), + Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0), + Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79), + Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0), + Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY}, + new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE}, + Double.NaN) + ); + } + + @ParameterizedTest + @MethodSource + void testParallelStream(double[] values, double expectedMin) { + double actualMin = Arrays.stream(values).parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble(); + Assertions.assertEquals(expectedMin, actualMin); + } + + static Stream<Arguments> testParallelStream() { + return Stream.of( + Arguments.of(new double[] {3.14}, 3.14), + Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0), + Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012), + Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN), + Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d), + Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY), + Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN) + ); + } + + @Test + void testNanParallelStream() { + DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN); + Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::create, Min::accept, Min::combine).getAsDouble())); + } + + @Test + void testSpecialValues() { + double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(-0.0d, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble()); + + Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble()); + } + + @ParameterizedTest + @MethodSource + void testNaNs(double[] values, double expectedMin) { + Min stat = Min.create(); + for (final double value: values) { + stat.accept(value); + } + Assertions.assertEquals(expectedMin, stat.getAsDouble()); + } + + static Stream<Arguments> testNaNs() { + return Stream.of( + Arguments.of(new double[] {Double.NaN, 2d, 3d}, Double.NaN), + Arguments.of(new double[] {1d, Double.NaN, 3d}, Double.NaN), + Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN) + ); + } + + @Test + void testNaN() { + double[] testArray = {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}; + Min stat = Min.create(); + + stat.accept(testArray[0]); + Assertions.assertEquals(0.0d, stat.getAsDouble()); + + stat.accept(testArray[1]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[2]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[3]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[4]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + stat.accept(testArray[5]); + Assertions.assertEquals(Double.NaN, stat.getAsDouble()); + + Assertions.assertEquals(Double.NaN, Min.of(testArray).getAsDouble()); + + Assertions.assertTrue(Double.isNaN(Min.of(Double.NaN, Double.NaN, Double.NaN).getAsDouble())); + } + + @ParameterizedTest + @MethodSource + void testArrayOfArrays(double[][] input, double expectedMin) { + Review Comment: Remove empty line ########## commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java: ########## @@ -0,0 +1,221 @@ +/* + * 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 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; + +import java.util.Arrays; +import java.util.function.DoubleSupplier; +import java.util.stream.DoubleStream; +import java.util.stream.Stream; + +/** + * Test for {@link Min}. + */ +final class MinTest { Review Comment: I think we should test that the same min is returned if the input array is randomised. I would add a TestHelper class with a Fisher-Yates shuffle copied from: [RNG ArraySampler](https://github.com/apache/commons-rng/blob/master/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/ArraySampler.java) If you update the signature to return the input argument then you can code: ```java @ParameterizedTest @MethodSource(value = "testMin") void testMinRandomOrder(double[] values, double expected) { UniformRandomProvider rng = RandomSource.SPLIT_MIX_64.create(); for (int i = 0; i < 10; i++) { testMin(TestHelper.shuffle(rng, values.clone()), expected); } } ``` The clone is only required if the `testMin` method destructively modifies the array. Otherwise you can continue to shuffle the same array each time. This change will require added the appropriate RNG modules to the pom. -- 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]
