[SYSTEMDS-2652] Fix function deep copy for eval fcalls (for predicates)
This patch fixes an issue of the deep copy of FOR statement blocks if
eval requires to keep the unoptimized functions. Before we introduced
eval, this deep copy was only used for uncontended recompilation in
parfor and thus, predicate hops (from, to, increment) were only copied
if necessary. However, this is invalid for the creation of unoptimized
functions from which otherwise no valid execution plans can be compiled.
This led to missing predicate variables because the predicate
instructions were missing.
diff --git a/src/main/java/org/apache/sysds/parser/ForStatement.java b/src/main/java/org/apache/sysds/parser/ForStatement.java
index 293e442..d8ed3b4 100644
--- a/src/main/java/org/apache/sysds/parser/ForStatement.java
+++ b/src/main/java/org/apache/sysds/parser/ForStatement.java
@@ -104,5 +104,4 @@
LOG.error(this.printErrorLocation() + "should not call variablesRead from ForStatement ");
return new VariableSet();
}
-}
-
+}
diff --git a/src/main/java/org/apache/sysds/runtime/util/ProgramConverter.java b/src/main/java/org/apache/sysds/runtime/util/ProgramConverter.java
index 144f347..3bba363 100644
--- a/src/main/java/org/apache/sysds/runtime/util/ProgramConverter.java
+++ b/src/main/java/org/apache/sysds/runtime/util/ProgramConverter.java
@@ -700,18 +700,12 @@
ret.setStatements( sb.getStatements() );
//deep copy predicate hops dag for concurrent recompile
- if( sb.requiresFromRecompilation() ){
- Hop hops = Recompiler.deepCopyHopsDag( sb.getFromHops() );
- ret.setFromHops( hops );
- }
- if( sb.requiresToRecompilation() ){
- Hop hops = Recompiler.deepCopyHopsDag( sb.getToHops() );
- ret.setToHops( hops );
- }
- if( sb.requiresIncrementRecompilation() ){
- Hop hops = Recompiler.deepCopyHopsDag( sb.getIncrementHops() );
- ret.setIncrementHops( hops );
- }
+ //or on create full statement block copies
+ ret.setFromHops( Recompiler.deepCopyHopsDag(sb.getFromHops()));
+ ret.setToHops(Recompiler.deepCopyHopsDag(sb.getToHops()));
+ if( sb.getIncrementHops() != null )
+ ret.setIncrementHops(Recompiler.deepCopyHopsDag(sb.getIncrementHops()));
+
ret.updatePredicateRecompilationFlags();
ret.setNondeterministic(sb.isNondeterministic());
}
diff --git a/src/test/java/org/apache/sysds/test/functions/misc/FunctionPotpourriTest.java b/src/test/java/org/apache/sysds/test/functions/misc/FunctionPotpourriTest.java
index 2c73485..814d116 100644
--- a/src/test/java/org/apache/sysds/test/functions/misc/FunctionPotpourriTest.java
+++ b/src/test/java/org/apache/sysds/test/functions/misc/FunctionPotpourriTest.java
@@ -25,32 +25,35 @@
import org.apache.sysds.test.TestConfiguration;
import org.apache.sysds.test.TestUtils;
import org.junit.Assert;
-import org.junit.Ignore;
import org.junit.Test;
public class FunctionPotpourriTest extends AutomatedTestBase
{
- private final static String TEST_NAME1 = "FunPotpourriNoReturn";
- private final static String TEST_NAME2 = "FunPotpourriComments";
- private final static String TEST_NAME3 = "FunPotpourriNoReturn2";
- private final static String TEST_NAME4 = "FunPotpourriEval";
- private final static String TEST_NAME5 = "FunPotpourriSubsetReturn";
- private final static String TEST_NAME6 = "FunPotpourriSubsetReturnDead";
- private final static String TEST_NAME7 = "FunPotpourriNamedArgsSingle";
- private final static String TEST_NAME8 = "FunPotpourriNamedArgsMulti";
- private final static String TEST_NAME9 = "FunPotpourriNamedArgsPartial";
- private final static String TEST_NAME10 = "FunPotpourriNamedArgsUnknown1";
- private final static String TEST_NAME11 = "FunPotpourriNamedArgsUnknown2";
- private final static String TEST_NAME12 = "FunPotpourriNamedArgsIPA";
- private final static String TEST_NAME13 = "FunPotpourriDefaultArgScalar";
- private final static String TEST_NAME14 = "FunPotpourriDefaultArgMatrix";
- private final static String TEST_NAME15 = "FunPotpourriDefaultArgScalarMatrix1";
- private final static String TEST_NAME16 = "FunPotpourriDefaultArgScalarMatrix2";
- private final static String TEST_NAME17 = "FunPotpourriNamedArgsQuotedAssign";
- private final static String TEST_NAME18 = "FunPotpourriMultiReturnBuiltin1";
- private final static String TEST_NAME19 = "FunPotpourriMultiReturnBuiltin2";
- private final static String TEST_NAME20 = "FunPotpourriNestedParforEval";
- private final static String TEST_NAME21 = "FunPotpourriMultiEval";
+ private final static String[] TEST_NAMES = new String[] {
+ "FunPotpourriNoReturn",
+ "FunPotpourriComments",
+ "FunPotpourriNoReturn2",
+ "FunPotpourriEval",
+ "FunPotpourriSubsetReturn",
+ "FunPotpourriSubsetReturnDead",
+ "FunPotpourriNamedArgsSingle",
+ "FunPotpourriNamedArgsMulti",
+ "FunPotpourriNamedArgsPartial",
+ "FunPotpourriNamedArgsUnknown1",
+ "FunPotpourriNamedArgsUnknown2",
+ "FunPotpourriNamedArgsIPA",
+ "FunPotpourriDefaultArgScalar",
+ "FunPotpourriDefaultArgMatrix",
+ "FunPotpourriDefaultArgScalarMatrix1",
+ "FunPotpourriDefaultArgScalarMatrix2",
+ "FunPotpourriNamedArgsQuotedAssign",
+ "FunPotpourriMultiReturnBuiltin1",
+ "FunPotpourriMultiReturnBuiltin2",
+ "FunPotpourriNestedParforEval",
+ "FunPotpourriMultiEval",
+ "FunPotpourriEvalPred",
+ "FunPotpourriEvalListArg",
+ };
private final static String TEST_DIR = "functions/misc/";
private final static String TEST_CLASS_DIR = TEST_DIR + FunctionPotpourriTest.class.getSimpleName() + "/";
@@ -58,149 +61,129 @@
@Override
public void setUp() {
TestUtils.clearAssertionInformation();
- addTestConfiguration( TEST_NAME1, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME2, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME2, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME3, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME3, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME4, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME4, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME5, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME5, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME6, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME6, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME7, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME7, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME8, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME8, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME9, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME9, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME10, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME10, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME11, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME11, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME12, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME12, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME13, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME13, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME14, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME14, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME15, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME15, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME16, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME16, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME17, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME17, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME18, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME18, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME19, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME19, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME20, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME20, new String[] { "R" }) );
- addTestConfiguration( TEST_NAME21, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME21, new String[] { "R" }) );
+ for(String testName : TEST_NAMES)
+ addTestConfiguration( testName, new TestConfiguration(TEST_CLASS_DIR, testName, new String[] { "R" }) );
}
@Test
public void testFunctionNoReturn() {
- runFunctionTest( TEST_NAME1, null );
+ runFunctionTest( TEST_NAMES[0], null );
}
@Test
public void testFunctionComments() {
- runFunctionTest( TEST_NAME2, null );
+ runFunctionTest( TEST_NAMES[1], null );
}
@Test
public void testFunctionNoReturnSpec() {
- runFunctionTest( TEST_NAME3, null );
+ runFunctionTest( TEST_NAMES[2], null );
}
@Test
public void testFunctionEval() {
- runFunctionTest( TEST_NAME4, null );
+ runFunctionTest( TEST_NAMES[3], null );
}
@Test
public void testFunctionSubsetReturn() {
- runFunctionTest( TEST_NAME5, null );
+ runFunctionTest( TEST_NAMES[4], null );
}
@Test
public void testFunctionSubsetReturnDead() {
- runFunctionTest( TEST_NAME6, null );
- }
-
- @Test
- @Ignore
- public void testFunctionNamedArgsSingle() {
- runFunctionTest( TEST_NAME7, ParseException.class );
+ runFunctionTest( TEST_NAMES[5], null );
}
@Test
public void testFunctionNamedArgsSingleErr() {
- runFunctionTest( TEST_NAME7, ParseException.class );
- }
-
- @Test
- @Ignore
- public void testFunctionNamedArgsMulti() {
- runFunctionTest( TEST_NAME8, ParseException.class);
+ runFunctionTest( TEST_NAMES[6], ParseException.class );
}
@Test
public void testFunctionNamedArgsMultiErr() {
- runFunctionTest( TEST_NAME8, ParseException.class );
+ runFunctionTest( TEST_NAMES[7], ParseException.class );
}
@Test
public void testFunctionNamedArgsPartial() {
- runFunctionTest( TEST_NAME9, LanguageException.class );
+ runFunctionTest( TEST_NAMES[8], LanguageException.class );
}
@Test
public void testFunctionNamedArgsUnkown1() {
- runFunctionTest( TEST_NAME10, LanguageException.class );
+ runFunctionTest( TEST_NAMES[9], LanguageException.class );
}
@Test
public void testFunctionNamedArgsUnkown2() {
- runFunctionTest( TEST_NAME11, NullPointerException.class );
+ runFunctionTest( TEST_NAMES[10], NullPointerException.class );
}
@Test
public void testFunctionNamedArgsIPA() {
- runFunctionTest( TEST_NAME12, null );
+ runFunctionTest( TEST_NAMES[11], null );
}
@Test
public void testFunctionDefaultArgsScalar() {
- runFunctionTest( TEST_NAME13, null );
+ runFunctionTest( TEST_NAMES[12], null );
}
@Test
public void testFunctionDefaultArgsMatrix() {
- runFunctionTest( TEST_NAME14, null );
+ runFunctionTest( TEST_NAMES[13], null );
}
@Test
public void testFunctionDefaultArgsScalarMatrix1() {
- runFunctionTest( TEST_NAME15, null );
+ runFunctionTest( TEST_NAMES[14], null );
}
@Test
public void testFunctionDefaultArgsScalarMatrix2() {
- runFunctionTest( TEST_NAME16, null );
+ runFunctionTest( TEST_NAMES[15], null );
}
@Test
public void testFunctionNamedArgsQuotedAssign() {
- runFunctionTest( TEST_NAME17, null );
+ runFunctionTest( TEST_NAMES[16], null );
}
@Test
public void testFunctionMultiReturnBuiltin1() {
- runFunctionTest( TEST_NAME18, null );
+ runFunctionTest( TEST_NAMES[17], null );
}
@Test
public void testFunctionMultiReturnBuiltin2() {
- runFunctionTest( TEST_NAME19, null );
+ runFunctionTest( TEST_NAMES[18], null );
}
@Test
public void testFunctionNestedParforEval() {
- runFunctionTest( TEST_NAME20, null );
+ runFunctionTest( TEST_NAMES[19], null );
}
@Test
public void testFunctionMultiEval() {
- runFunctionTest( TEST_NAME21, null );
+ runFunctionTest( TEST_NAMES[20], null );
}
+ @Test
+ public void testFunctionEvalPred() {
+ runFunctionTest( TEST_NAMES[21], null );
+ }
+
+// @Test
+// public void testFunctionEvalListArg() {
+// runFunctionTest( TEST_NAMES[22], null );
+// }
+
private void runFunctionTest(String testName, Class<?> error) {
TestConfiguration config = getTestConfiguration(testName);
loadTestConfiguration(config);
+ setOutputBuffering(false);
String HOME = SCRIPT_DIR + TEST_DIR;
fullDMLScriptName = HOME + testName + ".dml";
@@ -209,7 +192,7 @@
runTest(true, error != null, error, -1);
- if( testName.equals(TEST_NAME18) )
+ if( testName.equals(TEST_NAMES[17]) )
Assert.assertTrue(heavyHittersContainsString("print"));
}
}
diff --git a/src/test/scripts/functions/misc/FunPotpourriEvalPred.dml b/src/test/scripts/functions/misc/FunPotpourriEvalPred.dml
new file mode 100644
index 0000000..def5004
--- /dev/null
+++ b/src/test/scripts/functions/misc/FunPotpourriEvalPred.dml
@@ -0,0 +1,30 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+#-------------------------------------------------------------
+
+foo = function(Matrix[Double] X, Boolean verbose) return (Matrix[Double] s){
+ s = as.matrix(5)
+ for(i in 1:ncol(X))
+ print("this is i " +i)
+}
+
+X = rand(rows=100, cols=100)
+s = eval("foo", list(X, FALSE))
+print(toString(s))
