Enable Gemm (distributed) to be used with MatrixRef #1022
Conversation
albestro
force-pushed
the
alby/gemm-sub
branch
from
78f492b
to
ba30247
Compare
albestro
force-pushed
the
alby/gemm-sub
branch
from
fec1876
to
8235e1e
Compare
albestro
force-pushed
the
alby/matrix-ref-gemm-dist
branch
from
83ddf30
to
d5f4f3f
Compare
|
Currently, "empty" test-cases are not working: this is probably due to |
albestro
force-pushed
the
alby/matrix-ref-gemm-dist
branch
from
a71b005
to
74da6f3
Compare
| DLAF_ASSERT(matrix::equal_process_grid(mat_a, grid), mat_a, grid); | ||
| DLAF_ASSERT(matrix::equal_process_grid(mat_b, grid), mat_a, grid); | ||
| DLAF_ASSERT(matrix::equal_process_grid(mat_c, grid), mat_a, grid); |
There was a problem hiding this comment.
Why are these not simply:
| DLAF_ASSERT(matrix::equal_process_grid(mat_a, grid), mat_a, grid); | |
| DLAF_ASSERT(matrix::equal_process_grid(mat_b, grid), mat_a, grid); | |
| DLAF_ASSERT(matrix::equal_process_grid(mat_c, grid), mat_a, grid); | |
| DLAF_ASSERT(matrix::same_process_grid(mat_a, mat_b), mat_a, mat_b); | |
| DLAF_ASSERT(matrix::same_process_grid(mat_b, mat_c), mat_b, mat_c); |
? I guess the grid kind of additionally acts as a tag here to say that this is a distributed matrix multiplication? But then the pipelines already kind of do that as well...
There was a problem hiding this comment.
About the slightly more relaxed requirement, I was wrong.
About the check, as soon as we will get #993 we can add the check about the "matching" between matrices grid and pipelines.
For what concerns this PR, I think this change is enough 😉
There was a problem hiding this comment.
Should I update something in #993 now that this is merged or would you do the change you're talking about separately?
There was a problem hiding this comment.
Is it the only place where it applies? we can check/discuss at/after the meeting today how to proceed 😉
There was a problem hiding this comment.
Probably not... however, for this particular case, see 4726236.
since we can get the same information directly from matrix
albestro
force-pushed
the
alby/matrix-ref-gemm-dist
branch
from
4f9514d
to
3fdeea2
Compare
| /// @param mat_c On entry it contains the input matrix C. On exit matrix tiles in the range will be | ||
| /// overwritten with the result, while others are left untouched. | ||
| /// | ||
| /// @pre @p mat_a, @p mat_b and @p mat_c are distributed the same way, |
There was a problem hiding this comment.
What do you mean?
What I read -> same distribution
what should be -> same grid
|
cscs-ci run |
Implementation has been copied from the GeneralSub case, but it has been refined and cleaned up (thanks to MatrixRef functionalities). More importantly, the GeneralSub implementation was constrained to just work with square sub-matrices, while now this constraint has been removed.
TODO