This is an automated email from the ASF dual-hosted git repository.
mboehm7 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/systemds.git
The following commit(s) were added to refs/heads/main by this push:
new 63e6efe593 [SYSTEMDS-3805] Fix rewrite issues and python test versions
63e6efe593 is described below
commit 63e6efe593d198e9ae7113ef0f5bdf219deba7cd
Author: Matthias Boehm <[email protected]>
AuthorDate: Thu Dec 12 12:30:45 2024 +0100
[SYSTEMDS-3805] Fix rewrite issues and python test versions
- fix remaining edge cases of scalar right indexing in codegen and
various setting with inconsistent paths
- Python document from 3.7 to 3.8 because not avaiable on ubuntu 24
---
.github/workflows/documentation.yml | 4 ++--
.../sysds/hops/codegen/template/TemplateCell.java | 3 ++-
.../sysds/hops/codegen/template/TemplateRow.java | 1 +
.../RewriteAlgebraicSimplificationDynamic.java | 16 +++++++--------
.../RewriteAlgebraicSimplificationStatic.java | 3 ++-
.../codegenalg/partone/AlgorithmLinregCG.java | 2 +-
.../functions/unary/matrix/EigenFactorizeTest.java | 18 +++++-----------
src/test/scripts/functions/unary/matrix/eigen.dml | 24 ++++------------------
8 files changed, 25 insertions(+), 46 deletions(-)
diff --git a/.github/workflows/documentation.yml
b/.github/workflows/documentation.yml
index 3cdc86e505..38871d375e 100644
--- a/.github/workflows/documentation.yml
+++ b/.github/workflows/documentation.yml
@@ -55,7 +55,7 @@ jobs:
distribution: ${{ matrix.javadist }}
java-version: ${{ matrix.java }}
cache: 'maven'
-
+
- name: Make Documentation SystemDS Java
run: mvn -ntp -P distribution package
@@ -69,7 +69,7 @@ jobs:
- name: Setup Python
uses: actions/setup-python@v5
with:
- python-version: 3.7
+ python-version: 3.8
architecture: 'x64'
- name: Cache Pip Dependencies
diff --git
a/src/main/java/org/apache/sysds/hops/codegen/template/TemplateCell.java
b/src/main/java/org/apache/sysds/hops/codegen/template/TemplateCell.java
index e902c8ec07..8d0dc273eb 100644
--- a/src/main/java/org/apache/sysds/hops/codegen/template/TemplateCell.java
+++ b/src/main/java/org/apache/sysds/hops/codegen/template/TemplateCell.java
@@ -85,7 +85,8 @@ public class TemplateCell extends TemplateBase
return hop.dimsKnown() && isValidOperation(hop)
&& !(hop.getDim1()==1 && hop.getDim2()==1)
|| (hop instanceof IndexingOp &&
hop.getInput().get(0).getDim2() >= 0
- && (((IndexingOp)hop).isColLowerEqualsUpper()
|| hop.getDim2()==1))
+ && (((IndexingOp)hop).isColLowerEqualsUpper()
|| hop.getDim2()==1)
+ && !((IndexingOp)hop).isScalarOutput())
|| (HopRewriteUtils.isDataGenOpWithLiteralInputs(hop,
OpOpDG.SEQ)
&&
HopRewriteUtils.hasOnlyUnaryBinaryParents(hop, true))
|| (HopRewriteUtils.isNary(hop, OpOpN.MIN, OpOpN.MAX,
OpOpN.PLUS) && hop.isMatrix())
diff --git
a/src/main/java/org/apache/sysds/hops/codegen/template/TemplateRow.java
b/src/main/java/org/apache/sysds/hops/codegen/template/TemplateRow.java
index d811c3c14b..4ac38b1487 100644
--- a/src/main/java/org/apache/sysds/hops/codegen/template/TemplateRow.java
+++ b/src/main/java/org/apache/sysds/hops/codegen/template/TemplateRow.java
@@ -111,6 +111,7 @@ public class TemplateRow extends TemplateBase
&& HopRewriteUtils.isAggUnaryOp(hop,
SUPPORTED_ROW_AGG))
|| (hop instanceof IndexingOp &&
hop.getInput().get(0).getDim1() > 1
&& hop.getInput().get(0).getDim2() >= 0
+ && !((IndexingOp)hop).isScalarOutput()
&&
HopRewriteUtils.isColumnRangeIndexing((IndexingOp)hop))
|| (HopRewriteUtils.isDnn(hop, OpOpDnn.BIASADD,
OpOpDnn.BIASMULT)
&& hop.getInput().get(0).dimsKnown() &&
hop.getInput().get(1).dimsKnown()
diff --git
a/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
b/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
index 9c1f2174d0..787b87716b 100644
---
a/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
+++
b/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java
@@ -698,7 +698,7 @@ public class RewriteAlgebraicSimplificationDynamic extends
HopRewriteRule
HopRewriteUtils.cleanupUnreferenced(hi);
hi = input;
- LOG.debug("Applied
simplifyRowwiseAggregate1");
+ LOG.debug("Applied
simplifyRowwiseAggregate1 (line "+hi.getBeginLine()+")");
}
}
else if( input.getDim1() == 1 )
@@ -1371,7 +1371,7 @@ public class RewriteAlgebraicSimplificationDynamic
extends HopRewriteRule
if( HopRewriteUtils.isEqualSize(left, right) //dims(A)
== dims(B)
&& left.getDataType() == DataType.MATRIX
- && right.getDataType() == DataType.MATRIX )
+ && right.getDataType() == DataType.MATRIX )
{
OpOp2 applyOp = ( bop.getOp() == OpOp2.PLUS
//pattern a: sum(A+B)->sum(A)+sum(B)
|| bop.getOp() == OpOp2.MINUS )
//pattern b: sum(A-B)->sum(A)-sum(B)
@@ -1380,7 +1380,7 @@ public class RewriteAlgebraicSimplificationDynamic
extends HopRewriteRule
if( applyOp != null ) {
//create new subdag sum(A) bop sum(B)
AggUnaryOp sum1 =
HopRewriteUtils.createSum(left);
- AggUnaryOp sum2 =
HopRewriteUtils.createSum(right);
+ AggUnaryOp sum2 =
HopRewriteUtils.createSum(right);
BinaryOp newBin =
HopRewriteUtils.createBinary(sum1, sum2, applyOp);
//rewire new subdag
@@ -1389,8 +1389,8 @@ public class RewriteAlgebraicSimplificationDynamic
extends HopRewriteRule
hi = newBin;
- LOG.debug("Applied
pushdownSumOnAdditiveBinary.");
- }
+ LOG.debug("Applied
pushdownSumOnAdditiveBinary (line "+hi.getBeginLine()+").");
+ }
}
}
@@ -2292,7 +2292,7 @@ public class RewriteAlgebraicSimplificationDynamic
extends HopRewriteRule
//sum(v^2)/sum(v1*v2) --> as.scalar(t(v)%*%v) in order to
exploit tsmm vector dotproduct
//w/o materialization of intermediates
if( hi instanceof AggUnaryOp &&
((AggUnaryOp)hi).getOp()==AggOp.SUM //sum
- && ((AggUnaryOp)hi).getDirection()==Direction.RowCol
//full aggregate
+ && ((AggUnaryOp)hi).getDirection()==Direction.RowCol
//full aggregate
&& hi.getInput().get(0).getDim2() == 1 ) //vector (for
correctness)
{
Hop baLeft = null;
@@ -2337,12 +2337,12 @@ public class RewriteAlgebraicSimplificationDynamic
extends HopRewriteRule
UnaryOp cast =
HopRewriteUtils.createUnary(mmult, OpOp1.CAST_AS_SCALAR);
//rehang new subdag under parent node
- HopRewriteUtils.replaceChildReference(parent,
hi, cast, pos);
+ HopRewriteUtils.replaceChildReference(parent,
hi, cast, pos);
HopRewriteUtils.cleanupUnreferenced(hi, hi2);
hi = cast;
- LOG.debug("Applied simplifyDotProductSum.");
+ LOG.debug("Applied simplifyDotProductSum (line
"+hi.getBeginLine()+").");
}
}
diff --git
a/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
b/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
index d06f89d72e..d3c8757c13 100644
---
a/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
+++
b/src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationStatic.java
@@ -1184,7 +1184,7 @@ public class RewriteAlgebraicSimplificationStatic extends
HopRewriteRule
HopRewriteUtils.replaceChildReference(parent, hi, bop,
pos);
- LOG.debug("Applied pushdownSumBinaryMult.");
+ LOG.debug("Applied pushdownSumBinaryMult (line
"+hi.getBeginLine()+").");
return bop;
}
return hi;
@@ -1514,6 +1514,7 @@ public class RewriteAlgebraicSimplificationStatic extends
HopRewriteRule
//as.scalar(X[i,1]) -> X[i,1] w/ scalar output
if( HopRewriteUtils.isUnary(hi, OpOp1.CAST_AS_SCALAR)
&& hi.getInput(0).getParent().size() == 1 // only
consumer
+ && hi.getParent().size() == 1 //avoid temp inconsistency
&& hi.getInput(0) instanceof IndexingOp
&& ((IndexingOp)hi.getInput(0)).isScalarOutput()
&& hi.getInput(0).isMatrix() //no frame support yet
diff --git
a/src/test/java/org/apache/sysds/test/functions/codegenalg/partone/AlgorithmLinregCG.java
b/src/test/java/org/apache/sysds/test/functions/codegenalg/partone/AlgorithmLinregCG.java
index 3061b53cde..bef7dc8a15 100644
---
a/src/test/java/org/apache/sysds/test/functions/codegenalg/partone/AlgorithmLinregCG.java
+++
b/src/test/java/org/apache/sysds/test/functions/codegenalg/partone/AlgorithmLinregCG.java
@@ -318,7 +318,7 @@ public class AlgorithmLinregCG extends AutomatedTestBase
loadTestConfiguration(config);
fullDMLScriptName = getScript();
- programArgs = new String[]{ "-stats", "-nvargs",
"X="+input("X"), "Y="+input("y"),
+ programArgs = new String[]{ "-explain","-stats",
"-nvargs", "X="+input("X"), "Y="+input("y"),
"icpt="+String.valueOf(intercept),
"tol="+String.valueOf(epsilon),
"maxi="+String.valueOf(maxiter), "reg=0.001",
"B="+output("w")};
diff --git
a/src/test/java/org/apache/sysds/test/functions/unary/matrix/EigenFactorizeTest.java
b/src/test/java/org/apache/sysds/test/functions/unary/matrix/EigenFactorizeTest.java
index 750f7d6dc2..0916286632 100644
---
a/src/test/java/org/apache/sysds/test/functions/unary/matrix/EigenFactorizeTest.java
+++
b/src/test/java/org/apache/sysds/test/functions/unary/matrix/EigenFactorizeTest.java
@@ -20,7 +20,6 @@
package org.apache.sysds.test.functions.unary.matrix;
import org.junit.Test;
-import org.apache.sysds.api.DMLScript;
import org.apache.sysds.common.Types.ExecMode;
import org.apache.sysds.runtime.meta.MatrixCharacteristics;
import org.apache.sysds.test.AutomatedTestBase;
@@ -74,13 +73,8 @@ public class EigenFactorizeTest extends AutomatedTestBase
}
private void runTestEigenFactorize( int rows, ExecMode rt)
- {
- ExecMode rtold = rtplatform;
- rtplatform = rt;
-
- boolean sparkConfigOld = DMLScript.USE_LOCAL_SPARK_CONFIG;
- if( rtplatform == ExecMode.SPARK )
- DMLScript.USE_LOCAL_SPARK_CONFIG = true;
+ {
+ ExecMode rtold = setExecMode(rt);
try
{
@@ -100,16 +94,14 @@ public class EigenFactorizeTest extends AutomatedTestBase
for(int i=0; i < numEigenValuesToEvaluate; i++) {
D[i][0] = 0.0;
}
- writeExpectedMatrix("D", D);
+ writeExpectedMatrix("D", D);
boolean exceptionExpected = false;
runTest(true, exceptionExpected, null, -1);
compareResults(1e-8);
}
finally {
- DMLScript.USE_LOCAL_SPARK_CONFIG = sparkConfigOld;
- rtplatform = rtold;
+ resetExecMode(rtold);
}
}
-
-}
\ No newline at end of file
+}
diff --git a/src/test/scripts/functions/unary/matrix/eigen.dml
b/src/test/scripts/functions/unary/matrix/eigen.dml
index 12a2f8788d..f863a0a8e6 100644
--- a/src/test/scripts/functions/unary/matrix/eigen.dml
+++ b/src/test/scripts/functions/unary/matrix/eigen.dml
@@ -7,9 +7,9 @@
# 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
@@ -29,32 +29,16 @@ A = t(A) %*% A; # make the input matrix symmetric
[eval, evec] = eigen(A);
-/*
-B = evec %*% diag(eval) %*% t(evec);
-diff = sum(A - B);
-D = matrix(1,1,1);
-D = diff*D;
-*/
-
numEval = $2;
D = matrix(1, numEval, 1);
for ( i in 1:numEval ) {
Av = A %*% evec[,i];
+ while(FALSE){} #fix incorrect rewrite sequence
rhs = as.scalar(eval[i,1]) * evec[,i];
+ while(FALSE){} #fix incorrect rewrite sequence
diff = sum(Av-rhs);
D[i,1] = diff;
}
-/*
-# TODO: dummy if() must be removed
-v = evec[,1];
-Av = A %*% v;
-rhs = as.scalar(eval[1,1]) * evec[,1];
-diff = sum(Av-rhs);
-
-D = matrix(1,1,1);
-D = diff*D;
-*/
-
write(D, $3);
