-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARSE] Update oneMKL backends to match new sparse API #500
Conversation
Test logs: Tests are marked as skipped when some configurations are skipped but all the tests are run regardless. |
examples/sparse_blas/compile_time_dispatching/sparse_blas_spmv_usm_mklcpu.cpp
Show resolved
Hide resolved
examples/sparse_blas/compile_time_dispatching/sparse_blas_spmv_usm_mklcpu.cpp
Show resolved
Hide resolved
examples/sparse_blas/run_time_dispatching/sparse_blas_spmv_usm.cpp
Outdated
Show resolved
Hide resolved
tests/unit_tests/sparse_blas/include/common_sparse_reference.hpp
Outdated
Show resolved
Hide resolved
ec72831
to
2c2219e
Compare
2c2219e
to
7190c6a
Compare
Had to force push on my branch to fix my author name and email, there was no code changes since my last comment. |
I've pushed 2f59edc to reduce the number of times |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finally reviewed the entire PR. This is fantastic work and top notch code. Thanks a lot @Rbiessy for being patient on the review and feedback. I have some review comments, I'm ready to approve once those are resolved.
examples/sparse_blas/run_time_dispatching/sparse_blas_spmv_usm.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to approve after the last set of minor changes on moving around where some descriptor arguments are set and checked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved pending the last 2 outstanding comments about fixing exception text. Thank you for this fantastic work and for being patient while we reviewed the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reaffirming LGTM given my last outstanding comments were resolved.
Description
Update sparse API to follow the specification change: uxlfoundation/oneAPI-spec#522
Also see related small changes needed in the specification: uxlfoundation/oneAPI-spec#542
This also add some backend specific documentation in the
docs
folder. You can see how it is rendered here: https://rbiessy.github.io/oneMKL/domains/sparse_linear_algebra.htmlAll Submissions