Update cumsum() behavior to correctly propagate NaN

This has been corrected in GNU R last year by @kalibera wch/r-source@d6d913c#diff-e40310209d95e5ce79c5c8aa58c39582
bedatadriven · Jan 9, 2018 · dd71919 · dd71919
1 parent 0278e9f
commit dd71919
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/core/src/main/java/org/renjin/primitives/MathGroup.java b/core/src/main/java/org/renjin/primitives/MathGroup.java
@@ -440,7 +440,7 @@ public static DoubleVector cumsum(StringVector source) {
 
   @Builtin
   public static DoubleVector cumsum(Null x) {
-    return DoubleVector.EMPTY;
+    return DoubleArrayVector.EMPTY;
   }
 
   private static DoubleVector cumulativeRealSum(Vector source) {
@@ -449,7 +449,8 @@ private static DoubleVector cumulativeRealSum(Vector source) {
     double sum = 0;
     for (int i = 0; i < source.length(); i++) {
       sum += source.getElementAsDouble(i);
-      if (Double.isNaN(sum)) {
+      if (DoubleVector.isNA(sum)) {
+        // The rest of the vector is already initialized to NA
         break;
       }
       result.set(i, sum);
@@ -593,22 +594,22 @@ public static ComplexVector cummax(ComplexVector source) {
 
   @Builtin
   public static DoubleVector cummin(Null x) {
-    return DoubleVector.EMPTY;
+    return DoubleArrayVector.EMPTY;
   }
 
   @Builtin
   public static DoubleVector cummax(Null x) {
-    return DoubleVector.EMPTY;
+    return DoubleArrayVector.EMPTY;
   }
 
   private static ComplexVector cumulativeExtrema(String functionName, ComplexVector source) {
     // This is probably not intended behavior, but cumxxx(complex(0)) in GNU R
     // returns complex(0), so we'll mimic it here
     if(source.length() == 0) {
       if(source.getNames() == Null.INSTANCE) {
-        return ComplexVector.EMPTY;
+        return ComplexArrayVector.EMPTY;
       } else {
-        return ComplexVector.NAMED_EMPTY;
+        return ComplexArrayVector.NAMED_EMPTY;
       }
     } else {
       throw new EvalException(String.format("'%s' not defined for complex numbers", functionName));

diff --git a/tests/src/test/R/test.cumsum.R b/tests/src/test/R/test.cumsum.R
@@ -17,7 +17,7 @@
 # https://www.gnu.org/licenses/gpl-2.0.txt
 #
 
-# Generated by gen-summary-tests.R using GNU R version 3.3.1 (2016-06-21)
+# Generated by gen-summary-tests.R using GNU R version 3.4.2 (2017-09-28)
 library(hamcrest)
 cumsum.foo <- function(...) 41
 Math.bar <- function(...) 44
@@ -45,7 +45,7 @@ test.cumsum.20 <- function() assertThat(cumsum(c(3.14159, 6.28319, 9.42478, 12.5
 test.cumsum.21 <- function() assertThat(cumsum(c(-3.14159, -6.28319, -9.42478, -12.5664, -15.708)), identicalTo(c(-3.14159, -9.42478, -18.84956, -31.41596, -47.12396), tol = 0.000100))
 test.cumsum.22 <- function() assertThat(cumsum(c(1.5, 2.5)), identicalTo(c(1.5, 4), tol = 0.000100))
 test.cumsum.23 <- function() assertThat(cumsum(c(1.5, NA)), identicalTo(c(1.5, NA), tol = 0.000100))
-test.cumsum.24 <- function() assertThat(cumsum(c(1.5, NaN)), identicalTo(c(1.5, NA), tol = 0.000100))
+test.cumsum.24 <- function() assertThat(cumsum(c(1.5, NaN)), identicalTo(c(1.5, NaN), tol = 0.000100))
 test.cumsum.25 <- function() assertThat(cumsum(c(Inf, -1.5)), identicalTo(c(Inf, Inf)))
 test.cumsum.26 <- function() assertThat(cumsum(c(-Inf, -1.5)), identicalTo(c(-Inf, -Inf)))
 test.cumsum.27 <- function() assertThat(cumsum(c(Inf, 1.5)), identicalTo(c(Inf, Inf)))

diff --git a/tests/src/test/java/org/renjin/MicroTest.java b/tests/src/test/java/org/renjin/MicroTest.java
@@ -2979,7 +2979,7 @@ public void micro1099() {
   }
   @Test
   public void micro1100() {
-    assertIdentical("{ cumsum(c(1,2,3,0/0,5)) }", "c(1, 3, 6, NA, NA)");
+    assertIdentical("{ cumsum(c(1,2,3,0/0,5)) }", "c(1, 3, 6, NaN, NaN)");
   }
   @Test
   public void micro1101() {