[SYSTEMDS-3123] Rewrite cbind 0 Matrix Multiplication

[Baunsgaard]

This PR contains a rewrite that change the sequences if number of rows in X is 2x larger than Y:

 cbind((X %*% Y), matrix(0, nrow(X), 1)) -> X %*% (cbind(Y, matrix(0, nrow(Y), 1)))

This rewrite effects MLogReg in line 215 to not force allocation of the large X twice.

Review appreciated!

[Shafaq-Siddiqi]

LGTM!

[mboehm7]

Thanks for staring the discussion, a few additional points to consider (besides separating it from the compression-related changed):

• we need to check for know dims, otherwise the rewrite also triggers if hi.getDim2() is unknown (-1), and as we don't consider the other way around, it would never come back during recompilation
• The overall benefit comes from reduced allocation in the cbind output (while incurring more floating point operations in the matrix multiply in some kernels that avoid branches in the inner loop). Accordingly, we cannot just look at hi.getDim1() > hi.getDim2() * 2. Instead given a m-x-n matrix X and n-x-k matrix Y, we have to compare the following: (a): mk + m(k+1) versus (b) nk + m(k+1), which reduces to comparing m and n, not m and (k+1), isn't it?

[Baunsgaard]

Thanks for staring the discussion, a few additional points to consider (besides separating it from the compression-related changed):

I merged in the compression changes when i was happy with the rewrite.
I will separate the different elements in different commits once it goes to master.

* we need to check for know dims, otherwise the rewrite also triggers if hi.getDim2() is unknown (-1), and as we don't consider the other way around, it would never come back during recompilation

Yes good point, i will include it in the a check that the dims are known

* The overall benefit comes from reduced allocation in the cbind output (while incurring more floating point operations in the matrix multiply in some kernels that avoid branches in the inner loop). Accordingly, we cannot just look at `hi.getDim1() > hi.getDim2() * 2`. Instead given a m-x-n matrix X and n-x-k matrix Y, we have to compare the following: (a): m_k + m_(k+1) versus (b) n_k + m_(k+1), which reduces to comparing m and n, not m and (k+1), isn't it?

The comparison i make currently is based on the extra allocation therefore it has to compare m (the left hand side number of rows) with n (the common dimension/ number of rows in right).
this is because these two correspond to the memory allocation in the cbind operation. To make the rewrite better i could include a cost of the matrix multiplication for cbind before vs after. this would make the rewrite more stable.
Alternatively i think what i am going to do is to set the hi.getDim1() > hi.getDim2() * 2 to something like hi.getDim1() > hi.getDim2() * 100, since this still tricker the rewrite in the place intended, and is less prone to executing the rewrite in bad locations. (not that it matters much it is a very specific rewrite.)

[mboehm7]

no, you misunderstood my main comment - we need to compare m vs n (and apparently that's what you wanted to do), but what the code does is to compare m vs (k+1) because you look at rows and columns of the output cbind (variable hi) - simply replace hi.getDim2() with hi.getInput(1).getDim1()