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

Test updates of CCCL (thrust, cub, libcudacxx) to 2.1.0. #1247

Closed
wants to merge 1 commit into from

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Apr 15, 2023

This PR tests a rapids-cmake branch with CCCL (thrust, cub, libcudacxx) updated to 2.1.0. Do not merge this PR. The changes will be merged upstream in rapids-cmake after all libraries pass CI.

@github-actions github-actions bot added CMake Python Related to RMM Python API labels Apr 15, 2023
@bdice bdice added 5 - DO NOT MERGE Hold off on merging; see PR for details and removed CMake Python Related to RMM Python API labels Apr 15, 2023
@bdice
Copy link
Contributor Author

bdice commented Apr 19, 2023

@robertmaynard @vyasr Thrust added a dependency on libcudacxx in version 2.0.0. That means we get different behavior than I had hoped for with rapidsai/rapids-cmake#399. Specifically, Thrust is pulling in libcudacxx version 1.8.1.0 from its git submodule dependency, while I would like to pin to the latest version 2.1.0. What would you recommend doing here? Do we need to add a CMake requirement for libcudacxx before Thrust finds its own outdated submodule copy?

Before (Thrust 1.17.2):

-- CPM: adding package [email protected] (1.17.2)
-- Found Thrust: $SRC_DIR/build/_deps/thrust-src/thrust/cmake/thrust-config.cmake (found version "1.17.2.0") 
-- rapids-cmake [Thrust]: applied diff transform_iter_with_reduce_by_key.diff to fix issue: 'Support transform iterator with reduce by key [https://github.com/NVIDIA/thrust/pull/1805]'
-- rapids-cmake [Thrust]: applied diff install_rules.diff to fix issue: 'Thrust 1.X installs incorrect files [https://github.com/NVIDIA/thrust/issues/1790]'
-- Found CUB: $SRC_DIR/build/_deps/thrust-src/dependencies/cub/cub/cmake/cub-config.cmake (found suitable version "1.17.2.0", minimum required is "1.17.2.0") 

After (Thrust 2.1.0):

-- CPM: adding package [email protected] (2.1.0)
-- Found libcudacxx: $SRC_DIR/build/_deps/thrust-src/dependencies/libcudacxx/lib/cmake/libcudacxx/libcudacxx-config.cmake (found suitable version "1.8.1.0", minimum required is "1.8.0") 
-- Found Thrust: $SRC_DIR/build/_deps/thrust-src/thrust/cmake/thrust-config.cmake (found version "2.1.0.0") 
-- Found CUB: $SRC_DIR/build/_deps/thrust-src/dependencies/cub/cub/cmake/cub-config.cmake (found suitable version "2.1.0.0", minimum required is "2.1.0.0") 

@robertmaynard
Copy link
Contributor

@robertmaynard @vyasr Thrust added a dependency on libcudacxx in version 2.0.0. That means we get different behavior than I had hoped for with rapidsai/rapids-cmake#399. Specifically, Thrust is pulling in libcudacxx version 1.8.1.0 from its git submodule dependency, while I would like to pin to the latest version 2.1.0. What would you recommend doing here? Do we need to add a CMake requirement for libcudacxx before Thrust finds its own outdated submodule copy?

We should update rapids-cmake/cpm/thrust.cmake to be aware of this new dependency and have it call rapids-cmake/cpm/libcudacxx.cmake before it adds thrust so we always get the expected libcudacxx version.

@github-actions github-actions bot added CMake Python Related to RMM Python API labels Apr 26, 2023
@bdice bdice force-pushed the cccl-update-2.1.0 branch 2 times, most recently from 07e0d6e to 8d4031e Compare April 26, 2023 01:38
@bdice
Copy link
Contributor Author

bdice commented Apr 26, 2023

@robertmaynard I have now updated libcudacxx upstream and it appears that builds/tests are passing. I verified in the logs that Thrust/CUB/libcudacxx versions 2.1.0 are being used.

I will open testing PRs for other RAPIDS repositories.

@bdice bdice changed the base branch from branch-23.06 to branch-23.10 July 21, 2023 17:53
@bdice
Copy link
Contributor Author

bdice commented Aug 1, 2023

Note to self: switch rmm to use fetch_rapids.cmake. That way, the logic for rapids-cmake won’t be duplicated between the C++ and Python CMakeLists.txt. https://github.com/rapidsai/cudf/blob/branch-23.10/fetch_rapids.cmake

edit: done in #1319.

@bdice bdice mentioned this pull request Aug 1, 2023
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Aug 1, 2023
This PR migrates RMM to use `fetch_rapids.cmake` like most RAPIDS repos. This makes it easier to define a single source if the upstream branch of rapids-cmake needs to change for testing, like in #1247.

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Ray Douglass (https://github.com/raydouglass)
  - Robert Maynard (https://github.com/robertmaynard)

URL: #1319
@github-actions github-actions bot removed CMake Python Related to RMM Python API labels Aug 1, 2023
fetch_rapids.cmake Outdated Show resolved Hide resolved
@robertmaynard robertmaynard self-requested a review August 3, 2023 13:48
@bdice bdice self-assigned this Sep 13, 2023
@bdice bdice mentioned this pull request Dec 7, 2023
3 tasks
@bdice
Copy link
Contributor Author

bdice commented Dec 7, 2023

Closing in favor of #1404.

@bdice bdice closed this Dec 7, 2023
rapids-bot bot pushed a commit that referenced this pull request Dec 19, 2023
This PR updates RMM to CCCL 2.2.0. Do not merge until all of RAPIDS is ready to update.

Depends on rapidsai/rapids-cmake#495.

Replaces #1247.

Authors:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Mark Harris (https://github.com/harrism)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1404
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - DO NOT MERGE Hold off on merging; see PR for details
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants