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

Add stream and mr parameters for structs::detail::flatten_nested_columns #12892

Merged
merged 8 commits into from
Mar 9, 2023
Merged

Add stream and mr parameters for structs::detail::flatten_nested_columns #12892

merged 8 commits into from
Mar 9, 2023

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Mar 7, 2023

The internal API structs::detail::flatten_nested_columns currently always use default stream and device resources. This PR adds stream and mr parameters for it, allowing to pass in the current stream and mr values.

Closes #12349.

@ttnghia ttnghia added feature request New feature or request 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 7, 2023
@ttnghia ttnghia requested a review from a team as a code owner March 7, 2023 05:52
@ttnghia ttnghia self-assigned this Mar 7, 2023
@ttnghia ttnghia requested review from vyasr and karthikeyann March 7, 2023 05:52
@ttnghia ttnghia removed the feature request New feature or request label Mar 7, 2023
vyasr
vyasr approved these changes Mar 7, 2023
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

This LGTM pending the removal of the default mr from the flatten_nested_columns signature.

bdice
bdice reviewed Mar 8, 2023
View reviewed changes
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.

Two minor suggestions, otherwise LGTM!

cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved
cpp/src/structs/utilities.cpp Outdated Show resolved Hide resolved
@ttnghia
Copy link
Contributor Author

ttnghia commented Mar 8, 2023

/merge

@rapids-bot rapids-bot bot merged commit be0f583 into rapidsai:branch-23.04 Mar 9, 2023
@ttnghia ttnghia deleted the add_stream_structs_flattening branch March 9, 2023 00:20
@ttnghia ttnghia mentioned this pull request Mar 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PointKernel PointKernel PointKernel left review comments

@bdice bdice bdice left review comments

@vyasr vyasr vyasr approved these changes

@divyegala divyegala divyegala approved these changes

@karthikeyann karthikeyann karthikeyann approved these changes

@hyperbolic2346 hyperbolic2346 hyperbolic2346 approved these changes

Assignees

@ttnghia ttnghia

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] Support stream and mr parameters for structs::detail::flatten_nested_columns
7 participants
@ttnghia @vyasr @hyperbolic2346 @bdice @karthikeyann @PointKernel @divyegala