Skip to content

Revise Sort lowering and sort_actor assumptions#22315

Merged
rapids-bot[bot] merged 13 commits intorapidsai:mainfrom
rjzamora:revise-sort-actor
Apr 29, 2026
Merged

Revise Sort lowering and sort_actor assumptions#22315
rapids-bot[bot] merged 13 commits intorapidsai:mainfrom
rjzamora:revise-sort-actor

Conversation

@rjzamora
Copy link
Copy Markdown
Member

@rjzamora rjzamora commented Apr 27, 2026 •
edited
Loading

Description

Motivation: The Sort(ShuffleSorted(Sort(...))) pattern will be too messy when we start utilizing OrderScheme metadata (after #22291). The current assumption that the child IR node is always a Sort is also not true/safe.

Note: This PR technically adds code, but it will allow us to remove more code once "tasks" is removed (everything ShuffleSorted related).

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@rjzamora rjzamora self-assigned this Apr 27, 2026
@rjzamora rjzamora requested a review from a team as a code owner April 27, 2026 23:59
@rjzamora rjzamora requested a review from TomAugspurger April 27, 2026 23:59
@rjzamora rjzamora added bug Something isn't working 4 - Needs Review Waiting for reviewer to review or respond non-breaking Non-breaking change labels Apr 27, 2026
@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Apr 27, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Apr 27, 2026
mroeschke
mroeschke reviewed Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

Great, it looks like the affected test now xpass in python/cudf_polars/cudf_polars/testing/inject_gpu_engine.py and can be removed.

The new failure of test_sorted_flag looks similar (in traceback) to #22084

mroeschke
mroeschke reviewed Apr 28, 2026
View reviewed changes
Comment thread python/cudf_polars/cudf_polars/experimental/rapidsmpf/collectives/sort.py
mroeschke
mroeschke reviewed Apr 28, 2026
View reviewed changes
Comment thread python/cudf_polars/cudf_polars/experimental/rapidsmpf/collectives/sort.py
@rjzamora rjzamora added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs Review Waiting for reviewer to review or respond labels Apr 29, 2026
@rjzamora
Copy link
Copy Markdown
Member Author

rjzamora commented Apr 29, 2026

/merge

@rapids-bot rapids-bot Bot merged commit a94c7d1 into rapidsai:main Apr 29, 2026
108 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Apr 29, 2026
@rjzamora rjzamora deleted the revise-sort-actor branch April 29, 2026 18:26
@rjzamora rjzamora mentioned this pull request May 1, 2026
Merged
3 tasks
vyasr pushed a commit to vyasr/cudf that referenced this pull request May 4, 2026
@rjzamora
Revise Sort lowering and sort_actor assumptions (rapidsai#22315)
2e729a7 
- Closes rapidsai#22050
- **Updates `Sort` lowering**:
  - The "rapidsmpf" runtime no longer uses `Sort(ShuffleSorted(Sort(...)))` - We leave most `Sort` nodes alone.
  - We map `Sort` directly to `sort_actor` - The `sort_actor` performs it's own top/bottom-k optimization and performs chunk-wise sorting itself.

**Motivation**: The `Sort(ShuffleSorted(Sort(...)))` pattern will be too messy when we start utilizing `OrderScheme` metadata (after rapidsai#22291). The current assumption that the child IR node is always a `Sort` is also not true/safe.

**Note**: This PR technically adds code, but it will allow us to *remove* more code once "tasks" is removed (everything `ShuffleSorted` related).

Authors:
  - Richard (Rick) Zamora (https://github.com/rjzamora)

Approvers:
  - Matthew Roeschke (https://github.com/mroeschke)

URL: rapidsai#22315
rapids-bot Bot pushed a commit that referenced this pull request May 7, 2026
@rjzamora
Refactor sort_actor to prepare for OrderScheme changes (#22350)
c9ad1c5 
- Follow up to #22315 - Further revises `sort_actor` in preparation for rapidsai/rapidsmpf#853
- Part of #22128
- Breaks apart `sort_actor` logic into modular steps, so we can avoid collecting boundaries when we already know the boundaries (future work).

Authors:
  - Richard (Rick) Zamora (https://github.com/rjzamora)

Approvers:
  - Matthew Murray (https://github.com/Matt711)
  - Matthew Roeschke (https://github.com/mroeschke)

URL: #22350
galipremsagar pushed a commit to galipremsagar/cudf that referenced this pull request May 8, 2026
@rjzamora @galipremsagar
Refactor sort_actor to prepare for OrderScheme changes (rapid…
47b699d 
…sai#22350)

- Follow up to rapidsai#22315 - Further revises `sort_actor` in preparation for rapidsai/rapidsmpf#853
- Part of rapidsai#22128
- Breaks apart `sort_actor` logic into modular steps, so we can avoid collecting boundaries when we already know the boundaries (future work).

Authors:
  - Richard (Rick) Zamora (https://github.com/rjzamora)

Approvers:
  - Matthew Murray (https://github.com/Matt711)
  - Matthew Roeschke (https://github.com/mroeschke)

URL: rapidsai#22350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mroeschke mroeschke mroeschke approved these changes

@TomAugspurger TomAugspurger Awaiting requested review from TomAugspurger TomAugspurger is a code owner automatically assigned from rapidsai/cudf-polars-codeowners

Assignees

@rjzamora rjzamora

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working cudf-polars Issues specific to cudf-polars non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

cuDF Python
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Repartion IR may passed into a sort_actor in cudf_polars + rapidsmpf

3 participants

@rjzamora @mroeschke @GPUtester