eth-cscs / DLA-Future

DLA-Future
https://eth-cscs.github.io/DLA-Future/master/
BSD 3-Clause "New" or "Revised" License
64 stars 14 forks source link

Enable Gemm (local) to be used with MatrixRef #969

Closed albestro closed 11 months ago

albestro commented 1 year ago

This PR partially address #915 (just local variant)

TODO

Note: GeneralSub was intended to work on submatrices, while now the sub-matrix concept is hidden by MatrixRef. For this reason, the new gemm helper might just be considered a full general multiplication, and it might be moved in multiplication::general::General (or something like that).

msimberg commented 1 year ago

This looks good to me for the current use cases. I see now what the main difficulty would be with relaxing constraints on the input matrices: ETI. I don't know what the best way around this is. Inheritance works, but if possible I'd rather not go there. We might have to try out and see how bad the compile times would be from exposing the implementations again to allow arbitrary instantiations.

albestro commented 11 months ago

cscs-ci run

albestro commented 11 months ago

cscs-ci run

albestro commented 11 months ago

cscs-ci run

albestro commented 11 months ago

Quickly tested ROCm image, which was super slow. I had a look about bindings and they seemed ok.

Anyway, I did not investigate any further about slowness, and I went on by runnning just test_multiplication_general, which passed with all green. So, the "workaround" for scal using gemm with nullptrs works correctly there too.

That's enough for what concerns this PR. 🟢

albestro commented 11 months ago

cscs-ci run