8030212: Several api.java.util.stream tests got "NaN" value instead of "Infinity" or "-Infinity"
authordarcy
Fri, 03 Jan 2014 10:38:23 -0800
changeset 22101 231247ddf41a
parent 22100 b5238ea86488
child 22102 c070cb089853
8030212: Several api.java.util.stream tests got "NaN" value instead of "Infinity" or "-Infinity" Reviewed-by: mduigou, psandoz
jdk/src/share/classes/java/util/DoubleSummaryStatistics.java
jdk/src/share/classes/java/util/stream/Collectors.java
jdk/src/share/classes/java/util/stream/DoublePipeline.java
jdk/test/java/util/stream/TestDoubleSumAverage.java
--- a/jdk/src/share/classes/java/util/DoubleSummaryStatistics.java	Fri Jan 03 09:49:08 2014 -0800
+++ b/jdk/src/share/classes/java/util/DoubleSummaryStatistics.java	Fri Jan 03 10:38:23 2014 -0800
@@ -64,6 +64,7 @@
     private long count;
     private double sum;
     private double sumCompensation; // Low order bits of sum
+    private double simpleSum; // Used to compute right sum for non-finite inputs
     private double min = Double.POSITIVE_INFINITY;
     private double max = Double.NEGATIVE_INFINITY;
 
@@ -82,6 +83,7 @@
     @Override
     public void accept(double value) {
         ++count;
+        simpleSum += value;
         sumWithCompensation(value);
         min = Math.min(min, value);
         max = Math.max(max, value);
@@ -96,6 +98,7 @@
      */
     public void combine(DoubleSummaryStatistics other) {
         count += other.count;
+        simpleSum += other.simpleSum;
         sumWithCompensation(other.sum);
         sumWithCompensation(other.sumCompensation);
         min = Math.min(min, other.min);
@@ -147,7 +150,15 @@
      */
     public final double getSum() {
         // Better error bounds to add both terms as the final sum
-        return sum + sumCompensation;
+        double tmp =  sum + sumCompensation;
+        if (Double.isNaN(tmp) && Double.isInfinite(simpleSum))
+            // If the compensated sum is spuriously NaN from
+            // accumulating one or more same-signed infinite values,
+            // return the correctly-signed infinity stored in
+            // simpleSum.
+            return simpleSum;
+        else
+            return tmp;
     }
 
     /**
--- a/jdk/src/share/classes/java/util/stream/Collectors.java	Fri Jan 03 09:49:08 2014 -0800
+++ b/jdk/src/share/classes/java/util/stream/Collectors.java	Fri Jan 03 10:38:23 2014 -0800
@@ -507,16 +507,20 @@
     summingDouble(ToDoubleFunction<? super T> mapper) {
         /*
          * In the arrays allocated for the collect operation, index 0
-         * holds the high-order bits of the running sum and index 1
-         * holds the low-order bits of the sum computed via
-         * compensated summation.
+         * holds the high-order bits of the running sum, index 1 holds
+         * the low-order bits of the sum computed via compensated
+         * summation, and index 2 holds the simple sum used to compute
+         * the proper result if the stream contains infinite values of
+         * the same sign.
          */
         return new CollectorImpl<>(
-                () -> new double[2],
-                (a, t) -> { sumWithCompensation(a, mapper.applyAsDouble(t)); },
-                (a, b) -> { sumWithCompensation(a, b[0]); return sumWithCompensation(a, b[1]); },
-                // Better error bounds to add both terms as the final sum
-                a -> a[0] + a[1],
+                () -> new double[3],
+                (a, t) -> { sumWithCompensation(a, mapper.applyAsDouble(t));
+                            a[2] += mapper.applyAsDouble(t);},
+                (a, b) -> { sumWithCompensation(a, b[0]);
+                            a[2] += b[2];
+                            return sumWithCompensation(a, b[1]); },
+                a -> computeFinalSum(a),
                 CH_NOID);
     }
 
@@ -540,6 +544,20 @@
         return intermediateSum;
     }
 
+    /**
+     * If the compensated sum is spuriously NaN from accumulating one
+     * or more same-signed infinite values, return the
+     * correctly-signed infinity stored in the simple sum.
+     */
+    static double computeFinalSum(double[] summands) {
+        // Better error bounds to add both terms as the final sum
+        double tmp = summands[0] + summands[1];
+        double simpleSum = summands[summands.length - 1];
+        if (Double.isNaN(tmp) && Double.isInfinite(simpleSum))
+            return simpleSum;
+        else
+            return tmp;
+    }
 
     /**
      * Returns a {@code Collector} that produces the arithmetic mean of an integer-valued
@@ -608,11 +626,10 @@
          * summation, and index 2 holds the number of values seen.
          */
         return new CollectorImpl<>(
-                () -> new double[3],
-                (a, t) -> { sumWithCompensation(a, mapper.applyAsDouble(t)); a[2]++; },
-                (a, b) -> { sumWithCompensation(a, b[0]); sumWithCompensation(a, b[1]); a[2] += b[2]; return a; },
-                // Better error bounds to add both terms as the final sum to compute average
-                a -> (a[2] == 0) ? 0.0d : ((a[0] + a[1]) / a[2]),
+                () -> new double[4],
+                (a, t) -> { sumWithCompensation(a, mapper.applyAsDouble(t)); a[2]++; a[3]+= mapper.applyAsDouble(t);},
+                (a, b) -> { sumWithCompensation(a, b[0]); sumWithCompensation(a, b[1]); a[2] += b[2]; a[3] += b[3]; return a; },
+                a -> (a[2] == 0) ? 0.0d : (computeFinalSum(a) / a[2]),
                 CH_NOID);
     }
 
--- a/jdk/src/share/classes/java/util/stream/DoublePipeline.java	Fri Jan 03 09:49:08 2014 -0800
+++ b/jdk/src/share/classes/java/util/stream/DoublePipeline.java	Fri Jan 03 10:38:23 2014 -0800
@@ -379,21 +379,24 @@
     public final double sum() {
         /*
          * In the arrays allocated for the collect operation, index 0
-         * holds the high-order bits of the running sum and index 1
-         * holds the low-order bits of the sum computed via
-         * compensated summation.
+         * holds the high-order bits of the running sum, index 1 holds
+         * the low-order bits of the sum computed via compensated
+         * summation, and index 2 holds the simple sum used to compute
+         * the proper result if the stream contains infinite values of
+         * the same sign.
          */
-        double[] summation = collect(() -> new double[2],
+        double[] summation = collect(() -> new double[3],
                                (ll, d) -> {
                                    Collectors.sumWithCompensation(ll, d);
+                                   ll[2] += d;
                                },
                                (ll, rr) -> {
                                    Collectors.sumWithCompensation(ll, rr[0]);
                                    Collectors.sumWithCompensation(ll, rr[1]);
+                                   ll[2] += rr[2];
                                });
 
-        // Better error bounds to add both terms as the final sum
-        return summation[0] + summation[1];
+        return Collectors.computeFinalSum(summation);
     }
 
     @Override
@@ -421,21 +424,23 @@
          * In the arrays allocated for the collect operation, index 0
          * holds the high-order bits of the running sum, index 1 holds
          * the low-order bits of the sum computed via compensated
-         * summation, and index 2 holds the number of values seen.
+         * summation, index 2 holds the number of values seen, index 3
+         * holds the simple sum.
          */
-        double[] avg = collect(() -> new double[3],
+        double[] avg = collect(() -> new double[4],
                                (ll, d) -> {
                                    ll[2]++;
                                    Collectors.sumWithCompensation(ll, d);
+                                   ll[3] += d;
                                },
                                (ll, rr) -> {
                                    Collectors.sumWithCompensation(ll, rr[0]);
                                    Collectors.sumWithCompensation(ll, rr[1]);
                                    ll[2] += rr[2];
+                                   ll[3] += rr[3];
                                });
         return avg[2] > 0
-            // Better error bounds to add both terms as the final sum to compute average
-            ? OptionalDouble.of((avg[0] + avg[1]) / avg[2])
+            ? OptionalDouble.of(Collectors.computeFinalSum(avg) / avg[2])
             : OptionalDouble.empty();
     }
 
--- a/jdk/test/java/util/stream/TestDoubleSumAverage.java	Fri Jan 03 09:49:08 2014 -0800
+++ b/jdk/test/java/util/stream/TestDoubleSumAverage.java	Fri Jan 03 10:38:23 2014 -0800
@@ -25,17 +25,20 @@
 import java.util.function.*;
 import java.util.stream.*;
 
+import static java.lang.Double.*;
+
 /*
  * @test
- * @bug 8006572
+ * @bug 8006572 8030212
  * @summary Test for use of non-naive summation in stream-related sum and average operations.
  */
 public class TestDoubleSumAverage {
     public static void main(String... args) {
         int failures = 0;
 
+        failures += testZeroAverageOfNonEmptyStream();
         failures += testForCompenstation();
-        failures += testZeroAverageOfNonEmptyStream();
+        failures += testNonfiniteSum();
 
         if (failures > 0) {
             throw new RuntimeException("Found " + failures + " numerical failure(s).");
@@ -43,6 +46,15 @@
     }
 
     /**
+     * Test to verify that a non-empty stream with a zero average is non-empty.
+     */
+    private static int testZeroAverageOfNonEmptyStream() {
+        Supplier<DoubleStream> ds = () -> DoubleStream.iterate(0.0, e -> 0.0).limit(10);
+
+        return  compareUlpDifference(0.0, ds.get().average().getAsDouble(), 0);
+    }
+
+    /**
      * Compute the sum and average of a sequence of double values in
      * various ways and report an error if naive summation is used.
      */
@@ -83,19 +95,68 @@
         return failures;
     }
 
-    /**
-     * Test to verify that a non-empty stream with a zero average is non-empty.
-     */
-    private static int testZeroAverageOfNonEmptyStream() {
-        Supplier<DoubleStream> ds = () -> DoubleStream.iterate(0.0, e -> 0.0).limit(10);
+    private static int testNonfiniteSum() {
+        int failures = 0;
+
+        Map<Supplier<DoubleStream>, Double> testCases = new LinkedHashMap<>();
+        testCases.put(() -> DoubleStream.of(MAX_VALUE, MAX_VALUE),   POSITIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(-MAX_VALUE, -MAX_VALUE), NEGATIVE_INFINITY);
+
+        testCases.put(() -> DoubleStream.of(1.0d, POSITIVE_INFINITY, 1.0d), POSITIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(POSITIVE_INFINITY),             POSITIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(POSITIVE_INFINITY, POSITIVE_INFINITY), POSITIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(POSITIVE_INFINITY, POSITIVE_INFINITY, 0.0), POSITIVE_INFINITY);
+
+        testCases.put(() -> DoubleStream.of(1.0d, NEGATIVE_INFINITY, 1.0d), NEGATIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(NEGATIVE_INFINITY),             NEGATIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(NEGATIVE_INFINITY, NEGATIVE_INFINITY), NEGATIVE_INFINITY);
+        testCases.put(() -> DoubleStream.of(NEGATIVE_INFINITY, NEGATIVE_INFINITY, 0.0), NEGATIVE_INFINITY);
 
-        return  compareUlpDifference(0.0, ds.get().average().getAsDouble(), 0);
+        testCases.put(() -> DoubleStream.of(1.0d, NaN, 1.0d),               NaN);
+        testCases.put(() -> DoubleStream.of(NaN),                           NaN);
+        testCases.put(() -> DoubleStream.of(1.0d, NEGATIVE_INFINITY, POSITIVE_INFINITY, 1.0d), NaN);
+        testCases.put(() -> DoubleStream.of(1.0d, POSITIVE_INFINITY, NEGATIVE_INFINITY, 1.0d), NaN);
+        testCases.put(() -> DoubleStream.of(POSITIVE_INFINITY, NaN), NaN);
+        testCases.put(() -> DoubleStream.of(NEGATIVE_INFINITY, NaN), NaN);
+        testCases.put(() -> DoubleStream.of(NaN, POSITIVE_INFINITY), NaN);
+        testCases.put(() -> DoubleStream.of(NaN, NEGATIVE_INFINITY), NaN);
+
+        for(Map.Entry<Supplier<DoubleStream>, Double> testCase : testCases.entrySet()) {
+            Supplier<DoubleStream> ds = testCase.getKey();
+            double expected = testCase.getValue();
+
+            DoubleSummaryStatistics stats = ds.get().collect(DoubleSummaryStatistics::new,
+                                                             DoubleSummaryStatistics::accept,
+                                                             DoubleSummaryStatistics::combine);
+
+            failures += compareUlpDifference(expected, stats.getSum(), 0);
+            failures += compareUlpDifference(expected, stats.getAverage(), 0);
+
+            failures += compareUlpDifference(expected, ds.get().sum(), 0);
+            failures += compareUlpDifference(expected, ds.get().average().getAsDouble(), 0);
+
+            failures += compareUlpDifference(expected, ds.get().boxed().collect(Collectors.summingDouble(d -> d)), 0);
+            failures += compareUlpDifference(expected, ds.get().boxed().collect(Collectors.averagingDouble(d -> d)), 0);
+        }
+
+        return failures;
     }
 
     /**
      * Compute the ulp difference of two double values and compare against an error threshold.
      */
     private static int compareUlpDifference(double expected, double computed, double threshold) {
+        if (!Double.isFinite(expected)) {
+            // Handle NaN and infinity cases
+            if (Double.compare(expected, computed) == 0)
+                return 0;
+            else {
+                System.err.printf("Unexpected sum, %g rather than %g.%n",
+                                  computed, expected);
+                return 1;
+            }
+        }
+
         double ulpDifference = Math.abs(expected - computed) / Math.ulp(expected);
 
         if (ulpDifference > threshold) {