[systemds] branch master updated: [SYSTEMDS-2628] Fix repeated eval function calls (on-demand loading)

[mb...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mboehm7 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/systemds.git


The following commit(s) were added to refs/heads/master by this push:
     new f2b5d78  [SYSTEMDS-2628] Fix repeated eval function calls (on-demand loading)
f2b5d78 is described below

commit f2b5d788a8fac49cf7c9722c4a23b3a8b2c32b68
Author: Matthias Boehm <mb...@gmail.com>
AuthorDate: Wed Aug 19 17:43:23 2020 +0200

    [SYSTEMDS-2628] Fix repeated eval function calls (on-demand loading)
    
    This patch fixes repeated eval function calls of the same function,
    which succeeded on the first attempt but subsequent attempts failed. The
    issue was the new handling of optimized and unoptimized functions, where
    the dynamically compiled function was only added to unoptimized
    functions. For this reason the next contains check failed, but the load
    and compile determines that the function was already loaded and hence
    returns null.
---
 .../instructions/cp/EvalNaryCPInstruction.java     |  2 +-
 .../test/functions/misc/FunctionPotpourriTest.java |  7 +++++
 .../functions/misc/FunPotpourriMultiEval.dml       | 30 ++++++++++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/apache/sysds/runtime/instructions/cp/EvalNaryCPInstruction.java b/src/main/java/org/apache/sysds/runtime/instructions/cp/EvalNaryCPInstruction.java
index f870cc4..2cfd804 100644
--- a/src/main/java/org/apache/sysds/runtime/instructions/cp/EvalNaryCPInstruction.java
+++ b/src/main/java/org/apache/sysds/runtime/instructions/cp/EvalNaryCPInstruction.java
@@ -183,7 +183,7 @@ public class EvalNaryCPInstruction extends BuiltinNaryCPInstruction {
 			if( !prog.containsFunctionProgramBlock(null, fsb.getKey(), false) ) {
 				FunctionProgramBlock fpb = (FunctionProgramBlock) dmlt
 					.createRuntimeProgramBlock(prog, fsb.getValue(), ConfigurationManager.getDMLConfig());
-				//prog.addFunctionProgramBlock(null, fsb.getKey(), fpb, true); // optimized
+				prog.addFunctionProgramBlock(null, fsb.getKey(), fpb, true); // optimized
 				prog.addFunctionProgramBlock(null, fsb.getKey(), fpb, false);    // unoptimized -> eval
 			}
 		}
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 69db305..2c73485 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
@@ -50,6 +50,7 @@ public class FunctionPotpourriTest extends AutomatedTestBase
 	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_DIR = "functions/misc/";
 	private final static String TEST_CLASS_DIR = TEST_DIR + FunctionPotpourriTest.class.getSimpleName() + "/";
@@ -77,6 +78,7 @@ public class FunctionPotpourriTest extends AutomatedTestBase
 		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" }) );
 	}
 
 	@Test
@@ -191,6 +193,11 @@ public class FunctionPotpourriTest extends AutomatedTestBase
 		runFunctionTest( TEST_NAME20, null );
 	}
 	
+	@Test
+	public void testFunctionMultiEval() {
+		runFunctionTest( TEST_NAME21, null );
+	}
+	
 	private void runFunctionTest(String testName, Class<?> error) {
 		TestConfiguration config = getTestConfiguration(testName);
 		loadTestConfiguration(config);
diff --git a/src/test/scripts/functions/misc/FunPotpourriMultiEval.dml b/src/test/scripts/functions/misc/FunPotpourriMultiEval.dml
new file mode 100644
index 0000000..72b7b7b
--- /dev/null
+++ b/src/test/scripts/functions/misc/FunPotpourriMultiEval.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.
+#
+#-------------------------------------------------------------
+
+X = rand(rows=10, cols= 10)
+t1 = interQuartileMean(X[,7]);
+
+for(i in 1:5)
+  X = eval("winsorize", list(X, FALSE))
+
+t2 = interQuartileMean(X[,7]);
+print("expected=TRUE, actual="+(t2 < t1))
+