You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@systemds.apache.org by mb...@apache.org on 2020/10/10 18:27:55 UTC

[systemds] branch master updated: [SYSTEMDS-2684] Fix built-in function error handling, fix lasso args

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 8be71ec  [SYSTEMDS-2684] Fix built-in function error handling, fix lasso args
8be71ec is described below

commit 8be71ec01a80fa98b6e0e088a4476f65aabe7707
Author: Matthias Boehm <mb...@gmail.com>
AuthorDate: Sat Oct 10 20:27:35 2020 +0200

    [SYSTEMDS-2684] Fix built-in function error handling, fix lasso args
    
    This patch adds more robust error handling for the scenario of
    dml-bodied builtin functions whose scripts are are loaded but the
    functions are unavailable (e.g., due to typos).
    
    Furthermore, this also includes a fix of the lasso builtin function,
    where last minute renaming of function arguments (for consistency)
    failed the tests.
---
 scripts/builtin/lasso.dml                          |  6 +-
 scripts/builtin/xdummy1.dml                        | 23 ++++++++
 scripts/builtin/xdummy2.dml                        | 24 ++++++++
 .../java/org/apache/sysds/common/Builtins.java     |  4 +-
 .../sysds/parser/FunctionCallIdentifier.java       |  4 ++
 .../builtin/BuiltinErrorHandlingTest.java          | 69 ++++++++++++++++++++++
 src/test/scripts/functions/builtin/dummy1.dml      | 25 ++++++++
 src/test/scripts/functions/builtin/dummy2.dml      | 25 ++++++++
 8 files changed, 175 insertions(+), 5 deletions(-)

diff --git a/scripts/builtin/lasso.dml b/scripts/builtin/lasso.dml
index 7b6b464..3edadd9 100644
--- a/scripts/builtin/lasso.dml
+++ b/scripts/builtin/lasso.dml
@@ -67,7 +67,7 @@ m_lasso = function(Matrix[Double] X, Matrix[Double] y, Double tol = 10^(-15),
   inactive_set = matrix(1, rows=m, cols=1)
   iter = 0
   continue = TRUE
-  while(iter < maxiter & continue) {
+  while(iter < maxi & continue) {
     dw = matrix(0, rows=m, cols=1)
     dg = matrix(0, rows=m, cols=1)
     relChangeObj = -1.0
@@ -114,9 +114,7 @@ m_lasso = function(Matrix[Double] X, Matrix[Double] y, Double tol = 10^(-15),
     old_inactive_set = inactive_set
     inactive_set = w != 0
     diff = sum(abs(old_inactive_set - inactive_set))
-
-    if(diff == 0 & relChangeObj < tol)
-      continue = FALSE
+    continue = (diff != 0 | relChangeObj >= tol)
 
     num_inactive = sum(w != 0)
     if( verbose )
diff --git a/scripts/builtin/xdummy1.dml b/scripts/builtin/xdummy1.dml
new file mode 100644
index 0000000..187cf73
--- /dev/null
+++ b/scripts/builtin/xdummy1.dml
@@ -0,0 +1,23 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+
+m_xdummyTypo = function(Matrix[Double] X) return (Matrix[Double] Y) {
+  Y = cor(X);
+}
diff --git a/scripts/builtin/xdummy2.dml b/scripts/builtin/xdummy2.dml
new file mode 100644
index 0000000..286ba82
--- /dev/null
+++ b/scripts/builtin/xdummy2.dml
@@ -0,0 +1,24 @@
+#-------------------------------------------------------------
+#
+# 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.
+#
+
+m_xdummyTypo = function(Matrix[Double] X) return (Matrix[Double] Y, Matrix[Double] Z) {
+  Y = cor(X);
+  Z = Y;
+}
diff --git a/src/main/java/org/apache/sysds/common/Builtins.java b/src/main/java/org/apache/sysds/common/Builtins.java
index 157a411..a9e023e 100644
--- a/src/main/java/org/apache/sysds/common/Builtins.java
+++ b/src/main/java/org/apache/sysds/common/Builtins.java
@@ -232,7 +232,9 @@ public enum Builtins {
 	TRANSFORMDECODE("transformdecode", false, true),
 	TRANSFORMENCODE("transformencode", false, true),
 	TRANSFORMMETA("transformmeta", false, true),
-	UPPER_TRI("upper.tri", false, true);
+	UPPER_TRI("upper.tri", false, true),
+	XDUMMY1("xdummy1", true), //error handling test
+	XDUMMY2("xdummy2", true); //error handling test
 
 	Builtins(String name, boolean script) {
 		this(name, null, script, false, ReturnType.SINGLE_RETURN);
diff --git a/src/main/java/org/apache/sysds/parser/FunctionCallIdentifier.java b/src/main/java/org/apache/sysds/parser/FunctionCallIdentifier.java
index 497d591..533c3e0 100644
--- a/src/main/java/org/apache/sysds/parser/FunctionCallIdentifier.java
+++ b/src/main/java/org/apache/sysds/parser/FunctionCallIdentifier.java
@@ -134,6 +134,10 @@ public class FunctionCallIdentifier extends DataIdentifier
 			_name = Builtins.getInternalFName(_name, dt);
 			_namespace = DMLProgram.DEFAULT_NAMESPACE;
 			fblock = dmlp.getFunctionStatementBlock(_namespace, _name);
+			if( fblock == null ) {
+				raiseValidateError("Builtin function '"+_name+ "': script loaded "
+					+ "but function not found. Is there a typo in the function name?");
+			}
 		}
 		
 		// Step 6: validate default parameters (after block assignment)
diff --git a/src/test/java/org/apache/sysds/test/functions/builtin/BuiltinErrorHandlingTest.java b/src/test/java/org/apache/sysds/test/functions/builtin/BuiltinErrorHandlingTest.java
new file mode 100644
index 0000000..49a57f5
--- /dev/null
+++ b/src/test/java/org/apache/sysds/test/functions/builtin/BuiltinErrorHandlingTest.java
@@ -0,0 +1,69 @@
+/*
+ * 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.sysds.test.functions.builtin;
+
+import org.apache.sysds.parser.LanguageException;
+import org.apache.sysds.test.AutomatedTestBase;
+import org.apache.sysds.test.TestConfiguration;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class BuiltinErrorHandlingTest extends AutomatedTestBase {
+
+	private final static String TEST_NAME1 = "dummy1";
+	private final static String TEST_NAME2 = "dummy2";
+	private final static String TEST_DIR = "functions/builtin/";
+	private static final String TEST_CLASS_DIR = TEST_DIR + BuiltinErrorHandlingTest.class.getSimpleName() + "/";
+
+	@Override
+	public void setUp() {
+		addTestConfiguration(TEST_NAME1,new TestConfiguration(TEST_CLASS_DIR, TEST_NAME1, new String[]{"B"}));
+		addTestConfiguration(TEST_NAME2,new TestConfiguration(TEST_CLASS_DIR, TEST_NAME2, new String[]{"B"}));
+	}
+
+	@Test
+	public void runDummyTest1Return() {
+		runDummytest(TEST_NAME1);
+	}
+	
+	@Test
+	public void runDummyTest2Return2() {
+		runDummytest(TEST_NAME2);
+	}
+	
+	private void runDummytest(String testname) {
+		loadTestConfiguration(getTestConfiguration(testname));
+		String HOME = SCRIPT_DIR + TEST_DIR;
+		fullDMLScriptName = HOME + testname + ".dml";
+		List<String> proArgs = new ArrayList<>();
+		proArgs.add("-args");
+		proArgs.add(input("X"));
+		proArgs.add(output("Y"));
+		programArgs = proArgs.toArray(new String[proArgs.size()]);
+
+		double[][] X = getRandomMatrix(10, 5, 0, 1, 0.8, -1);
+		writeInputMatrixWithMTD("X", X, true);
+		
+		setOutputBuffering(true);
+		runTest(true, true, LanguageException.class, "typo", -1);
+	}
+}
diff --git a/src/test/scripts/functions/builtin/dummy1.dml b/src/test/scripts/functions/builtin/dummy1.dml
new file mode 100644
index 0000000..cb99407
--- /dev/null
+++ b/src/test/scripts/functions/builtin/dummy1.dml
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------
+#
+# 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 = read($1);
+# should raise and error w/ "typo"
+Y = xdummy1(X);
+write(Y, $2);
diff --git a/src/test/scripts/functions/builtin/dummy2.dml b/src/test/scripts/functions/builtin/dummy2.dml
new file mode 100644
index 0000000..f2dee46
--- /dev/null
+++ b/src/test/scripts/functions/builtin/dummy2.dml
@@ -0,0 +1,25 @@
+#-------------------------------------------------------------
+#
+# 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 = read($1);
+# should raise and error w/ "typo"
+[Y,Z] = xdummy2(X);
+write(Y, $2);