-
Notifications
You must be signed in to change notification settings - Fork 310
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
Refactor CMakeLists.txt to better express usage requirements #4309
Refactor CMakeLists.txt to better express usage requirements #4309
Conversation
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 for doing this Robert! I can rebase the wheels work on this once it's merged and it should help clean up quite a few things. Also, removing the unnecessary NCCL linkage from the library should substantially reduce the wheel sizes in #4297, which is exciting. I have a few questions here but nothing blocking, feel free to resolve as you see fit and then merge.
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 looks good to me. I provided a little feedback. Happy to approve unless there are reactions to my comments.
/merge |
/merge |
Previously the cugraph CMakeLists.txt genernated non-relocatable targets due to embedding absolute paths to RAFT and the CUDAToolkit. While refactoring those issues out I also made a general cleanup pass over the rest of the CMake code.