Pressio / pressio

core C++ library
Other
45 stars 7 forks source link

ops: revise kokkos matrix-vector product #497

Closed mzuzek closed 1 year ago

mzuzek commented 1 year ago

refs #446

fnrizzi commented 1 year ago

I find the use of impl::execution_space and has_execution_space confusing. Seems to me that you are defining one to aid the other. I think we should just have has_execution_space and specialize it for kokkos tpetra and tpetrablock. This would make things easier.

Please make a separate PR for this execution space trait thing.

mzuzek commented 1 year ago

@fnrizzi As we have discussed and planned, I rebased this PR on #502 which removes have_matching_execution_space predicate all together. This obsoletes c40b2c473698dc87184d7eaccfcd8b36ae0a62cf (making it empty) and hopefully resolves all raised issues and questions on execution_space and has_execution_space utilities which supported that predicate.

mzuzek commented 1 year ago

for the record, we had weird CI failure that did not reproduce on the re-run:

terminate called after throwing an instance of 'std::invalid_argument'
  what():  stoi
mzuzek commented 1 year ago

@fnrizzi

Kokkos test coverage

Both nontranspose and transpose overloads of pressio::ops::product() are tested with all combinations of plain views and available Pressio expressions: A x y
view2d view1d view1d
view2d span view1d
view2d diag view1d
view2d view1d span
view2d view1d diag
view2d span span
view2d span diag
view2d diag span
view2d diag diag
subspan view1d view1d
subspan span view1d
subspan diag view1d
subspan view1d span
subspan view1d diag
subspan span span
subspan span diag
subspan diag span
subspan diag diag

Note: A could be also pressio::as_diagonal_matrix() in future - once it's implemented, see #304

fnrizzi commented 1 year ago

@mzuzek