-
Notifications
You must be signed in to change notification settings - Fork 913
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 cudf::strings::contains_multiple #16900
Add cudf::strings::contains_multiple #16900
Conversation
This PR has better performance compared to the previous PR. previous PR:
End to End time is: 53s this PR
End to End time is: 46s |
Sorry for the delay. It took a while to get my head around it. Thank you, this is an impressive speedup. |
@davidwendt Other PRs are depending on this, could you merge this PR? |
/merge |
1 similar comment
/merge |
/merge |
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. Thanks!
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 will apply one small docstring fix I saw while reviewing C++, then I can approve the CMake (I didn't want to approve yet because it would auto-merge since /merge
has already been added).
This is Java JNI interface for [multiple contains PR](#16900) Authors: - Chong Gao (https://github.com/res-life) Approvers: - Alessandro Bellina (https://github.com/abellina) - Robert (Bobby) Evans (https://github.com/revans2) URL: #17281
Description
Add new
cudf::strings::contains_multiple
API to search multiple targets within a strings column.Output is a table where the number of columns is the number of targets and each row is a boolean indicating that target was found at the row or not.
This PR is to help in collaboration with #16641
Checklist