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 to CCCL 2.2.0. #5702

Merged
merged 6 commits into from
Dec 19, 2023
Merged

Update to CCCL 2.2.0. #5702

merged 6 commits into from
Dec 19, 2023

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Dec 16, 2023

This PR updates cuml to CCCL 2.2.0. Do not merge until all of RAPIDS is ready to update.

Replaces #5623.

ci/build_cpp.sh Outdated Show resolved Hide resolved
fetch_rapids.cmake Outdated Show resolved Hide resolved
@bdice bdice marked this pull request as ready for review December 18, 2023 16:01
@bdice bdice requested review from a team as code owners December 18, 2023 16:01
cpp/CMakeLists.txt Show resolved Hide resolved
@github-actions github-actions bot added the ci label Dec 19, 2023
@@ -76,13 +76,18 @@ void get_probabilities(const raft::handle_t& handle,
rmm::device_uvector<value_t> deaths(n_clusters, stream);
thrust::fill(exec_policy, deaths.begin(), deaths.end(), 0.0f);

cudaError_t (*reduce_func)(void*,
Copy link
Member

Choose a reason for hiding this comment

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

Why was the change to a function pointer needed?

Copy link
Contributor Author

@bdice bdice Dec 19, 2023

Choose a reason for hiding this comment

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

I'm not sure. CUB was failing to compile the previous code because of some API change. This code is borrowed from #5623. @hcho3 Do you have anything to add here?

@divyegala We may have to merge this as-is and revisit the question, or else CI will be blocked once the rest of RAPIDS is updated to CCCL 2.2.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change was made in b1302bd (#5623) but the commit message only says "Fix build."

Copy link
Member

Choose a reason for hiding this comment

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

Okay, it's not egregious so I will approve for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I kept getting the build error like

error: no instance of function template "detail::Utils::cub_segmented_reduce" matches the argument list
            argument types are: (float *, float *, int, int *, cudaStream_t, <unknown-type>)

because the compiler failed to build the type of cub::DeviceSegmentedReduce::Max. I had to explicitly declare the type of cub::DeviceSegmentedReduce::Max by declaring a function pointer.

@bdice bdice added improvement Improvement / enhancement to an existing function breaking Breaking change labels Dec 19, 2023
@vyasr vyasr removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Dec 19, 2023
@bdice
Copy link
Contributor Author

bdice commented Dec 19, 2023

/merge

@rapids-bot rapids-bot bot merged commit 546bcb5 into rapidsai:branch-24.02 Dec 19, 2023
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change ci CMake CUDA/C++ improvement Improvement / enhancement to an existing function
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants