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.

Sign up for GitHub

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

Jump to bottom

Update to CMake 3.20 features, rapids-cmake and CPM #187

Merged
merged 24 commits into from
May 14, 2021
Merged

Update to CMake 3.20 features, rapids-cmake and CPM #187

merged 24 commits into from
May 14, 2021

Conversation

dantegd
Copy link
Member

@dantegd dantegd commented Mar 29, 2021 •
edited
Loading

PR updates RAFT's CMake and uses CPM for dependency management. Still WIP, meant for early 0.20, but opening the PR to start debugging in CI.

This is heavily based on cuDF's, RMM's and cumlprims' changes. PR does the following:

  • Adopt CPM:
    • RMM
    • FAISS
    • GTest
    • NCCL: Missing building from source, will be added if/when required
    • UCX: Missing building from source, will be added if/when required
    • CUB for CUDA < 11.0
  • Update arch detection
  • Use generators to allow clang compilation as well
  • Updates to outdated cmake parts and remove old code
  • Update code to aid transition to [FEA] Deploy raft globally #83

@dantegd dantegd added the 2 - In Progress Currenty a work in progress label Mar 29, 2021
@dantegd dantegd requested a review from divyegala as a code owner March 29, 2021 17:23
@dantegd dantegd requested a review from a team as a code owner April 6, 2021 15:55
@dantegd dantegd added enhancement New feature or request non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Apr 6, 2021
@dantegd dantegd mentioned this pull request Apr 11, 2021
Closed
@ajschmidt8 ajschmidt8 removed the request for review from a team April 22, 2021 14:09
@ajschmidt8
Copy link
Member

Removing ops-codeowners from the required reviews since it doesn't seem there are any file changes that we're responsible for. Feel free to add us back if necessary.

@dantegd dantegd changed the title [WIP] Update CMake for 3.18+ features, modern usage and use CPM for dependencies Update CMake for 3.18+ features, modern usage and use CPM for dependencies Apr 22, 2021
@dantegd dantegd added 3 - Ready for Review and removed 2 - In Progress Currenty a work in progress labels Apr 22, 2021
@JohnZed JohnZed requested a review from robertmaynard April 26, 2021 16:15
robertmaynard
robertmaynard reviewed Apr 26, 2021
View reviewed changes
cpp/cmake/thirdparty/get_faiss.cmake Outdated Show resolved Hide resolved
cpp/cmake/thirdparty/get_faiss.cmake Show resolved Hide resolved
cpp/cmake/thirdparty/get_rmm.cmake Outdated Show resolved Hide resolved
cpp/test/CMakeLists.txt Outdated
set_target_properties(test_raft
PROPERTIES BUILD_RPATH "\$ORIGIN"
# set target compile options
CXX_STANDARD 14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you can now require c++17 for both CXX and CUDA

cpp/CMakeLists.txt Outdated Show resolved Hide resolved
robertmaynard
robertmaynard reviewed Apr 26, 2021
View reviewed changes
cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@github-actions github-actions bot added the python label May 5, 2021
@robertmaynard robertmaynard mentioned this pull request May 7, 2021
Merged
@trxcllnt trxcllnt mentioned this pull request May 7, 2021
Merged
@dantegd dantegd mentioned this pull request May 9, 2021
Merged
robertmaynard
robertmaynard reviewed May 11, 2021
View reviewed changes
cpp/CMakeLists.txt Outdated
@@ -127,6 +128,7 @@ INTERFACE
CUDA::cudart
CUDA::cusparse
rmm::rmm
# cuco::cuco
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? should this be commented out?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally forgot to un-comment it, since it is not used ye. But an upcoming PR (#207) requires it (so added it here to aid there). Will go ahead and un-comment it preemptively

@dantegd dantegd mentioned this pull request May 13, 2021
Closed
@dantegd dantegd changed the title Update CMake for 3.18+ features, modern usage and use CPM for dependencies Update to CMake 3.20 features, rapids-cmake and CPM May 14, 2021
@dantegd
Copy link
Member Author

dantegd commented May 14, 2021

@gpucibot merge

divyegala
divyegala approved these changes May 14, 2021
View reviewed changes
Copy link
Member

@divyegala divyegala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving just because it needs my review for whatever reason this keeps happening for..

@rapids-bot rapids-bot bot merged commit fe5f40a into rapidsai:branch-0.20 May 14, 2021
rapids-bot bot pushed a commit to rapidsai/cuml that referenced this pull request May 25, 2021
@dantegd
Update to CMake 3.20 features, rapids-cmake and CPM (#3844)
a8d5a87 
Depends on rapidsai/raft#187

Based on the work started by @mdemoret-nv on https://github.com/mdemoret-nv/cuml/tree/imp-modernize-cmake with updates for CPM, rapids-cmake and consistency with the equivalent PRs in RAFT  and cuGraph

Details of changes will be added soon

cc @trxcllnt and @robertmaynard for viz

Authors:
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Michael Demoret (https://github.com/mdemoret-nv)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - AJ Schmidt (https://github.com/ajschmidt8)
  - John Zedlewski (https://github.com/JohnZed)

URL: #3844
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
@dantegd
Update to CMake 3.20 features, rapids-cmake and CPM (rapidsai#3844)
57364fb 
Depends on rapidsai/raft#187

Based on the work started by @mdemoret-nv on https://github.com/mdemoret-nv/cuml/tree/imp-modernize-cmake with updates for CPM, rapids-cmake and consistency with the equivalent PRs in RAFT  and cuGraph

Details of changes will be added soon

cc @trxcllnt and @robertmaynard for viz

Authors:
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Michael Demoret (https://github.com/mdemoret-nv)

Approvers:
  - Robert Maynard (https://github.com/robertmaynard)
  - AJ Schmidt (https://github.com/ajschmidt8)
  - John Zedlewski (https://github.com/JohnZed)

URL: rapidsai#3844
dantegd pushed a commit to dantegd/raft that referenced this pull request Jul 23, 2024
@benfred
Change cagra.build_index to cagra.build (rapidsai#187)
615749e 
This changes the build_index method to build in the python API for cagra. All of the other python api's use a `build` method for building the index, as do both the C++ and Rust api's as well.

Authors:
  - Ben Frederickson (https://github.com/benfred)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: rapidsai/cuvs#187
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertmaynard robertmaynard robertmaynard approved these changes

@rlratzel rlratzel rlratzel approved these changes

@divyegala divyegala divyegala approved these changes

Assignees
No one assigned
Labels
3 - Ready for Review CMake cpp enhancement New feature or request improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dantegd @ajschmidt8 @robertmaynard @rlratzel @divyegala