Skip to content

Fix order preservation in grouped-window evaluation - #23524

Merged
rapids-bot[bot] merged 6 commits into
NVIDIA:mainfrom
rjzamora:fix-order-preservation
Aug 4, 2026
Merged

Fix order preservation in grouped-window evaluation#23524
rapids-bot[bot] merged 6 commits into
NVIDIA:mainfrom
rjzamora:fix-order-preservation

Conversation

@rjzamora

@rjzamora rjzamora commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #23466

That PR introduced support for input-order-sensitive over(...) expressions without an explicit order_by argument. However, there is a bug: We do not properly sort by the original local row-position (only the rank and chunk index). Since the hash-partitioning of the forward shuffle is not technically "stable", we need to include this local row position in the forward sort operation as well.

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 Aug 4, 2026
@rjzamora
rjzamora requested a review from a team as a code owner August 4, 2026 14:12
@rjzamora rjzamora added the bug Something isn't working label Aug 4, 2026
@rjzamora
rjzamora requested a review from madsbk August 4, 2026 14:12
@rjzamora rjzamora added the non-breaking Non-breaking change label Aug 4, 2026
@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Aug 4, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Aug 4, 2026
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 609c6ab1-85b7-4a30-a732-07846bb3f2b8

📥 Commits

Reviewing files that changed from the base of the PR and between 12ac15c and c6a6c1d.

📒 Files selected for processing (2)
  • python/cudf_polars/cudf_polars/streaming/actor_graph/over.py
  • python/cudf_polars/tests/streaming/test_spmd.py

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved deterministic ordering behavior in window operations to ensure consistent within-chunk input-order preservation.
  • Tests

    • Added streaming test case for partitioned data sources with sorting and cumulative aggregation operations.

Walkthrough

The stamped-row sort now preserves input order within origin chunks by including row position. A single-rank streaming test validates cumulative grouped window results from a partitioned Parquet scan.

Changes

Window ordering

Layer / File(s) Summary
Add origin row ordering
python/cudf_polars/cudf_polars/streaming/actor_graph/over.py
The stamped-row sort adds origin row position as an ascending key after origin rank and chunk index.
Validate partitioned cumulative windows
python/cudf_polars/tests/streaming/test_spmd.py
A single-rank streaming test scans partitioned Parquet data, sorts by x, computes cumulative sums over g, and validates the result.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • rapidsai/cudf#23306: Updates the same window evaluation path with stamped-row input-order sorting.
  • PR 23466: Adds the stamped-row input-order sorting that this change extends with origin row-position ordering.

Suggested labels: improvement

Suggested reviewers: mroeschke, nirandaperera, tomaugspurger

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly addresses the main change: fixing order preservation in grouped-window evaluation, which matches the core fix described in the PR objectives.
Description check ✅ Passed The description explains the bug fix context, references a related PR, and details the specific problem with hash-partitioning stability and the required sort operation changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@rjzamora rjzamora added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Aug 4, 2026
@rjzamora

rjzamora commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit fab61d4 into NVIDIA:main Aug 4, 2026
112 checks passed
@rjzamora
rjzamora deleted the fix-order-preservation branch August 4, 2026 18:07
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants