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.26.4 #13538

Merged
merged 2 commits into from
Jun 13, 2023
Merged

Update to CMake 3.26.4 #13538

merged 2 commits into from
Jun 13, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jun 8, 2023 •
edited
Loading

Updates minimum required CMake version to 3.26.4. This will allow us to take advantage of a variety of new features in the last three minor releases, particularly with respect to better CUDA and Python support. See rapidsai/rapids-cmake#315 for more information.

@vyasr vyasr requested review from a team as code owners June 8, 2023 22:17
@github-actions github-actions bot added CMake CMake build issue conda Java Affects Java cuDF API. Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Jun 8, 2023
@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 8, 2023
ttnghia
ttnghia approved these changes Jun 8, 2023
View reviewed changes
Copy link
Contributor

@ttnghia ttnghia left a comment

Choose a reason for hiding this comment

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

Is there any specific reason why this upgrade is needed?

@vyasr
Copy link
Contributor Author

vyasr commented Jun 8, 2023

Is there any specific reason why this upgrade is needed?

Good question. We want to upgrade beyond 3.23 because there are features in newer versions that we could take advantage of mainly with improvements to FindCUDAToolkit and FindPython as well as better CUDA support in other areas (e.g. the new built-in CMAKE_CUDA_ARCHITECTURES=native option).

@ttnghia
Copy link
Contributor

ttnghia commented Jun 8, 2023

Then can you update that into the PR description, please?

@vyasr
Copy link
Contributor Author

vyasr commented Jun 9, 2023

Sure! Done.

@ttnghia
Copy link
Contributor

ttnghia commented Jun 9, 2023

Although this is not merged, upstream changes have been propagated:

[INFO]      [exec] CMake Error at build/_deps/kvikio-src/cpp/CMakeLists.txt:15 (cmake_minimum_required):
[INFO]      [exec]   CMake 3.26.4 or higher is required.  You are running version 3.23.3

@vyasr
Copy link
Contributor Author

vyasr commented Jun 9, 2023

Although this is not merged, upstream changes have been propagated:

[INFO]      [exec] CMake Error at build/_deps/kvikio-src/cpp/CMakeLists.txt:15 (cmake_minimum_required):
[INFO]      [exec]   CMake 3.26.4 or higher is required.  You are running version 3.23.3

Where are you seeing this? Locally? It shouldn't be an issue in CI anywhere because the conda environments and pip builds will just install the latest CMake from conda-forge/PyPI. If it's local you need to update your local env.

@ttnghia
Copy link
Contributor

ttnghia commented Jun 10, 2023 •
edited
Loading

Yes I see it locally. Not sure if CI will have any issue with it because the upstream kvikio PR was merged (rapidsai/kvikio#238).

But it doesn't matter to me since I've updated my local CMake.

@NvTimLiu NvTimLiu mentioned this pull request Jun 12, 2023
Merged
rapids-bot bot pushed a commit that referenced this pull request Jun 12, 2023
@NvTimLiu
Update CMake in docker to 3.26.4 (#13550)
deec3f8 
As Rapids updates CMake version to 3.26.4, we also need to update it in our docker image.

Similar PR :  #13538

Signed-off-by: Tim Liu <[email protected]>

Authors:
  - Tim Liu (https://github.com/NvTimLiu)

Approvers:
  - Peixin (https://github.com/pxLi)
  - Nghia Truong (https://github.com/ttnghia)

URL: #13550
bdice
bdice approved these changes Jun 12, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Thanks! Can we merge this, or is there a list of downstream PRs that we need to merge first?

conda/recipes/cudf_kafka/conda_build_config.yaml
@@ -6,3 +6,6 @@ cxx_compiler_version:

sysroot_version:
- "2.17"

cmake_version:
Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to avoid using conda_build_config.yaml for defining a variable when the version is only specified once in a recipe. I see the value of being consistent between cudf_kafka and other recipes, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I made this change since it then made it easily to do a batch upgrade across RAPIDS when everything is consistent. Could always add one more file to sed though.

@vyasr
Copy link
Contributor Author

vyasr commented Jun 13, 2023

Nope this is mergeable now.

@vyasr
Copy link
Contributor Author

vyasr commented Jun 13, 2023

/merge

@rapids-bot rapids-bot bot merged commit 59238c1 into rapidsai:branch-23.08 Jun 13, 2023
@vyasr vyasr deleted the updatetocmake3264 branch June 13, 2023 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttnghia ttnghia ttnghia approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

@bdice bdice bdice approved these changes

@wence- wence- Awaiting requested review from wence- wence- is a code owner automatically assigned from rapidsai/cudf-python-codeowners

@isVoid isVoid Awaiting requested review from isVoid isVoid is a code owner automatically assigned from rapidsai/cudf-python-codeowners

@hyperbolic2346 hyperbolic2346 Awaiting requested review from hyperbolic2346 hyperbolic2346 is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

@nvdbaranec nvdbaranec Awaiting requested review from nvdbaranec nvdbaranec is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees
No one assigned
Labels
CMake CMake build issue improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vyasr @ttnghia @bdice @ajschmidt8