Conversation
📝 WalkthroughWalkthroughRAFT retrieval in CMake switched to fork Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cpp/cmake/thirdparty/get_raft.cmake`:
- Around line 41-44: The diff introduces a personal fork and a moving branch:
update the find_and_configure_raft invocation to remove the personal fork
reference by replacing FORK aamijar with FORK rapidsai (or omit FORK to default
to rapidsai) and replace the branch PINNED_TAG raft-deprecated-headers with a
deterministic pin (either ${rapids-cmake-checkout-tag} or an explicit commit
SHA) so find_and_configure_raft uses the official rapidsai repo and a fixed
commit/ tag for reproducible builds.
hlinsen
left a comment
There was a problem hiding this comment.
Lgtm. Let's wait for: rapidsai/raft#2939 before merging
We should merge this one before rapidsai/raft#2939 to avoid breaking cuopt. |
|
/merge |
Depends on rapidsai/cuvs#1763. Resolves #2937 This PR does the following: 1. Removes files with the following message ```cpp #ifndef RAFT_HIDE_DEPRECATION_WARNINGS #pragma message(__FILE__ \ " is deprecated and will be removed in a future release." \ " Please use the raft/sparse/solver version instead.") #endif ``` 2. Removes files with the following message ```cpp /** * This file is deprecated and will be removed in a future release. */ ``` 3. Pulls from correct non-deprecated headers like using`raft/util/cudart_utils.cuh` instead of `raft/core/cudart_utils.cuh` 4. Pulls correct functionality from non-deprecated headers like using `raft/linalg/matrix_vector.cuh` instead of `raft/matrix/math.cuh`. This requires using the newer mdspan based apis. 5. Consolidates `gemm.cuh` and `gemm.hpp` into just `gemm.cuh` **Update:** There were a couple of fixes included in this PR that I have decoupled. Instead those fixes should be merged in #2940. This will allow us to unravel the merging sequence better to avoid breaking changes altogether. Still marking this PR as breaking for awareness purposes. After merging #2940, the cuml side and cuvs side can be updated to use the non-deprecated apis. Then we can merge this raft PR to remove the deprecated and unused headers. **Update:** There are a couple of breaking changes that affect cugraph and cuopt, so we will need to merge fixes for those first. **Merging sequence:** #2940 -> rapidsai/cuvs#1763 -> rapidsai/cuml#7752 -> rapidsai/cuml#7797 -> rapidsai/cugraph#5429 -> NVIDIA/cuopt#865 -> #2939 **Downstream libraries CI status:** cuvs: 🟢 cuml: 🟢 cugraph: 🟢 cuopt: 🟢 Authors: - Anupam (https://github.com/aamijar) Approvers: - Bradley Dice (https://github.com/bdice) - Divye Gala (https://github.com/divyegala) - Dante Gama Dessavre (https://github.com/dantegd) URL: #2939
Testing with rapidsai/raft#2939 and adding fixes.
Summary by CodeRabbit
Chores
Tests