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 5d73570 [SYSTEMDS-3004] Fix function inlining w/ partially bound
outputs
5d73570 is described below
commit 5d735700d7872396163d19ea9e21c2dfbf66c0de
Author: Matthias Boehm <[email protected]>
AuthorDate: Wed Jun 2 20:56:29 2021 +0200
[SYSTEMDS-3004] Fix function inlining w/ partially bound outputs
This patch improves the IPA pass for function inlining to no longer only
apply to fully bound outputs but also partially bound outputs which are
assigned by position as they are defined. For example, a function that
returns (a,b,c) and a function call only bind these to [x,y], not
inlines the sub-DAGs of a, and b. The advantages are more inlining, and
even partial redundancy elimination if certain inputs are not needed.
---
.../org/apache/sysds/hops/ipa/IPAPassInlineFunctions.java | 14 ++++++++++----
.../test/functions/compress/WorkloadAnalysisTest.java | 6 +++++-
src/test/scripts/functions/compress/WorkloadAnalysisLm.dml | 6 +-----
.../scripts/functions/compress/WorkloadAnalysisMlogreg.dml | 6 +-----
4 files changed, 17 insertions(+), 15 deletions(-)
diff --git
a/src/main/java/org/apache/sysds/hops/ipa/IPAPassInlineFunctions.java
b/src/main/java/org/apache/sysds/hops/ipa/IPAPassInlineFunctions.java
index 77ca7ba..dee5617 100644
--- a/src/main/java/org/apache/sysds/hops/ipa/IPAPassInlineFunctions.java
+++ b/src/main/java/org/apache/sysds/hops/ipa/IPAPassInlineFunctions.java
@@ -21,6 +21,7 @@ package org.apache.sysds.hops.ipa;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -85,8 +86,12 @@ public class IPAPassInlineFunctions extends IPAPass
LOG.debug("-- inline '"+fkey+"'
at line "+op.getBeginLine());
//step 0: robustness for special cases
(named args, paramserv)
+ // (no need to check for equal output
length as the IPA and runtime is
+ // robust enough to handle the case of
partially bound outputs and
+ // inlining should not depend on it,
however, binding more outputs
+ // than the function returns is
impossible and not inlined)
if( op.getInput().size() !=
fstmt.getInputParams().size()
- ||
op.getOutputVariableNames().length != fstmt.getOutputParams().size()
+ ||
op.getOutputVariableNames().length > fstmt.getOutputParams().size()
|| op.isPseudoFunctionCall() ) {
removedAll = false;
continue;
@@ -112,12 +117,13 @@ public class IPAPassInlineFunctions extends IPAPass
String[] opOutputs =
op.getOutputVariableNames();
for(int j=0; j<opOutputs.length; j++)
outMap.put(fstmt.getOutputParams().get(j).getName(), opOutputs[j]);
- for(int j=0; j<hops2.size(); j++) {
- Hop out = hops2.get(j);
+ Iterator<Hop> iterFout =
hops2.iterator();
+ while( iterFout.hasNext() ) {
+ Hop out = iterFout.next();
if( HopRewriteUtils.isData(out,
OpOpData.TRANSIENTWRITE) ) {
out.setName(outMap.get(out.getName()));
if( out.getName() ==
null )
- hops2.remove(j);
+
iterFout.remove(); //not all outputs bound
}
}
fcallsSB.get(i).getHops().remove(op);
diff --git
a/src/test/java/org/apache/sysds/test/functions/compress/WorkloadAnalysisTest.java
b/src/test/java/org/apache/sysds/test/functions/compress/WorkloadAnalysisTest.java
index 5585617..59cb067 100644
---
a/src/test/java/org/apache/sysds/test/functions/compress/WorkloadAnalysisTest.java
+++
b/src/test/java/org/apache/sysds/test/functions/compress/WorkloadAnalysisTest.java
@@ -26,6 +26,7 @@ import org.apache.sysds.hops.ipa.InterProceduralAnalysis;
import org.apache.sysds.test.AutomatedTestBase;
import org.apache.sysds.test.TestConfiguration;
import org.apache.sysds.test.TestUtils;
+import org.junit.Assert;
import org.junit.Test;
public class WorkloadAnalysisTest extends AutomatedTestBase
@@ -64,7 +65,7 @@ public class WorkloadAnalysisTest extends AutomatedTestBase
InterProceduralAnalysis.CLA_WORKLOAD_ANALYSIS = true;
String HOME = SCRIPT_DIR + TEST_DIR;
fullDMLScriptName = HOME + testname + ".dml";
- programArgs = new String[]{"-args", input("X"),
input("y"), output("B") };
+ programArgs = new String[]{"-stats","-args",
input("X"), input("y"), output("B") };
double[][] X = getRandomMatrix(10000, 20, 0, 1, 1.0, 7);
writeInputMatrixWithMTD("X", X, false);
@@ -74,6 +75,9 @@ public class WorkloadAnalysisTest extends AutomatedTestBase
runTest(true, false, null, -1);
//TODO check for compressed operations
//(right now test only checks that the workload
analysis does not crash)
+
+ //check various additional expectations
+
Assert.assertFalse(heavyHittersContainsString("m_scale"));
}
finally {
resetExecMode(oldPlatform);
diff --git a/src/test/scripts/functions/compress/WorkloadAnalysisLm.dml
b/src/test/scripts/functions/compress/WorkloadAnalysisLm.dml
index ccaec92..3a9686c 100644
--- a/src/test/scripts/functions/compress/WorkloadAnalysisLm.dml
+++ b/src/test/scripts/functions/compress/WorkloadAnalysisLm.dml
@@ -22,11 +22,7 @@
X = read($1);
y = read($2);
-#X = scale(X=X, scale=TRUE, center=TRUE);
-X = X - colMeans(X);
-X = X / sqrt(colVars(X));
-
-while(FALSE){}
+X = scale(X=X, scale=TRUE, center=TRUE);
B = lm(X=X, y=y, verbose=TRUE);
write(B, $3)
diff --git a/src/test/scripts/functions/compress/WorkloadAnalysisMlogreg.dml
b/src/test/scripts/functions/compress/WorkloadAnalysisMlogreg.dml
index 7f0a76a..4281a5e 100644
--- a/src/test/scripts/functions/compress/WorkloadAnalysisMlogreg.dml
+++ b/src/test/scripts/functions/compress/WorkloadAnalysisMlogreg.dml
@@ -22,11 +22,7 @@
X = read($1);
y = read($2);
-#X = scale(X=X, scale=TRUE, center=TRUE);
-X = X - colMeans(X);
-X = X / sqrt(colVars(X));
-
-while(FALSE){}
+X = scale(X=X, scale=TRUE, center=TRUE);
B = multiLogReg(X=X, Y=y, verbose=TRUE);
write(B, $3)
