Baunsgaard commented on pull request #1385:
URL: https://github.com/apache/systemds/pull/1385#issuecomment-915556882
> 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.)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
