j143 commented on code in PR #1986:
URL: https://github.com/apache/systemds/pull/1986#discussion_r1460477292
##########
src/main/java/org/apache/sysds/hops/rewrite/RewriteAlgebraicSimplificationDynamic.java:
##########
@@ -1137,46 +1138,88 @@ else if( right.getDim2()>1 ) //multi column vector
return hi;
}
-
- private static Hop simplifyDiagMatrixMult(Hop parent, Hop hi, int pos)
- {
- if( hi instanceof ReorgOp &&
((ReorgOp)hi).getOp()==ReOrgOp.DIAG && hi.getDim2()==1 ) //diagM2V
+
+ private static Hop simplifyDiagMatrixMult(Hop parent, Hop hi, int pos) {
+ if(hi instanceof ReorgOp && ((ReorgOp) hi).getOp() ==
ReOrgOp.DIAG && hi.getDim2() == 1) //diagM2V
{
Hop hi2 = hi.getInput().get(0);
- if( HopRewriteUtils.isMatrixMultiply(hi2) ) //X%*%Y
+ if(HopRewriteUtils.isMatrixMultiply(hi2)) //X%*%Y
{
Hop left = hi2.getInput().get(0);
Hop right = hi2.getInput().get(1);
-
+
//create new operators (incl refresh size
inside for transpose)
ReorgOp trans =
HopRewriteUtils.createTranspose(right);
BinaryOp mult =
HopRewriteUtils.createBinary(left, trans, OpOp2.MULT);
AggUnaryOp rowSum =
HopRewriteUtils.createAggUnaryOp(mult, AggOp.SUM, Direction.Row);
-
+
//rehang new subdag under parent node
HopRewriteUtils.replaceChildReference(parent,
hi, rowSum, pos);
HopRewriteUtils.cleanupUnreferenced(hi, hi2);
-
+
hi = rowSum;
LOG.debug("Applied simplifyDiagMatrixMult");
- }
+ }
}
-
+
return hi;
}
-
- private static Hop simplifySumDiagToTrace(Hop hi)
- {
- if( hi instanceof AggUnaryOp )
- {
+
+ private static Hop simplifyMatrixFactorization(Hop parent, Hop hi, int
pos) {
+ // Check for Hop being Binary
+ if(hi instanceof BinaryOp) {
+ Hop left = hi.getInput().get(0);
Review Comment:
> I just realized that the name of the method simplifyMatrixFactorization()
might be misleading because somebody might think we perform a matrix
factorization (i.e.,
[decomposition](https://en.wikipedia.org/wiki/Matrix_decomposition)) instead of
factoring out a term (i.e., here matrix A). Should I change the name to
something more neutral like simplifyMatrixDistributiveFactorization() ?
@ReneEnjilian -- if you prefer you can call this, may be
`MatrixCombineMultiply`. It's up to you. 😺
Factorization has a different meaning!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@systemds.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
