diff --git a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java index 06562ff9dc4a0951c5010efda82e303d2368d1a6..881043c13445bfb06edc555db3a27ceef48b433c 100644 --- a/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java +++ b/com.oracle.truffle.r.nodes/src/com/oracle/truffle/r/nodes/unary/UnaryArithmeticReduceNode.java @@ -23,6 +23,7 @@ package com.oracle.truffle.r.nodes.unary; import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.dsl.Cached; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.dsl.TypeSystemReference; import com.oracle.truffle.api.profiles.BranchProfile; @@ -47,7 +48,12 @@ import com.oracle.truffle.r.runtime.ops.BinaryArithmeticFactory; import com.oracle.truffle.r.runtime.ops.na.NACheck; /** - * TODO: handle "finite" parameter correctly. + * This node is used at several places, but only 'range' actually uses the 'finite' parameter, + * others should typically use {@code false} as its value. The 'finite' parameter is not handled + * consistently in GnuR: the documentation reads ‘finite = TRUE’ _includes_ ‘na.rm = TRUE’, but this + * only applies to some types (e.g. double or integer), for other types 'finite' seems to be ignored + * (e.g. logical). The only situation where semantics of finite is different to na.rm is double + * values: na.rm removes NA and NaN, but not -/+Inf. */ @TypeSystemReference(RTypes.class) public abstract class UnaryArithmeticReduceNode extends RBaseNode { @@ -110,7 +116,7 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode { } @Specialization - protected int doInt(int operand, boolean naRm, @SuppressWarnings("unused") boolean finite) { + protected int doInt(int operand, boolean naRm, boolean finite) { na.enable(operand); if (naRmProfile.profile(naRm)) { if (na.check(operand)) { @@ -125,16 +131,21 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode { } @Specialization - protected double doDouble(double operand, boolean naRm, @SuppressWarnings("unused") boolean finite) { + protected double doDouble(double operand, boolean naRm, boolean finite, + @Cached("createBinaryProfile()") ConditionProfile finiteProfile, + @Cached("createBinaryProfile()") ConditionProfile isInfiniteProfile) { na.enable(operand); - if (naRmProfile.profile(naRm)) { - if (na.check(operand)) { + if (naRmProfile.profile(naRm || finite)) { + boolean profiledFinite = finiteProfile.profile(finite); + if (na.checkNAorNaN(operand) || (profiledFinite && isInfiniteProfile.profile(!RRuntime.isFinite(operand)))) { + // the only value we have should be removed... emptyWarning(); return semantics.getIntStart(); } else { return operand; } } else { + // since !naRm and !finite, NaN or +/-Inf can be valid results return na.check(operand) ? RRuntime.DOUBLE_NA : operand; } } @@ -204,9 +215,9 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode { } @Specialization - protected int doIntVector(RIntVector operand, boolean naRm, @SuppressWarnings("unused") boolean finite) { + protected int doIntVector(RIntVector operand, boolean naRm, boolean finite) { RBaseNode.reportWork(this, operand.getLength()); - boolean profiledNaRm = naRmProfile.profile(naRm); + boolean profiledNaRm = naRmProfile.profile(naRm || finite); int result = semantics.getIntStart(); na.enable(operand); int opCount = 0; @@ -235,24 +246,33 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode { } @Specialization - protected double doDoubleVector(RDoubleVector operand, boolean naRm, @SuppressWarnings("unused") boolean finite) { + protected double doDoubleVector(RDoubleVector operand, boolean naRm, boolean finite, + @Cached("createBinaryProfile()") ConditionProfile finiteProfile, + @Cached("createBinaryProfile()") ConditionProfile isInfiniteProfile) { RBaseNode.reportWork(this, operand.getLength()); - boolean profiledNaRm = naRmProfile.profile(naRm); + boolean profiledNaRm = naRmProfile.profile(naRm || finite); + boolean profiledFinite = finiteProfile.profile(finite); double result = semantics.getDoubleStart(); na.enable(operand); int opCount = 0; double[] data = operand.getDataWithoutCopying(); for (int i = 0; i < operand.getLength(); i++) { double d = data[i]; - if (na.check(d)) { + if (na.checkNAorNaN(d)) { if (profiledNaRm) { - continue; - } else { + continue; // ignore NA/NaN + } else if (na.check(d)) { + // NA produces NA directly, but NaN should be handled by arithmetics.op to + // produce NaN. We cannot directly return NaN because if we encounter NA later + // on, we should return NA not NaN return RRuntime.DOUBLE_NA; } - } else { - result = arithmetic.op(result, d); + } else if (profiledFinite && isInfiniteProfile.profile(!RRuntime.isFinite(d))) { + // ignore -/+Inf if 'infinite == TRUE' + continue; } + + result = arithmetic.op(result, d); opCount++; } if (opCount == 0) { @@ -327,10 +347,10 @@ public abstract class UnaryArithmeticReduceNode extends RBaseNode { } @Specialization - protected RComplex doComplexVector(RComplexVector operand, boolean naRm, @SuppressWarnings("unused") boolean finite) { + protected RComplex doComplexVector(RComplexVector operand, boolean naRm, boolean finite) { RBaseNode.reportWork(this, operand.getLength()); if (semantics.supportComplex) { - boolean profiledNaRm = naRmProfile.profile(naRm); + boolean profiledNaRm = naRmProfile.profile(naRm || finite); RComplex result = RRuntime.double2complex(semantics.getDoubleStart()); int opCount = 0; na.enable(operand); diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test index 95ec299279cdc5a982b66bb159f2b0cc415577d0..2f006d4fad6ad5074a50a74ab48dc0e8595e5278 100644 --- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test +++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test @@ -41782,6 +41782,66 @@ NULL #argv <- list(structure(integer(0), .Label = character(0), class = 'factor'), TRUE, FALSE); .Internal(radixsort(argv[[1]], argv[[2]], argv[[3]])) NULL +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, -Inf, 3.44, Inf, 1.513)) +[1] -Inf Inf + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, -Inf, 3.44, Inf, 1.513), finite=T) +[1] 1.513 3.440 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, -Inf, 3.44, Inf, 1.513), na.rm=T) +[1] -Inf Inf + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, 2.62, 3.44, NA, NA, 1.513)) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, 2.62, 3.44, NA, NA, 1.513), finite=T) +[1] 1.513 3.440 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, 2.62, 3.44, NA, NA, 1.513), na.rm=T) +[1] 1.513 3.440 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, 2.62, 3.44, NaN, 1.513)) +[1] NaN NaN + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, 2.62, 3.44, NaN, 1.513), finite=T) +[1] 1.513 3.440 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithDoubles# +#range(c(1.615, 3.19, 2.62, 3.44, NaN, 1.513), na.rm=T) +[1] 1.513 3.440 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithIntegers# +#range(c(2L, 3L, NA, NA, 1L)) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithIntegers# +#range(c(2L, 3L, NA, NA, 1L), finite=T) +[1] 1 3 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithIntegers# +#range(c(2L, 3L, NA, NA, 1L), na.rm=T) +[1] 1 3 + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithLogical# +#range(c(T, F, NA, NA, T)) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithLogical# +#range(c(T, F, NA, NA, T), finite=T) +[1] NA NA + +##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testNaRmAndFiniteWithLogical# +#range(c(T, F, NA, NA, T), na.rm=T) +[1] 0 1 + ##com.oracle.truffle.r.test.builtins.TestBuiltin_range.testrange1# #argv <- list(c(0.0303542455381287, 0.030376780241572, 0.030376780241572, 0.0317964665585001, 0.0332612222823148, 0.0332612222823148, 0.0332612222823148, 0.0332612222823148, 0.0332612222823148, 0.0332612222823148, 0.0332612222823148, 0.0334189652064179, 0.0352217414818821, 0.0354245538128718, 0.0354245538128718, 0.0376780241572021, 0.0376780241572021, 0.0376780241572021, 0.0376780241572021, 0.0406300703082748, 0.0406300703082748, 0.0406300703082748, 0.0440778799351001, 0.048021453037678, 0.0524607896160087, 0.0524607896160087, 0.0524607896160087, 0.0628267531999279, 0.0693167477915991, 0.0981611681990265, 0.134937804218497, 0.179646655850009, 0.437804218496485));range(argv[[1]][[1]],argv[[1]][[2]], na.rm = FALSE); [1] 0.03035425 0.03037678 diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_range.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_range.java index 0eae3eaab5cacd9b8cf6ee22767d8f741c073f32..46cd9ba2ed297edc052f85606e5b4bc15a42af6c 100644 --- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_range.java +++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/builtins/TestBuiltin_range.java @@ -4,7 +4,7 @@ * http://www.gnu.org/licenses/gpl-2.0.html * * Copyright (c) 2014, Purdue University - * Copyright (c) 2014, 2016, Oracle and/or its affiliates + * Copyright (c) 2014, 2017, Oracle and/or its affiliates * * All rights reserved. */ @@ -156,4 +156,25 @@ public class TestBuiltin_range extends TestBase { public void testrange31() { assertEval("argv <- list(structure(c(1, 0.666666666666667, 0.333333333333333, 0, -0.333333333333333, -0.666666666666667, -1, -1.33333333333333, -1.66666666666667, 1.5, 1, 0.5, 0, -0.5, -1, -1.5, -2, -2.5, 3, 2, 1, 0, -1, -2, -3, -4, -5, -Inf, -Inf, -Inf, NaN, Inf, Inf, Inf, Inf, Inf, -3, -2, -1, 0, 1, 2, 3, 4, 5, -1.5, -1, -0.5, 0, 0.5, 1, 1.5, 2, 2.5, -1, -0.666666666666667, -0.333333333333333, 0, 0.333333333333333, 0.666666666666667, 1, 1.33333333333333, 1.66666666666667, -0.75, -0.5, -0.25, 0, 0.25, 0.5, 0.75, 1, 1.25, -0.6, -0.4, -0.2, 0, 0.2, 0.4, 0.6, 0.8, 1), .Dim = c(9L, 9L)));range(argv[[1]][[1]],argv[[1]][[2]], na.rm = TRUE);"); } + + // Following tests run range with the same input, but once with na.rm=F (default), na.rm=T and + // finite=T + private static final String[][] OPTIONAL_ARGS = new String[][]{{"", ", na.rm=T", ", finite=T"}}; + + @Test + public void testNaRmAndFiniteWithDoubles() { + assertEval(template("range(c(1.615, 3.19, 2.62, 3.44, NA, NA, 1.513)%0)", OPTIONAL_ARGS)); + assertEval(template("range(c(1.615, 3.19, 2.62, 3.44, NaN, 1.513)%0)", OPTIONAL_ARGS)); + assertEval(template("range(c(1.615, 3.19, -Inf, 3.44, Inf, 1.513)%0)", OPTIONAL_ARGS)); + } + + @Test + public void testNaRmAndFiniteWithIntegers() { + assertEval(template("range(c(2L, 3L, NA, NA, 1L)%0)", OPTIONAL_ARGS)); + } + + @Test + public void testNaRmAndFiniteWithLogical() { + assertEval(template("range(c(T, F, NA, NA, T)%0)", OPTIONAL_ARGS)); + } }