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

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

Merged
merged 17 commits into from
Feb 4, 2021

Conversation

rlratzel
Copy link
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

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
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.

cpp/CMakeLists.txt Outdated Show resolved Hide resolved
…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
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
Member

@gpucibot merge

cpp/CMakeLists.txt Outdated Show resolved Hide resolved
@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