-
Notifications
You must be signed in to change notification settings - Fork 917
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
Rework nvtext::detokenize to use indexalator for row indices #12267
Rework nvtext::detokenize to use indexalator for row indices #12267
Conversation
Codecov ReportBase: 88.37% // Head: 88.16% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## branch-23.02 #12267 +/- ##
================================================
- Coverage 88.37% 88.16% -0.21%
================================================
Files 137 137
Lines 22657 22665 +8
================================================
- Hits 20022 19982 -40
- Misses 2635 2683 +48
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one comment on naming. Otherwise LGTM!
cpp/src/text/detokenize.cu
Outdated
cudf::size_type tokens_counts, | ||
rmm::cuda_stream_view stream) | ||
{ | ||
index_changed_fn pfn{cudf::detail::indexalator_factory::make_input_iterator(row_indices), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what pfn
means. "P..." function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'p' is silent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@gpucibot merge |
Description
Rework
nvtext::detokenize
to use thecudf::detail::make_strings_children
and thecudf::detail::indexalator
.This allows the operation to throw an error if it the output would exceed the size limit of a column.
The indexalator usage removes the need for a type-dispatcher call for the row-indices.
No function has been added, removed, or changed.
Reference #12167
Checklist