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 cudf::dictionary::make_dictionary_pair_iterator #6651

Merged

Conversation

davidwendt
Copy link
Contributor

This adds a pair iterator for dictionary column types that returns key/valid pairs for each row access.
This is useful in PR #6585 and will be useful in supporting dictionary type columns in cudf::reduce.

This PR also adds an cudf::is_dictionary to traits.hpp
The cudf::is_unsigned was declared in an odd place in the traits.hpp and so was also moved in this PR.

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

codecov bot commented Nov 3, 2020

Codecov Report

Merging #6651 into branch-0.17 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           branch-0.17    #6651   +/-   ##
============================================
  Coverage        82.33%   82.33%           
============================================
  Files               94       94           
  Lines            15369    15369           
============================================
  Hits             12654    12654           
  Misses            2715     2715           

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 1556a49...cb87204. Read the comment docs.

Copy link
Contributor

@codereport codereport left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Couple small changes

cpp/include/cudf/dictionary/detail/iterator.cuh Outdated Show resolved Hide resolved
cpp/include/cudf/dictionary/detail/iterator.cuh Outdated Show resolved Hide resolved
cpp/include/cudf/dictionary/detail/iterator.cuh Outdated Show resolved Hide resolved
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

@davidwendt davidwendt merged commit 1f8f262 into rapidsai:branch-0.17 Nov 5, 2020
@davidwendt davidwendt deleted the dictionary-make-pair-iterator branch November 9, 2020 16:19
rapids-bot bot pushed a commit that referenced this pull request Dec 1, 2020
Reference #5963 

This PR adds dictionary column type support to the set of `cudf::reduce` functions.
This PR depends on utilities added in PR #6651 

Here are the reduce operations that will be included in this PR.
- [x] all
- [x] any
- [x] max
- [x] mean
- [x] median
- [x] min
- [x] nth_element
- [x] product
- [x] quantile
- [x] std
- [x] sum_of_squares
- [x] sum
- [x] unique_count
- [x] var

Authors:
  - davidwendt <[email protected]>

Approvers:
  - Mike Wendt
  - AJ Schmidt
  - Ram (Ramakrishna Prabhu)
  - Karthikeyan

URL: #6666
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 libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants