Use cusparsespmv_preprocess() now that Raft implements it. - #120
Conversation
ramakrishnap-nv
added
non-breaking
|
/ok to test c77e264 |
@rgsl888prabhu, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
|
/ok to test 457269b |
|
/ok to test 5ce59ef |
|
Please run : https://github.com/NVIDIA/cuopt/blob/branch-25.08/ci/check_style.sh (or just the pre-commit part) to pass the check-style |
Thanks for the tip, done! |
|
Hi @Kh4ster! Can you have a look at this PR? Anything else needed from my side? |
|
@vitor1001 |
Can you elaborate? I didn't find any comment about those. The only comment I saw was about running check_style. |
|
@vitor1001 Sorry maybe it's on my side then. I have shared a screenshot with the review comments I left, don't you see those in you go up the thread or in the file section? |
@Kh4ster I confirm those are not visible. Maybe there is a "finish review" or "publish comments" button? About the comment: Raft doesn't do dlopen() explicitly (link). That said, I guess if one have a similar runtime/header mismatch Raft will be broken and maybe a better fix is having Raft CMake checking for the mismatch and asking the user to fix the environment? Such work-arounds sound dangerous, is it still necessary? |
|
@rgsl888prabhu regarding the comments I put on the PR as review, do you know why external contributors would not see those? |
|
@vitor1001 sorry for the delay in the response I was on PTO. I don't know why you can't see my review comment but this is an issue, we will try to understand why. Regarding dl_open: Regarding my last comment @vitor1001 let me put it here as screenshot: |
@Kh4ster this is a fair point. That said, I think the Raft API do make sense, since it forces the buffer to have the right alignment. Should I do something like in this line: ? |
I agree, those comments are pending and need to be submitted, that's why you can see them but not us. |
|
@vitor1001 your point is valid. I thought RMM would always return aligned data but I'm not 100% sure, let me check with them. |
Yes, we build on cuda 12.9, but we test on cuda > 12.4 and few other options since we can't test complete matrix. |
|
|
/ok to test 38b202f |
|
/ok to test |
@Kh4ster, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
|
/ok to test 2871288 |
|
/merge |
| } | ||
| #endif | ||
|
|
||
| // This cstr is used in pdhg |
There was a problem hiding this comment.
If we use RAFT cusparsespmv_preprocess will the above logic be maintained?
| rmm::device_uvector<uint8_t> buffer_non_transpose; | ||
| rmm::device_uvector<uint8_t> buffer_transpose; | ||
| rmm::device_uvector<f_t> buffer_non_transpose; | ||
| rmm::device_uvector<f_t> buffer_transpose; |
There was a problem hiding this comment.
Since we use resize on those, I think it will overallocate if we use f_t (double in most cases) instead of uint8_t or am I mistaking?
This removes a temporary hack. Authors: - Vitor Sessak (https://github.com/vitor1001) - Ramakrishnap (https://github.com/rgsl888prabhu) - Nicolas Blin (https://github.com/Kh4ster) Approvers: - Nicolas Blin (https://github.com/Kh4ster) URL: #120
This removes a temporary hack. Authors: - Vitor Sessak (https://github.com/vitor1001) - Ramakrishnap (https://github.com/rgsl888prabhu) - Nicolas Blin (https://github.com/Kh4ster) Approvers: - Nicolas Blin (https://github.com/Kh4ster) URL: NVIDIA#120
4 participants
Description
This removes a temporary hack.
Checklist