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 6036745 [SYSTEMDS-2929] Fix builtin namespace handling imports (merge
semantics)
6036745 is described below
commit 6036745ccc0c77d394f6a203f19c0197aa0dfea9
Author: Matthias Boehm <[email protected]>
AuthorDate: Sat Apr 17 21:55:20 2021 +0200
[SYSTEMDS-2929] Fix builtin namespace handling imports (merge semantics)
The previously reworked handling of builtin namespaces was important for
correct precedence ordering and builtin functions in different
namespaces. However, a remaining issue was that the builtin namespace of
imported functions overwrote any existing builtin namespace (with
potentially different functions) of the default namespace. We now
correctly merge these function dictionaries and add proper tests.
---
.../apache/sysds/parser/FunctionDictionary.java | 13 ++++++++++
.../apache/sysds/parser/dml/DMLParserWrapper.java | 12 ++++++---
.../sysds/parser/dml/DmlSyntacticValidator.java | 2 +-
.../test/functions/misc/FunctionNamespaceTest.java | 7 +++++
src/test/scripts/functions/misc/Functions15.dml | 30 ++++++++++++++++++++++
src/test/scripts/functions/misc/Functions15b.dml | 26 +++++++++++++++++++
6 files changed, 86 insertions(+), 4 deletions(-)
diff --git a/src/main/java/org/apache/sysds/parser/FunctionDictionary.java
b/src/main/java/org/apache/sysds/parser/FunctionDictionary.java
index 0f6ae5e..7eabc00 100644
--- a/src/main/java/org/apache/sysds/parser/FunctionDictionary.java
+++ b/src/main/java/org/apache/sysds/parser/FunctionDictionary.java
@@ -103,4 +103,17 @@ public class FunctionDictionary<T extends FunctionBlock> {
for( Entry<String,T> fe : _funs.entrySet() )
_funsOrig.put(fe.getKey(),
(T)fe.getValue().cloneFunctionBlock());
}
+
+ public void merge(FunctionDictionary<T> that) {
+ //merge optimized functions
+ for( Entry<String, T> e : that._funs.entrySet() )
+ if( !_funs.containsKey(e.getKey()) )
+ _funs.put(e.getKey(), e.getValue());
+
+ //merge unoptimized functions
+ if( _funsOrig != null && that._funsOrig != null )
+ for( Entry<String, T> e : that._funsOrig.entrySet() )
+ if( !_funsOrig.containsKey(e.getKey()) )
+ _funsOrig.put(e.getKey(), e.getValue());
+ }
}
diff --git a/src/main/java/org/apache/sysds/parser/dml/DMLParserWrapper.java
b/src/main/java/org/apache/sysds/parser/dml/DMLParserWrapper.java
index 4b5e282..c3f8ca5 100644
--- a/src/main/java/org/apache/sysds/parser/dml/DMLParserWrapper.java
+++ b/src/main/java/org/apache/sysds/parser/dml/DMLParserWrapper.java
@@ -259,8 +259,14 @@ public class DMLParserWrapper extends ParserWrapper
}
private static void addFunctions(DMLProgram dmlPgm, String namespace,
FunctionDictionary<FunctionStatementBlock> dict) {
- // TODO handle namespace key already exists for different
program value instead of overwriting
- if (dict != null)
- dmlPgm.getNamespaces().put(namespace, dict);
+ if( dict != null ) {
+ // merge function dictionary into existing dictionary
+ // (e.g., for builtin namespaces)
+ if( dmlPgm.getNamespaces().containsKey(namespace) )
+
dmlPgm.getNamespaces().get(namespace).merge(dict);
+ // add entire dictionary for non-existing namespace
+ else
+ dmlPgm.getNamespaces().put(namespace, dict);
+ }
}
}
diff --git
a/src/main/java/org/apache/sysds/parser/dml/DmlSyntacticValidator.java
b/src/main/java/org/apache/sysds/parser/dml/DmlSyntacticValidator.java
index 2c4e8a3..0d0e41a 100644
--- a/src/main/java/org/apache/sysds/parser/dml/DmlSyntacticValidator.java
+++ b/src/main/java/org/apache/sysds/parser/dml/DmlSyntacticValidator.java
@@ -1139,7 +1139,7 @@ public class DmlSyntacticValidator implements DmlListener
{
String filePath, String filePath2, DMLProgram prog ) {
info.namespaces = new HashMap<>();
if(prog != null) {
- info.namespaces.put(getQualifiedNamespace(namespace),
prog.getDefaultFunctionDictionary());
+ //add loaded namespaces (imported namespace already w/
correct name, not default)
for( Entry<String,
FunctionDictionary<FunctionStatementBlock>> e : prog.getNamespaces().entrySet()
)
info.namespaces.put(getQualifiedNamespace(e.getKey()), e.getValue());
ImportStatement istmt = new ImportStatement();
diff --git
a/src/test/java/org/apache/sysds/test/functions/misc/FunctionNamespaceTest.java
b/src/test/java/org/apache/sysds/test/functions/misc/FunctionNamespaceTest.java
index e8b016b..617c83f 100644
---
a/src/test/java/org/apache/sysds/test/functions/misc/FunctionNamespaceTest.java
+++
b/src/test/java/org/apache/sysds/test/functions/misc/FunctionNamespaceTest.java
@@ -49,6 +49,7 @@ public class FunctionNamespaceTest extends AutomatedTestBase
private final static String TEST_NAME12 = "Functions12";
private final static String TEST_NAME13 = "Functions13";
private final static String TEST_NAME14 = "Functions14";
+ private final static String TEST_NAME15 = "Functions15";
private final static String TEST_DIR = "functions/misc/";
private final static String TEST_CLASS_DIR = TEST_DIR +
FunctionNamespaceTest.class.getSimpleName() + "/";
@@ -73,6 +74,7 @@ public class FunctionNamespaceTest extends AutomatedTestBase
addTestConfiguration(TEST_NAME12, new
TestConfiguration(TEST_CLASS_DIR, TEST_NAME12));
addTestConfiguration(TEST_NAME13, new
TestConfiguration(TEST_CLASS_DIR, TEST_NAME13));
addTestConfiguration(TEST_NAME14, new
TestConfiguration(TEST_CLASS_DIR, TEST_NAME14));
+ addTestConfiguration(TEST_NAME15, new
TestConfiguration(TEST_CLASS_DIR, TEST_NAME15));
}
@Test
@@ -159,6 +161,11 @@ public class FunctionNamespaceTest extends
AutomatedTestBase
runFunctionNamespaceTest(TEST_NAME14);
}
+ @Test
+ public void testFunctionBuiltinNS() {
+ runFunctionNamespaceTest(TEST_NAME15);
+ }
+
private void runFunctionNamespaceTest(String TEST_NAME)
{
getAndLoadTestConfiguration(TEST_NAME);
diff --git a/src/test/scripts/functions/misc/Functions15.dml
b/src/test/scripts/functions/misc/Functions15.dml
new file mode 100644
index 0000000..32276e7
--- /dev/null
+++ b/src/test/scripts/functions/misc/Functions15.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.
+#
+#-------------------------------------------------------------
+
+source("./src/test/scripts/functions/misc/Functions15b.dml") as f1;
+
+X = rand(rows=10, cols=10, seed=1)
+
+Z = f1::foo(X)
+print(toString(Z))
+
+Y = imputeByMean(X, matrix(1, 1, ncol(X)))
+print(toString(Y))
diff --git a/src/test/scripts/functions/misc/Functions15b.dml
b/src/test/scripts/functions/misc/Functions15b.dml
new file mode 100644
index 0000000..6834f49
--- /dev/null
+++ b/src/test/scripts/functions/misc/Functions15b.dml
@@ -0,0 +1,26 @@
+#-------------------------------------------------------------
+#
+# 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)
+ return (Matrix[Double] Y)
+{
+ Y = winsorize(X, FALSE)
+}
