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

[REVIEW] Add dictionary support to cudf::quantile #6676

Merged
merged 15 commits into from
Nov 11, 2020

Conversation

davidwendt
Copy link
Contributor

@davidwendt davidwendt commented Nov 4, 2020

Reference #5963

Add support for dictionary column types in cudf::quantile. This function is also used by cudf::reduce in PR #6666.
The code change here is simply to dispatch on the dictionary key type and then pass a dictionary-iterator to transform function that computes the quantile values -- select_quantile_data.
This means this PR is also dependent on #6651 for the is_dictionary trait.

Also, added a simple gtest for testing this new dictionary code path.

@davidwendt davidwendt requested a review from a team as a code owner November 4, 2020 21:29
@davidwendt davidwendt self-assigned this Nov 4, 2020
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. labels Nov 4, 2020
@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #6676 (2ad86da) into branch-0.17 (88821fb) will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.17    #6676      +/-   ##
===============================================
+ Coverage        82.21%   82.27%   +0.06%     
===============================================
  Files               94       94              
  Lines            15478    15539      +61     
===============================================
+ Hits             12725    12785      +60     
- Misses            2753     2754       +1     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/lists.py 92.74% <0.00%> (+5.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88821fb...2ad86da. Read the comment docs.

Copy link
Contributor

@rgsl888prabhu rgsl888prabhu left a comment

Choose a reason for hiding this comment

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

LGTM

@harrism harrism added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Nov 11, 2020
@davidwendt davidwendt merged commit b2d281b into rapidsai:branch-0.17 Nov 11, 2020
@davidwendt davidwendt deleted the dictionary-quantile branch November 11, 2020 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants