-
Notifications
You must be signed in to change notification settings - Fork 108
Remove cuda 11 specific changes from repo #222
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
Remove cuda 11 specific changes from repo #222
Conversation
Co-authored-by: Kyle Edwards <kyedwards@nvidia.com>
…8prabhu/cuopt_public into remove_cuda_11_missing_references
jakirkham
left a comment
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.
Thanks Ram! 🙏
Had a couple comments below
| secrets: inherit | ||
| uses: rapidsai/shared-workflows/.github/workflows/conda-cpp-build.yaml@branch-25.08 | ||
| with: | ||
| matrix_filter: map(select((.CUDA_VER | startswith("12")))) |
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.
Recognize we are only testing CUDA 12, but could we keep this?
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.
Since we might move to cuda 13 in 25.10 or later, I removed it just for future case.
|
Could we please also take a look at this code? cuopt/cpp/src/linear_programming/utilities/ping_pong_graph.cuh Lines 48 to 63 in 9952c26
Edit: Also this CMake code Lines 222 to 224 in 47fb158
|
Yes, I am currently discussing with team on how much effort would it have and asked them to push changes to this PR. Meanwhile, I will keep don't merge tag to this. |
|
Thanks Ram! 🙏 Can you please look at this CMake code as well? Lines 222 to 224 in 47fb158
|
Removed if conditions around it. |
|
@jakirkham May I get another set of review and also had few questions on previous reviews |
akifcorduk
left a comment
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.
Looks good on cpp side.
bdice
left a comment
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.
One small suggestion to clarify a comment, otherwise LGTM. We should land this soon for 25.08.
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
|
/merge |
|
Thanks Ram and thanks to all the reviewers! 🙏 Agree this looks good |
Description
This PR removes CUDA 11 pertaining changes since we have moved to CUDA 12 support only
Issue
closes #220
Checklist