-
Notifications
You must be signed in to change notification settings - Fork 198
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
Make ucx linkage explicit and add a new CMake target for it #1032
Conversation
rerun tests |
Thanks for fixing this, @vyasr. Now we just need to add ucx as an explicit link dependency of cuml and cugraph |
Done. |
@robertmaynard this adds a new optional component for |
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.
LGTM, thanks @vyasr!
@gpucibot merge |
1 similar comment
@gpucibot merge |
This is a follow-up to #1032 to include NCCL in raft::distributed so that all communications libraries are now cleanly encapsulated in a single interface dependency, allowing us to remove it from the raft-dask Python build and simplifying the process for other consumers. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Robert Maynard (https://github.com/robertmaynard) - Corey J. Nolet (https://github.com/cjnolet) URL: #1077
This PR removes the dlopen logic for libucp in ucp_helper.hpp in favor of calling the relevant APIs directly. It also adds a new CMake component
raft::distributed
that can be used by dependent libraries to indicate the dependency on parts of raft that require UCX.While it does not change any public APIs, I have marked this PR as breaking since it does mean that any C++ code linking to UCX must now ensure that UCX is available at link time. It is no longer sufficient to make the library available at runtime.
Resolves #1031.