[REVIEW] Apply modifications to account for RAFT changes #1707
Conversation
BradReesWork
added
3 - Ready for Review
improvement
There was a problem hiding this comment.
This addresses the changes prior to rapidsai/raft#286. Once that PR is merged, couldn't a bunch of these changes eliminate the inclusion of rmm/exec_policy.hpp and simply use handle.get_thrust_policy()?
It seems a shame to make all of these changes and later go back and remake all of these changes a little differently.
|
Thank you for your review @ChuckHastings! At first, we planned to do minimal modifications to make the code to compile with the newest RAFT changes and to make a deeper change later on. However, as you said, it's probably better to include the |
|
Tagging you @afender to raise awareness on the ongoing changes in |
|
@viclafargue @ChuckHastings This PR is getting old and further out of sync - lots of conflicts. Can we either get it updated or closed? |
|
This can't be merged until the RAFT PR is done. Perhaps @viclafargue has some insight into a time frame for when the raft work will be complete. There's a bunch of changes going into cugraph that will make these diverge even more. |
|
@BradReesWork @ChuckHastings, the concurrent merge of changes in |
cpp/cmake/thirdparty/get_raft.cmake
Outdated
| @@ -42,6 +42,6 @@ set(CUGRAPH_BRANCH_VERSION_raft "${CUGRAPH_VERSION_MAJOR}.${CUGRAPH_VERSION_MINO | |||
| # To use a different RAFT locally, set the CMake variable | |||
| # RPM_raft_SOURCE=/path/to/local/raft | |||
| find_and_configure_raft(VERSION ${CUGRAPH_MIN_VERSION_raft} | |||
| FORK rapidsai | |||
| PINNED_TAG branch-${CUGRAPH_BRANCH_VERSION_raft} | |||
| FORK viclafargue | |||
There was a problem hiding this comment.
Just noting... this will be switched back once the RAFT PR is merged.
There was a problem hiding this comment.
Yes, just for testing. Thanks for putting a reminder
|
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-21.10 #1707 +/- ##
================================================
- Coverage 59.85% 59.68% -0.17%
================================================
Files 77 77
Lines 3547 3567 +20
================================================
+ Hits 2123 2129 +6
- Misses 1424 1438 +14
Continue to review full report at Codecov.
|
PR essentially backports the fixes in #1707 for the deprecated RMM exec policy, and unpins the RMM nighlty to unblock local builds and CI and local development. Conflicts with the bigger PR should be minimal, and mainly around a single change: ``` rmm::exec_policy(handle.get_stream()) ``` to ``` handle.get_thrust_policy() ``` Authors: - Dante Gama Dessavre (https://github.com/dantegd) Approvers: - AJ Schmidt (https://github.com/ajschmidt8) - Chuck Hastings (https://github.com/ChuckHastings) - Seunghwa Kang (https://github.com/seunghwak) URL: #1789
|
rerun tests |
|
@gpucibot merge |
This reverts commit 65ca876.
This PR apply modifications to the cuGraph codebase to account for changes in RAFT and RMM :
device::bufferadapter with RMMdevice_uvectorraft#283device::allocatoradapter raft#285This PR requires some changes in the cuHornet dependency : rapidsai/cuhornet#52