Skip to content

Update CMakeLists.txt files for consistency with RAPIDS and to support cugraph as an external project and other tech debt removal#1367

Merged
rapids-bot[bot] merged 17 commits intorapidsai:branch-0.18from
rlratzel:branch-0.18-cmakeupdate
Feb 4, 2021
Merged

Update CMakeLists.txt files for consistency with RAPIDS and to support cugraph as an external project and other tech debt removal#1367
rapids-bot[bot] merged 17 commits intorapidsai:branch-0.18from
rlratzel:branch-0.18-cmakeupdate

Conversation

@rlratzel
Copy link
Copy Markdown
Contributor

@rlratzel rlratzel commented Feb 1, 2021

This PR makes cuGraph's cmake files more consistent with other RAPIDS libs by matching the minimum required cmake version, adding project() statements to cugraph's thirdparty modules, and using CMAKE_CURRENT_SOURCE_DIR appropriately so paths are relative to the CMakeLists.txt file rather than the top-level cmake dir of the project (since that may not be the cugraph cpp dir in the case of cugraph being used as an external project by another application).

This also adds a CUDA_ARCHITECTURES=OFF setting to suppress the warning printed for each test target. This setting may be replaced/changed once the findcudatoolkit feature is used in a future cmake version.

This also removes the Arrow and GTest cmake files since Arrow is not a direct dependency and those files were not being used, and GTest is now a build requirement in the conda dev environment and does not need to be built from source (the conda dev env files have been updated accordingly).

This PR also addresses much of #1075 , but not completely since gunrock is still using ExternalProject due to (I think) updates that need to be made to their cmake files to support this.

This was tested by observing a successful clean build, however it was not tested by creating a separate cmake application to simulate cugraph being used as a 3rd party package.

Note: the changes in this PR were modeled after rapidsai/rmm#541

closes #1137
closes #1266

…, replaced usage of CMAKE_SOURCE_DIR with CMAKE_CURRENT_SOURCE_DIR and added project() lines to thirdparty packages to better support users including cugraph as a 3rd party project in their applications (and for consistency with RAPIDS).
…e requirement back to thirdparty modules to remove warning.
@rlratzel rlratzel added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 1, 2021
@rlratzel rlratzel self-assigned this Feb 1, 2021
@rlratzel rlratzel requested a review from a team as a code owner February 1, 2021 06:31
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 1, 2021

Codecov Report

Merging #1367 (30640f7) into branch-0.18 (2fb0725) will decrease coverage by 0.46%.
The diff coverage is 57.84%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.18    #1367      +/-   ##
===============================================
- Coverage        60.38%   59.92%   -0.47%     
===============================================
  Files               67       68       +1     
  Lines             3029     3084      +55     
===============================================
+ Hits              1829     1848      +19     
- Misses            1200     1236      +36     
Impacted Files Coverage Δ
python/cugraph/centrality/__init__.py 100.00% <ø> (ø)
python/cugraph/link_analysis/pagerank.py 100.00% <ø> (ø)
python/cugraph/comms/comms.py 34.52% <25.00%> (ø)
python/cugraph/dask/common/input_utils.py 23.07% <28.57%> (+1.14%) ⬆️
python/cugraph/utilities/utils.py 67.18% <35.71%> (-4.37%) ⬇️
python/cugraph/dask/common/mg_utils.py 37.50% <38.09%> (-2.50%) ⬇️
python/cugraph/community/spectral_clustering.py 72.54% <38.46%> (-11.67%) ⬇️
python/cugraph/structure/number_map.py 58.12% <50.00%> (+2.16%) ⬆️
python/cugraph/structure/graph.py 66.99% <76.47%> (+0.19%) ⬆️
python/cugraph/__init__.py 100.00% <100.00%> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e112a3...30640f7. Read the comment docs.

@BradReesWork BradReesWork added this to the 0.18 milestone Feb 1, 2021
Copy link
Copy Markdown
Collaborator

@ChuckHastings ChuckHastings left a comment

Choose a reason for hiding this comment

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

One little thing would be nice.

Comment thread cpp/CMakeLists.txt Outdated
…env file (rapids docs meta pacakge is now recommended)
…es with gunrock cmake files), updated python setup utils to understand new FetchContent dirs and syntax when reading from CMakeLists.txt
@rlratzel rlratzel requested a review from a team as a code owner February 2, 2021 21:14
@rlratzel rlratzel requested a review from a team as a code owner February 2, 2021 21:16
Copy link
Copy Markdown
Collaborator

@ChuckHastings ChuckHastings left a comment

Choose a reason for hiding this comment

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

LGTM

@rlratzel rlratzel changed the title Update CMakeLists.txt files for consistency with RAPIDS and to support cugraph as an external project Update CMakeLists.txt files for consistency with RAPIDS and to support cugraph as an external project and other tech debt removal Feb 3, 2021
@BradReesWork
Copy link
Copy Markdown
Member

@gpucibot merge

Comment thread cpp/CMakeLists.txt Outdated
@rapids-bot rapids-bot Bot merged commit 7266cdb into rapidsai:branch-0.18 Feb 4, 2021
@rlratzel rlratzel deleted the branch-0.18-cmakeupdate branch June 17, 2022 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENH] Add gtest to conda package [ENH] Fix CMake warnings in external projects

5 participants