Move replicated-output dedup to the Dask and Ray frontends - #22394
Merged
rapids-bot[bot] merged 13 commits intoMay 13, 2026
Conversation
github-actions
Bot
added
Python
madsbk
added a commit
to madsbk/cudf
that referenced
this pull request
May 7, 2026
wence-
reviewed
May 7, 2026
wence-
approved these changes
May 7, 2026
Contributor
|
/ok to test |
@vyasr, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
Contributor
|
/ok to test b513cf4 |
Member
Author
|
This PR depends on #22381 |
Matt711
force-pushed
the
bug/polars/multi-rank-fixes
branch
from
May 12, 2026 10:55
b513cf4 to
7663fc5
Compare
Member
Author
|
/ok to test 7663fc5 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFrontend execution layers (dask and ray) now always collect channel metadata and clear non-root rank outputs when marked duplicated. Downstream DAG nodes (union, drain, select, join) are simplified by removing rank-aware conditional forwarding and communicator wiring. Regression tests validate the new behavior. ChangesCentralized duplicate suppression refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/cudf_polars/tests/experimental/test_spmd.py`:
- Around line 312-313: The test currently only asserts the "name" column from
result = lf.collect(engine=spmd_engine); instead, produce a CPU baseline by
collecting the same LazyFrame with the CPU/Polars engine (e.g., baseline =
lf.collect(engine=<cpu_engine> or default)), normalize both frames (sort rows
deterministically, reset row order/index and column order if needed, and ensure
dtypes are comparable), then assert the full frames are equal (e.g., assert
normalized_spmd_frame.frame_equal(normalized_cpu_frame)). Update the test to use
lf.collect(engine=spmd_engine) and lf.collect(...) for the baseline and compare
the fully normalized frames to ensure full-parity checking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 75c8bd16-29bc-49b6-b405-2ba08284c21f
📒 Files selected for processing (9)
python/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/dask.pypython/cudf_polars/cudf_polars/experimental/rapidsmpf/frontend/ray.pypython/cudf_polars/cudf_polars/experimental/rapidsmpf/join.pypython/cudf_polars/cudf_polars/experimental/rapidsmpf/nodes.pypython/cudf_polars/cudf_polars/experimental/rapidsmpf/union.pypython/cudf_polars/cudf_polars/experimental/select.pypython/cudf_polars/tests/experimental/test_spmd.pypython/cudf_polars/tests/experimental/test_union.pypython/cudf_polars/tests/test_groupby.py
Member
Author
|
/ok to test ca34558 |
Matt711
force-pushed
the
bug/polars/multi-rank-fixes
branch
from
May 12, 2026 11:32
ca34558 to
da95957
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/cudf_polars/tests/experimental/test_spmd.py (1)
313-314: ⚡ Quick winMake the slice actually truncating to strengthen this regression.
head(10)on a 2-row input is a no-op, so this doesn’t strongly exercise slice behavior. Consider reducing the limit (or increasing input rows) so slicing is truly validated.Proposed minimal tweak
- lf = pl.concat([lf1, lf2]).sort("score").head(10) + lf = pl.concat([lf1, lf2]).sort("score").head(1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cudf_polars/tests/experimental/test_spmd.py` around lines 313 - 314, The test uses lf = pl.concat([lf1, lf2]).sort("score").head(10) but because the concatenated input is only 2 rows the head(10) is a no-op and doesn't exercise slice behavior; update the test so slicing actually truncates — either reduce the head limit (e.g., head(1) or head(0)) on the existing concatenated DataFrame (lf) or increase the size of lf1/lf2 so pl.concat([...]).sort("score").head(10) will actually drop rows; ensure the assertion still uses assert_gpu_result_equal(lf, engine=spmd_engine, check_row_order=False).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@python/cudf_polars/tests/experimental/test_spmd.py`:
- Around line 313-314: The test uses lf = pl.concat([lf1,
lf2]).sort("score").head(10) but because the concatenated input is only 2 rows
the head(10) is a no-op and doesn't exercise slice behavior; update the test so
slicing actually truncates — either reduce the head limit (e.g., head(1) or
head(0)) on the existing concatenated DataFrame (lf) or increase the size of
lf1/lf2 so pl.concat([...]).sort("score").head(10) will actually drop rows;
ensure the assertion still uses assert_gpu_result_equal(lf, engine=spmd_engine,
check_row_order=False).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e887bb01-7e0d-4a75-8682-a5c7d6338d0f
📒 Files selected for processing (1)
python/cudf_polars/tests/experimental/test_spmd.py
Member
Author
|
/ok to test 6b190c1 |
Matt711
force-pushed
the
bug/polars/multi-rank-fixes
branch
from
May 12, 2026 13:07
6b190c1 to
814ca39
Compare
Member
Author
|
/ok to test a9b7480 |
Matt711
force-pushed
the
bug/polars/multi-rank-fixes
branch
from
May 12, 2026 18:01
a9b7480 to
814f4d1
Compare
Member
Author
|
/ok to test 4a903a9 |
madsbk
reviewed
May 13, 2026
Member
Author
|
/ok to test 8db9ba0 |
Member
Author
|
/ok to test a842d16 |
Member
Author
|
/merged |
Member
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ChannelMetadata.duplicated=Truenow consistently means every rank holds the data.DaskEngineandRayEngine#22381Checklist