You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemml.apache.org by re...@apache.org on 2017/11/03 18:02:49 UTC
[47/50] [abbrv] systemml git commit: [SYSTEMML-1981] Fix graceful
value type casts on function invocations
[SYSTEMML-1981] Fix graceful value type casts on function invocations
The existing graceful value type casts - on function invocations with
wrong value type - incorrectly took the input parameter type instead of
the function parameter type for comparison. This patch fixes this
casting issue, which avoids unnecessary warnings on function invocations
and recompilation exceptions for boolean parameters.
Project: http://git-wip-us.apache.org/repos/asf/systemml/repo
Commit: http://git-wip-us.apache.org/repos/asf/systemml/commit/a2f0598c
Tree: http://git-wip-us.apache.org/repos/asf/systemml/tree/a2f0598c
Diff: http://git-wip-us.apache.org/repos/asf/systemml/diff/a2f0598c
Branch: refs/heads/master
Commit: a2f0598c606db16e75790cdbc3dbe37dc32d89a0
Parents: fc47891
Author: Matthias Boehm <mb...@gmail.com>
Authored: Wed Nov 1 21:37:49 2017 -0700
Committer: Matthias Boehm <mb...@gmail.com>
Committed: Thu Nov 2 00:39:16 2017 -0700
----------------------------------------------------------------------
.../cp/FunctionCallCPInstruction.java | 29 ++++----
.../functions/misc/FunctionReturnTest.java | 78 ++++++++++++++++++++
.../functions/misc/FunctionReturnBoolean.dml | 34 +++++++++
.../functions/misc/FunctionReturnInt.dml | 34 +++++++++
.../functions/misc/ZPackageSuite.java | 1 +
5 files changed, 160 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/systemml/blob/a2f0598c/src/main/java/org/apache/sysml/runtime/instructions/cp/FunctionCallCPInstruction.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/sysml/runtime/instructions/cp/FunctionCallCPInstruction.java b/src/main/java/org/apache/sysml/runtime/instructions/cp/FunctionCallCPInstruction.java
index 402d4a5..b901dfc 100644
--- a/src/main/java/org/apache/sysml/runtime/instructions/cp/FunctionCallCPInstruction.java
+++ b/src/main/java/org/apache/sysml/runtime/instructions/cp/FunctionCallCPInstruction.java
@@ -60,7 +60,6 @@ public class FunctionCallCPInstruction extends CPInstruction {
private FunctionCallCPInstruction(String namespace, String functName, ArrayList<CPOperand> boundInParamOperands,
ArrayList<String> boundInParamNames, ArrayList<String> boundOutParamNames, String istr) {
super(null, functName, istr);
-
_cptype = CPINSTRUCTION_TYPE.External;
_functionName = functName;
_namespace = namespace;
@@ -72,7 +71,7 @@ public class FunctionCallCPInstruction extends CPInstruction {
public static FunctionCallCPInstruction parseInstruction(String str)
throws DMLRuntimeException
- {
+ {
//schema: extfunct, fname, num inputs, num outputs, inputs, outputs
String[] parts = InstructionUtils.getInstructionPartsWithValueType ( str );
String namespace = parts[1];
@@ -94,8 +93,7 @@ public class FunctionCallCPInstruction extends CPInstruction {
return new FunctionCallCPInstruction ( namespace,functionName,
boundInParamOperands, boundInParamNames, boundOutParamNames, str );
}
-
-
+
@Override
public Instruction preprocessInstruction(ExecutionContext ec)
throws DMLRuntimeException
@@ -114,7 +112,7 @@ public class FunctionCallCPInstruction extends CPInstruction {
@Override
public void processInstruction(ExecutionContext ec)
throws DMLRuntimeException
- {
+ {
if( LOG.isTraceEnabled() ){
LOG.trace("Executing instruction : " + this.toString());
}
@@ -130,19 +128,19 @@ public class FunctionCallCPInstruction extends CPInstruction {
// create bindings to formal parameters for given function call
// These are the bindings passed to the FunctionProgramBlock for function execution
- LocalVariableMap functionVariables = new LocalVariableMap();
+ LocalVariableMap functionVariables = new LocalVariableMap();
for( int i=0; i<fpb.getInputParams().size(); i++)
- {
+ {
DataIdentifier currFormalParam = fpb.getInputParams().get(i);
String currFormalParamName = currFormalParam.getName();
Data currFormalParamValue = null;
-
+
CPOperand operand = _boundInputParamOperands.get(i);
String varname = operand.getName();
//error handling non-existing variables
if( !operand.isLiteral() && !ec.containsVariable(varname) ) {
throw new DMLRuntimeException("Input variable '"+varname+"' not existing on call of " +
- DMLProgram.constructFunctionKey(_namespace, _functionName) + " (line "+getLineNum()+").");
+ DMLProgram.constructFunctionKey(_namespace, _functionName) + " (line "+getLineNum()+").");
}
//get input matrix/frame/scalar
currFormalParamValue = (operand.getDataType()!=DataType.SCALAR) ? ec.getVariable(varname) :
@@ -150,19 +148,18 @@ public class FunctionCallCPInstruction extends CPInstruction {
//graceful value type conversion for scalar inputs with wrong type
if( currFormalParamValue.getDataType() == DataType.SCALAR
- && currFormalParamValue.getValueType() != operand.getValueType() )
+ && currFormalParamValue.getValueType() != currFormalParam.getValueType() )
{
- ScalarObject so = (ScalarObject) currFormalParamValue;
- currFormalParamValue = ScalarObjectFactory
- .createScalarObject(operand.getValueType(), so);
+ currFormalParamValue = ScalarObjectFactory.createScalarObject(
+ currFormalParam.getValueType(), (ScalarObject) currFormalParamValue);
}
- functionVariables.put(currFormalParamName, currFormalParamValue);
+ functionVariables.put(currFormalParamName, currFormalParamValue);
}
// Pin the input variables so that they do not get deleted
// from pb's symbol table at the end of execution of function
- HashMap<String,Boolean> pinStatus = ec.pinVariables(_boundInputParamNames);
+ HashMap<String,Boolean> pinStatus = ec.pinVariables(_boundInputParamNames);
// Create a symbol table under a new execution context for the function invocation,
// and copy the function arguments into the created table.
@@ -185,7 +182,7 @@ public class FunctionCallCPInstruction extends CPInstruction {
String fname = DMLProgram.constructFunctionKey(_namespace, _functionName);
throw new DMLRuntimeException("error executing function " + fname, e);
}
- LocalVariableMap retVars = fn_ec.getVariables();
+ LocalVariableMap retVars = fn_ec.getVariables();
// cleanup all returned variables w/o binding
Collection<String> retVarnames = new LinkedList<>(retVars.keySet());
http://git-wip-us.apache.org/repos/asf/systemml/blob/a2f0598c/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionReturnTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionReturnTest.java b/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionReturnTest.java
new file mode 100644
index 0000000..b83ac39
--- /dev/null
+++ b/src/test/java/org/apache/sysml/test/integration/functions/misc/FunctionReturnTest.java
@@ -0,0 +1,78 @@
+/*
+ * 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.
+ */
+
+package org.apache.sysml.test.integration.functions.misc;
+
+import org.junit.Test;
+
+import org.apache.sysml.hops.OptimizerUtils;
+import org.apache.sysml.test.integration.AutomatedTestBase;
+import org.apache.sysml.test.integration.TestConfiguration;
+
+public class FunctionReturnTest extends AutomatedTestBase
+{
+ private final static String TEST_DIR = "functions/misc/";
+ private final static String TEST_NAME1 = "FunctionReturnInt";
+ private final static String TEST_NAME2 = "FunctionReturnBoolean";
+ private final static String TEST_CLASS_DIR = TEST_DIR + FunctionReturnTest.class.getSimpleName() + "/";
+
+ @Override
+ public void setUp() {
+ addTestConfiguration( TEST_NAME1, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1, new String[] { "Rout" }) );
+ addTestConfiguration( TEST_NAME2, new TestConfiguration(TEST_CLASS_DIR, TEST_NAME2, new String[] { "Rout" }) );
+ }
+
+ @Test
+ public void testFunctionReturnInt() {
+ runFunctionReturnTest(TEST_NAME1, false);
+ }
+
+ @Test
+ public void testFunctionReturnBool() {
+ runFunctionReturnTest(TEST_NAME2, false);
+ }
+
+ @Test
+ public void testFunctionReturnIntIPA() {
+ runFunctionReturnTest(TEST_NAME1, true);
+ }
+
+ @Test
+ public void testFunctionReturnBoolIPA() {
+ runFunctionReturnTest(TEST_NAME2, true);
+ }
+
+ private void runFunctionReturnTest( String testname, boolean IPA ) {
+ boolean oldIPA = OptimizerUtils.ALLOW_INTER_PROCEDURAL_ANALYSIS;
+ OptimizerUtils.ALLOW_INTER_PROCEDURAL_ANALYSIS = IPA;
+ try {
+ TestConfiguration config = getTestConfiguration(testname);
+ loadTestConfiguration(config);
+
+ String HOME = SCRIPT_DIR + TEST_DIR;
+ fullDMLScriptName = HOME + testname + ".dml";
+ programArgs = new String[]{"-explain"};
+
+ runTest(true, false, null, -1);
+ }
+ finally {
+ OptimizerUtils.ALLOW_INTER_PROCEDURAL_ANALYSIS = oldIPA;
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/systemml/blob/a2f0598c/src/test/scripts/functions/misc/FunctionReturnBoolean.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/FunctionReturnBoolean.dml b/src/test/scripts/functions/misc/FunctionReturnBoolean.dml
new file mode 100644
index 0000000..afa563b
--- /dev/null
+++ b/src/test/scripts/functions/misc/FunctionReturnBoolean.dml
@@ -0,0 +1,34 @@
+#-------------------------------------------------------------
+#
+# 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 input) return (boolean out) {
+ out = input;
+ for(i in 1:2) {
+ tmp = sum(X);
+ out = input & as.logical(tmp);
+ }
+}
+
+X = seq(1,100);
+xmax = max(X);
+y = foo(X, xmax);
+print(y);
http://git-wip-us.apache.org/repos/asf/systemml/blob/a2f0598c/src/test/scripts/functions/misc/FunctionReturnInt.dml
----------------------------------------------------------------------
diff --git a/src/test/scripts/functions/misc/FunctionReturnInt.dml b/src/test/scripts/functions/misc/FunctionReturnInt.dml
new file mode 100644
index 0000000..ba916a4
--- /dev/null
+++ b/src/test/scripts/functions/misc/FunctionReturnInt.dml
@@ -0,0 +1,34 @@
+#-------------------------------------------------------------
+#
+# 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, int input) return (int out) {
+ out = input;
+ for(i in 1:2) {
+ tmp = sum(X);
+ out = input + as.integer(tmp);
+ }
+}
+
+X = seq(1,100);
+xmax = max(X);
+y = foo(X, xmax);
+print(y);
http://git-wip-us.apache.org/repos/asf/systemml/blob/a2f0598c/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
----------------------------------------------------------------------
diff --git a/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java b/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
index cac39e1..a453cbd 100644
--- a/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
+++ b/src/test_suites/java/org/apache/sysml/test/integration/functions/misc/ZPackageSuite.java
@@ -31,6 +31,7 @@ import org.junit.runners.Suite;
DataTypeChangeTest.class,
FunctionInliningTest.class,
FunctionNamespaceTest.class,
+ FunctionReturnTest.class,
IfTest.class,
InvalidBuiltinFunctionCallTest.class,
InvalidFunctionAssignmentTest.class,