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

Add device_memory_resource parameter to create_string_vector_from_column #10673

Merged

Conversation

davidwendt
Copy link
Contributor

@davidwendt davidwendt commented Apr 15, 2022

Adds the rmm::mr::device_memory_resource parameter to the cudf::strings::detail::create_string_vector_from_column function. This will be called in a future API in a later PR and the resulting memory object will returned to the user.

Also found and removed a few related functions that are no longer necessary and updated the callers appropriately simplifying the logic there.

@davidwendt davidwendt added 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 15, 2022
@davidwendt davidwendt self-assigned this Apr 15, 2022
@codecov
Copy link

codecov bot commented Apr 15, 2022

Codecov Report

Merging #10673 (58b03c1) into branch-22.06 (94a5d41) will increase coverage by 0.02%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           branch-22.06   #10673      +/-   ##
================================================
+ Coverage         86.38%   86.41%   +0.02%     
================================================
  Files               142      142              
  Lines             22334    22334              
================================================
+ Hits              19294    19300       +6     
+ Misses             3040     3034       -6     
Impacted Files Coverage Δ
python/cudf/cudf/core/column/string.py 89.22% <0.00%> (+0.12%) ⬆️
python/cudf/cudf/core/groupby/groupby.py 91.72% <0.00%> (+0.22%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <0.00%> (+0.30%) ⬆️
python/cudf/cudf/core/column/lists.py 92.79% <0.00%> (+1.27%) ⬆️

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 94a5d41...58b03c1. Read the comment docs.

@davidwendt davidwendt added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Apr 18, 2022
@davidwendt davidwendt marked this pull request as ready for review April 18, 2022 12:22
@davidwendt davidwendt requested a review from a team as a code owner April 18, 2022 12:22
@davidwendt davidwendt requested review from vyasr and ttnghia April 18, 2022 12:22
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Nice! All looks good to me.

@davidwendt
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 08cd428 into rapidsai:branch-22.06 Apr 19, 2022
@davidwendt davidwendt deleted the string-view-vector-update branch April 19, 2022 15:35
abellina added a commit to abellina/cudf that referenced this pull request Apr 20, 2022
rapids-bot bot pushed a commit that referenced this pull request Apr 27, 2022
Closes #10717 

Fixes bug introduced with changes in #10673 which uses the `cudf::make_strings_column` that accepts a span of `string_view` objects with a null-placeholder. The placeholder can be unintentionally created in `create_string_vector_from_column` when given a strings column where all the rows are empty. The utility is fixed to prevent creating the placeholder for empty strings.

A gtest was added to scatter from/to an all-empty strings column to verify this behavior.

Authors:
  - David Wendt (https://github.com/davidwendt)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - Bradley Dice (https://github.com/bdice)
  - Robert Maynard (https://github.com/robertmaynard)

URL: #10724
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 improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants