Bugfix

Author: janniklinde

src/main/java/org/apache/sysds/hops/rewriter/MetaPropagator.java

@@ -71,6 +71,8 @@ public RewriterStatement apply(RewriterStatement root) {
 	}
 
 	private RewriterStatement propagateDims(RewriterStatement root, RewriterStatement parent, int pIdx, RewriterAssertions assertions) {
+		if (root.getResultingDataType(ctx) == null)
+			throw new IllegalArgumentException("Null type: " + root.toParsableString(ctx));
 		if (!root.getResultingDataType(ctx).startsWith("MATRIX")) {
 			if (root.isInstruction()) {
 				String ti = root.trueTypedInstruction(ctx);

src/main/java/org/apache/sysds/hops/rewriter/RewriterHeuristic.java

@@ -58,7 +58,7 @@ public RewriterStatement apply(RewriterStatement currentStmt, @Nullable BiFuncti
 			foundRewrite.setValue(true);
 
 		while (rule != null) {
-			//System.out.println("Pre-apply: " + rule.rule.getName());
+			System.out.println("Pre-apply: " + rule.rule.getName());
 			/*if (currentStmt.toParsableString(ruleSet.getContext()).equals("%*%(X,[](B,1,ncol(X),1,ncol(B)))"))
 				System.out.println("test");*/
 			/*System.out.println("Expr: " + rule.matches.get(0).getExpressionRoot().toParsableString(ruleSet.getContext()));

src/main/java/org/apache/sysds/hops/rewriter/RewriterInstruction.java

@@ -189,6 +189,7 @@ public RewriterStatement copyNode() {
 		mCopy.costFunction = costFunction;
 		mCopy.consolidated = consolidated;
 		mCopy.operands = new ArrayList<>(operands);
+		mCopy.returnType = returnType;
 		if (meta != null)
 			mCopy.meta = new HashMap<>(meta);
 		else
@@ -215,6 +216,7 @@ public RewriterStatement nestedCopyOrInject(Map<RewriterStatement, RewriterState
 		mCopy.costFunction = costFunction;
 		mCopy.consolidated = consolidated;
 		mCopy.operands = new ArrayList<>(operands.size());
+		mCopy.returnType = returnType;
 		mCopy.hashCode = hashCode;
 		if (meta != null)
 			mCopy.meta = new HashMap<>(meta);
@@ -245,6 +247,7 @@ public boolean isInstruction() {
 		return true;
 	}
 
+	@Deprecated
 	@Override
 	public RewriterStatement clone() {
 		RewriterInstruction mClone = new RewriterInstruction();
@@ -259,6 +262,7 @@ public RewriterStatement clone() {
 		mClone.operands = clonedOperands;
 		mClone.costFunction = costFunction;
 		mClone.consolidated = consolidated;
+		mClone.returnType = returnType;
 		mClone.meta = meta;
 		return mClone;
 	}

src/test/java/org/apache/sysds/test/component/codegen/rewrite/RewriterClusteringTest.java

@@ -69,6 +69,8 @@ public void testExpressionClustering() {
 		db.forEach(expr -> {
 			if (ctr.incrementAndGet() % 10 == 0)
 				System.out.println("Done: " + ctr.intValue() + " / " + size);
+			if (ctr.intValue() > 1000)
+				return; // Skip
 			// First, build all possible subtrees
 			System.out.println("Eval: " + expr.toParsableString(ctx));
 			List<RewriterStatement> subExprs = RewriterUtils.generateSubtrees(expr, ctx, 500);
@@ -163,12 +165,26 @@ private boolean checkRelevance(List<RewriterStatement> stmts) {
 				TopologicalSort.sort(stmt2, ctx);
 
 				if (!stmt1.match(RewriterStatement.MatcherContext.exactMatchWithDifferentLiteralValues(ctx, stmt2))) {
+					// TODO: Minimal difference can still prune valid rewrites (e.g. sum(A %*% B) -> sum(A * t(B)))
 					RewriterStatement.MatcherContext mCtx = RewriterStatement.MatcherContext.findMinimalDifference(ctx, stmts.get(j));
 					stmts.get(i).match(mCtx);
 					Tuple2<RewriterStatement, RewriterStatement> minimalDifference = mCtx.getFirstMismatch();
 
 					if (minimalDifference._1 == stmts.get(i))
 						match = false;
+					else {
+						// Otherwise we need to work ourselves backwards to the root if both canonical forms don't match now
+						RewriterStatement minStmt1 = minimalDifference._1.nestedCopy();
+						RewriterStatement minStmt2 = minimalDifference._2.nestedCopy();
+						minStmt1 = converter.apply(minStmt1);
+						minStmt2 = converter.apply(minStmt2);
+
+						if (minStmt1.match(RewriterStatement.MatcherContext.exactMatch(ctx, minStmt2))) {
+							// Then the minimal difference does not imply equivalence
+							// For now, just keep every result then
+							match = false;
+						}
+					}
 				}
 			}
 		}

src/test/java/org/apache/sysds/test/component/codegen/rewrite/RewriterStreamTests.java

@@ -556,5 +556,14 @@ public void myTest5() {
 		System.out.println(stmt.toParsableString(ctx, true));
 	}
 
+	@Test
+	public void myTest6() {
+		RewriterStatement stmt = RewriterUtils.parse("rowSums(<=(D,minD))", ctx, "MATRIX:D,minD");
+		stmt = canonicalConverter.apply(stmt);
+
+		System.out.println("==========");
+		System.out.println(stmt.toParsableString(ctx, true));
+	}
+
 	// TODO: There is a problem if e.g. _EClass(argList(ncol(X), +(ncol(X), 0))) as then ncol(X) will be replaced again with the _EClass
 }