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

[FEA] Move nullable index iterator creation logic to indexalator_factory #6469

Closed
davidwendt opened this issue Oct 8, 2020 · 1 comment · Fixed by #7399
Closed

[FEA] Move nullable index iterator creation logic to indexalator_factory #6469

davidwendt opened this issue Oct 8, 2020 · 1 comment · Fixed by #7399
Assignees
@davidwendt
Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.

Comments

@davidwendt
Copy link
Contributor

Reference discussion here #6457 (comment)

The existing logic for

cudf/cpp/src/dictionary/replace.cu

Line 41 in 48e9d1b

struct nullable_index_accessor {

and

cudf/cpp/src/dictionary/replace.cu

Lines 72 to 77 in 48e9d1b

template <bool has_nulls>
auto make_nullable_index_iterator(column_view const& col)
{
return thrust::make_transform_iterator(thrust::make_counting_iterator<size_type>(0),
nullable_index_accessor<has_nulls>{col});
}

should be moved to be an indexalatory utility for use in other places where a pair-iterator with an indexalator is needed.

Also, the logic should check that an indices column contains no nulls.
@davidwendt davidwendt added feature request New feature or request Needs Triage Need team to review and classify labels Oct 8, 2020
@davidwendt davidwendt self-assigned this Oct 8, 2020
@davidwendt davidwendt added the libcudf Affects libcudf (C++/CUDA) code. label Oct 8, 2020
@davidwendt davidwendt mentioned this issue Oct 8, 2020
Merged
@kkraus14 kkraus14 removed the Needs Triage Need team to review and classify label Oct 15, 2020
@github-actions
Copy link

This issue has been marked rotten due to no recent activity in the past 90d. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

@davidwendt davidwendt mentioned this issue Feb 17, 2021
Merged
rapids-bot bot pushed a commit that referenced this issue Feb 19, 2021
@davidwendt
Move nullable index iterator to indexalator factory (#7399)
2ae7498 
Closes #6469 

The `cudf::dictionary::replace` function uses a pair iterator over the child column indices. The logic uses an indexalator input iterator to build the pair iterator. This PR moves the pair iterator creation to the indexalator_factory so it can be used more generally.

Authors:
  - David (@davidwendt)

Approvers:
  - Conor Hoekstra (@codereport)
  - Karthikeyan (@karthikeyann)

URL: #7399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@davidwendt davidwendt

Labels
feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Move nullable index iterator to indexalator factory davidwendt/cudf
2 participants
@kkraus14 @davidwendt