Add support for tdigest and merge_tdigest aggregations through cudf::reduce #10433
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nvdbaranec
added
feature request
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #10433 +/- ##
================================================
+ Coverage 86.13% 86.18% +0.04%
================================================
Files 139 139
Lines 22438 22468 +30
================================================
+ Hits 19328 19363 +35
+ Misses 3110 3105 -5
Continue to review full report at Codecov.
|
…tdigest aggregations is STRUCT.
devavret
reviewed
Mar 18, 2022
devavret
reviewed
Mar 18, 2022
| struct make_centroid_no_nulls { | ||
| column_device_view const col; | ||
|
|
||
| centroid operator() __device__(size_type index) const |
There was a problem hiding this comment.
I've heard we need to replace thrust::tuple with cuda::std::tuple.
devavret
reviewed
Mar 18, 2022
| offset_type const* group_offsets; | ||
|
|
||
| thrust::pair<double, int> operator() __device__(double next_limit, size_type group_index) | ||
| thrust::pair<double, int> operator() __device__(double next_limit, size_type group_index) const |
There was a problem hiding this comment.
The same thing about thrust::pair
devavret
reviewed
Mar 18, 2022
| * | ||
| * This functor assumes the weight for all scalars is simply 1. Under this assumption, | ||
| * the nearest weight that will be <= the next limit is simply the nearest integer < the limit, | ||
| * which we can get by just taking floor(next_limit). For example if our next limit is 3.56, the | ||
| * nearest whole number <= it is floor(3.56) == 3. | ||
| */ | ||
| struct nearest_value_scalar_weights { | ||
| struct nearest_value_scalar_weights_grouped { | ||
| offset_type const* group_offsets; |
cwharris
approved these changes
Mar 21, 2022
|
rerun tests |
|
@gpucibot merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, these aggregations only worked with groupby. Now they can be invoked through
cudf::reduce, producing scalar tdigest values (which under the hood are simply struct columns with 1 row).The difference between the groupby and reduce versions is minimal. They are both fundamentally
reduce_by_keyoperations, where the keys represent the bucketing of input values to merged centroids. In the case ofgroupby, the keys are further partitioned by the specific input group. So the bulk of the changes are simply adding a few extra template parameters to various internal functions to allow thereducepath to behave as if it were just a constant group.Similarly, many of the groupby tests which involved single groups have been refactored/repurposed for the reduce tests.
Most of the important changes are in
tdigest_aggregation.cu