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

Implement percentile computation from histogram data #1416

Merged
merged 36 commits into from
Oct 16, 2023
Merged

Implement percentile computation from histogram data #1416

merged 36 commits into from
Oct 16, 2023

Conversation

ttnghia
Copy link
Collaborator

@ttnghia ttnghia commented Sep 16, 2023 •
edited
Loading

This implements the last evaluation step to support distributed percentile aggregation. The first (update) and second (merge) steps are implemented through HISTOGRAM and MERGE_HISTOGRAM aggregations in cudf (rapidsai/cudf#14045).

Depends on:

Status:

  • Working prototypes.
  • Integration tests in spark-rapids integration tests (reduction + groupby) passed.
  • Clean up and docs.

@ttnghia ttnghia self-assigned this Sep 16, 2023
@ttnghia
Add Java interface
17d7c9e 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Input percentage as GPU column
58969ea 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Wrap in list if needed
ba1f543 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Implementing groupby percentile
e958e51 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Merge branch 'branch-23.10' into percentile
fda35fe
@ttnghia
Cleanup and docs
9368413 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Fix null issue
a801a14 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Implement null check
85e7465 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Fix typo
69c8df8 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Cleanup
981d1ca 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Cleanup
b54aa51 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Change back to use the less error-prone segmented sort
43ead20 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia ttnghia marked this pull request as ready for review September 30, 2023 04:04
@ttnghia
Copy link
Collaborator Author

ttnghia commented Sep 30, 2023

build

@ttnghia
Fix all nulls cases
e762784 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Debugging
1c5949a 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Fix null handling
b918a64 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Fix empty input handling
84390e1 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Copy link
Collaborator Author

ttnghia commented Oct 1, 2023

build

ttnghia added 2 commits October 1, 2023 14:34
@ttnghia
Fix access indices
16f71c7 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Fix accumulated counts computation
3904468 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia
Copy link
Collaborator Author

ttnghia commented Oct 2, 2023

build

@ttnghia ttnghia mentioned this pull request Oct 2, 2023
Merged
@ttnghia ttnghia marked this pull request as draft October 2, 2023 05:37
@ttnghia
WIP
d24207b 
Signed-off-by: Nghia Truong <[email protected]>
@ttnghia ttnghia marked this pull request as ready for review October 6, 2023 20:44
@ttnghia ttnghia changed the base branch from branch-23.10 to branch-23.12 October 11, 2023 15:59
revans2
revans2 reviewed Oct 11, 2023
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

The java Side of things looks good, but I don't feel confident really reviewing the C++ code. I could follow what was being done and it looks good, but I am no expert here.

hyperbolic2346
hyperbolic2346 reviewed Oct 13, 2023
View reviewed changes
Copy link
Collaborator

@hyperbolic2346 hyperbolic2346 left a comment

Choose a reason for hiding this comment

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

First pass looks good. Some questions.

src/main/cpp/src/histogram.cu Show resolved Hide resolved
src/main/cpp/src/histogram.cu Outdated Show resolved Hide resolved
src/main/cpp/src/histogram.cu Show resolved Hide resolved
src/main/cpp/src/histogram.cu Show resolved Hide resolved
@ttnghia
Copy link
Collaborator Author

ttnghia commented Oct 14, 2023

build

@ttnghia ttnghia merged commit 150bafc into NVIDIA:branch-23.12 Oct 16, 2023
1 check passed
@ttnghia ttnghia deleted the percentile branch October 16, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@revans2 revans2 revans2 left review comments

@hyperbolic2346 hyperbolic2346 hyperbolic2346 approved these changes

Assignees

@ttnghia ttnghia

Labels
feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ttnghia @revans2 @hyperbolic2346