-
Notifications
You must be signed in to change notification settings - Fork 197
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
Allow enabling NVTX markers by downstream projects after install #610
Allow enabling NVTX markers by downstream projects after install #610
Conversation
This PR should replace rapidsai/cuml#4684 . Tagging @robertmaynard @vinaydes @cjnolet to continue the discussion, please let me know what you think of it. Note specifically, if |
Looks good to me. I'll close my PR if we decide to merge this one. |
Just curious, why not just enable nvtx markers all the time? |
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.
This LGTM but would like @robertmaynard’s thoughts before merging
Update, as per prior suggestions:
|
@achirkin the PR looks good. I think there's still a compilation error showing up in the CI logs:
|
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.
Approving ops-codeowner
file changes
Apparently, the NVTX implementation was bugged for cuda <= 11.2 because of the usage of |
rerun tests |
2 similar comments
rerun tests |
rerun tests |
@gpucibot merge |
Adjust the name of the option according to the changes proposed in rapidsai/raft#610 Authors: - Artem M. Chirkin (https://github.com/achirkin) Approvers: - Corey J. Nolet (https://github.com/cjnolet) URL: #4718
Adjust the name of the option according to the changes proposed in rapidsai/raft#610 Authors: - Artem M. Chirkin (https://github.com/achirkin) Approvers: - Corey J. Nolet (https://github.com/cjnolet) URL: rapidsai#4718
Allow downstream projects enable
NVTX
option for theraft::raft
target, if it hasn't been enabled at the install time.