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

Add groupby max aggregation benchmark #11464

Merged
merged 20 commits into from
Aug 4, 2022

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Aug 4, 2022

This adds a simple benchmark for groupby max aggregation.

@ttnghia ttnghia added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change not a bug labels Aug 4, 2022
@ttnghia ttnghia requested a review from a team as a code owner August 4, 2022 02:24
@ttnghia ttnghia added this to PR-WIP in v22.10 Release via automation Aug 4, 2022
@ttnghia ttnghia self-assigned this Aug 4, 2022
@github-actions github-actions bot added the CMake CMake build issue label Aug 4, 2022
@ttnghia ttnghia moved this from PR-WIP to PR-Needs review in v22.10 Release Aug 4, 2022
@ttnghia ttnghia added the improvement Improvement / enhancement to an existing function label Aug 4, 2022
@codecov
Copy link

codecov bot commented Aug 4, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@9429099). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.10   #11464   +/-   ##
===============================================
  Coverage                ?   86.47%           
===============================================
  Files                   ?      144           
  Lines                   ?    22856           
  Branches                ?        0           
===============================================
  Hits                    ?    19765           
  Misses                  ?     3091           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@karthikeyann karthikeyann left a comment

Choose a reason for hiding this comment

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

LGTM 👍

v22.10 Release automation moved this from PR-Needs review to PR-Reviewer approved Aug 4, 2022

auto const input_table = [&] {
data_profile profile;
profile.set_null_frequency(std::nullopt);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be curious to know the performance impact when including nulls.
Could this be added as a state parameter?
Perhaps just a single value like 10% or less would be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added nulls for the values column.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, that makes sense.

@ttnghia
Copy link
Contributor Author

ttnghia commented Aug 4, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 95e2206 into rapidsai:branch-22.10 Aug 4, 2022
v22.10 Release automation moved this from PR-Reviewer approved to Done Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team CMake CMake build issue improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants