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

Refactor cudf::detail::sorted_order #13062

Merged
merged 16 commits into from
Apr 12, 2023
Merged

Refactor cudf::detail::sorted_order #13062

merged 16 commits into from
Apr 12, 2023

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Apr 4, 2023 •
edited
Loading

This PR does some cleanup for the src/sort/sort_impl.cuh file and the related headers/source files:

  • Moving some include<header> from there to the directly used source files.
  • Adding constexpr for the if/else statements.
  • Adding missing doxygen tag.
  • Removing code duplicate by extracting the common code into a lambda.

There is not any new implementation added in this PR.

@ttnghia ttnghia added feature request New feature or request 3 - Ready for Review Ready for review by team libcudf blocker libcudf Affects libcudf (C++/CUDA) code. Spark Functionality that helps Spark RAPIDS non-breaking Non-breaking change labels Apr 4, 2023
@ttnghia ttnghia requested review from vyasr, bdice and divyegala April 4, 2023 22:57
@ttnghia ttnghia self-assigned this Apr 4, 2023
@ttnghia ttnghia requested a review from a team as a code owner April 4, 2023 22:57
@ttnghia ttnghia marked this pull request as draft April 5, 2023 16:45
@ttnghia ttnghia marked this pull request as ready for review April 5, 2023 17:11
@ttnghia ttnghia requested a review from davidwendt April 5, 2023 17:12
@ttnghia ttnghia marked this pull request as draft April 5, 2023 17:29
@ttnghia ttnghia changed the title Allow passing different types of physical element comparator to cudf::detail::sorted_order Refactor cudf::detail::sorted_order Apr 7, 2023
ttnghia
ttnghia commented Apr 7, 2023
View reviewed changes
cpp/src/sort/sort_column_impl.cuh
Comment on lines +60 to +61
template <typename T>
struct simple_comparator {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is moved from sort_impl.cuh.

@ttnghia ttnghia removed request for vyasr, bdice and divyegala April 7, 2023 18:38
@ttnghia ttnghia added improvement Improvement / enhancement to an existing function and removed libcudf blocker feature request New feature or request Spark Functionality that helps Spark RAPIDS labels Apr 7, 2023
@ttnghia ttnghia marked this pull request as ready for review April 7, 2023 18:39
vyasr
vyasr requested changes Apr 7, 2023
View reviewed changes
cpp/src/sort/sort.cu Outdated Show resolved Hide resolved
cpp/src/sort/stable_sort.cu Outdated Show resolved Hide resolved
cpp/src/sort/sort_impl.cuh Outdated Show resolved Hide resolved
cpp/src/sort/sort_column_impl.cuh Show resolved Hide resolved
@ttnghia ttnghia requested a review from vyasr April 9, 2023 22:49
@ttnghia
Copy link
Contributor Author

ttnghia commented Apr 12, 2023

/merge

@rapids-bot rapids-bot bot merged commit 1d77984 into rapidsai:branch-23.06 Apr 12, 2023
@ttnghia ttnghia deleted the extend_sorting branch April 12, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidwendt davidwendt davidwendt approved these changes

@vyasr vyasr vyasr 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.
3 participants
@ttnghia @vyasr @davidwendt