Skip to content
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

Updating cuML to use consolidated RAFT libs #5272

Merged

Conversation

cjnolet
Copy link
Member

@cjnolet cjnolet commented Mar 14, 2023

This is intended to be merged in 23.06 w/ this PR: rapidsai/raft#1333.

@cjnolet cjnolet added 5 - DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 14, 2023
@cjnolet cjnolet requested review from a team as code owners March 14, 2023 21:55
@cjnolet cjnolet self-assigned this Mar 14, 2023
rapids-bot bot pushed a commit to rapidsai/raft that referenced this pull request Mar 20, 2023
…#1333)

Now that we no longer need to rely on the FAISS dependency, this PR:
1. Consolidates  the `libraft-distance` and `libraft-nn` targets into a single `libraft` artifact
2. Introduces a new `raft::compiled` target to link against the new `libraft.so` binary. Removes `raft::distance`and `raft::nn` targets.
3. Consolidates `RAFT_DISTANCE_COMPILED` and `RAFT_NN_COMPILED` pre-processor vars into a single `RAFT_COMPILED` (to match similar pattern implementated by `spdlog`)
4. Consolidates `specializations.cuh` headers
5. Updates all docs, scripts, and build infra

This change has been a long time coming and is intended to be a 23.04 feature. This is further going to require updates to several projects downstream. Here's a checklist to track that progress:
- [x] offline announcement
- [x] cuml rapidsai/cuml#5272
- [x] cugraph rapidsai/cugraph#3348
- [x] cuopt rapidsai/cuopt#1023
- [x] cugraph-ops rapidsai/cugraph-ops#429


This PR depended on #1340 (removing FAISS from the build) and on #1202 (replacing the FAISS bfknn w/ our own), both of which have been merged. 

Closes #824

Authors:
  - Corey J. Nolet (https://github.com/cjnolet)
  - Ben Frederickson (https://github.com/benfred)

Approvers:
  - Sevag H (https://github.com/sevagh)
  - Divye Gala (https://github.com/divyegala)
  - Ben Frederickson (https://github.com/benfred)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1333
@cjnolet cjnolet removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Mar 20, 2023
cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@github-actions github-actions bot added the ci label Mar 21, 2023
@cjnolet
Copy link
Member Author

cjnolet commented Mar 21, 2023

/merge

ci/build_cpp.sh Outdated Show resolved Hide resolved
ci/build_python.sh Outdated Show resolved Hide resolved
ci/test_python_common.sh Outdated Show resolved Hide resolved
@vyasr vyasr requested a review from a team as a code owner March 22, 2023 13:19
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Mar 22, 2023
Co-authored-by: Charles Blackmon-Luca <[email protected]>
@github-actions github-actions bot removed the ci label Mar 22, 2023
@cjnolet
Copy link
Member Author

cjnolet commented Mar 22, 2023

Update for folks who are waiting for this PR to be merged. This is currently blocked on rapidsai/raft#1365, which resolvesa couple Dask issues that appeared recently. Once that PR is merged, we should see a successful CI run of this PR and it'll be ready to merge.

@github-actions github-actions bot added the ci label Mar 23, 2023
@cjnolet
Copy link
Member Author

cjnolet commented Mar 23, 2023

/merge

@rapids-bot rapids-bot bot merged commit 1ee3a2f into rapidsai:branch-23.04 Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci CMake conda conda issue CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

5 participants