Skip to content

Make RapidsMPF the default runtime for cudf_polars streaming executor - #22281

Merged
rapids-bot[bot] merged 86 commits into
NVIDIA:mainfrom
mroeschke:enh/cudf_polars/rapidsmpf_default
May 7, 2026
Merged

Make RapidsMPF the default runtime for cudf_polars streaming executor#22281
rapids-bot[bot] merged 86 commits into
NVIDIA:mainfrom
mroeschke:enh/cudf_polars/rapidsmpf_default

Conversation

@mroeschke

@mroeschke mroeschke commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Description

closes #21466
closes #21767

Waiting for #22212

  • Makes rapidsmpf a required dependency of cudf_polars
  • Removes the following StreamingExecutor options as they were "experimental" with associated code paths
    • StreamingExecutor.runtime
    • StreamingExecutor.shuffle_method
    • StreamingExecutor.unique_fraction
    • StreamingExecutor.groupby_n_ary
    • StreamingExecutor.rapidsmpf_spill
  • Removes the task runtime and associated tests
  • Some tests we modified to only test 1 specific test configuration because of [FEA] Change cudf_polars testing assertion function to require an engine argument. #22346 to pass these tests for now. Planning on revisiting this once rapidsmpf becomes the default

Ops-Bot-Merge-Barrier: true

Checklist

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

@mroeschke mroeschke self-assigned this Apr 23, 2026
@mroeschke mroeschke added improvement Improvement / enhancement to an existing function breaking Breaking change labels Apr 23, 2026
@copy-pr-bot

copy-pr-bot Bot commented Apr 23, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions Bot added Python Affects Python cuDF API. cudf-polars Issues specific to cudf-polars labels Apr 23, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Apr 23, 2026
@mroeschke

Copy link
Copy Markdown
Contributor Author

/ok to test 4a37ee2

@mroeschke

Copy link
Copy Markdown
Contributor Author

/ok to test 3e4dfe0

@mroeschke

Copy link
Copy Markdown
Contributor Author

/ok to test 49849d0

@mroeschke

Copy link
Copy Markdown
Contributor Author

/ok to test bc92082

@Matt711 Matt711 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Exciting!

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devcontainers look better now. I approve as long as CI passes.

)
for i, key in enumerate(partition_info[ir].keys(ir))
}
return evaluate_rapidsmpf(ir, config_options)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In a followup we should merge evaluate_rapidsmpf and evaluate_streaming...

Comment on lines +294 to +297
# Target about 12.5% of the device memory when
# default_target_partition_size is used to set the
# target partition size (i.e. 5x the 2.5% default).
return min(5, int(max(1, (device_size * 0.125) // 1e9)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TODO: I think we want to move towards making these limits all byte-based, rather than partition count.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I agree. I proposed this verbally on Monday, and had a feeling you'd agree. It seems unnecessarily complicated tie target_partition_size and broadcast_join_limit together in the way we currently do. After this PR goes in, we may want to consider replacing broadcast_join_limit with a byte-based broadcast_limit before 26.02.

Comment on lines 583 to +584
client_device_threshold
Threshold for spilling data from device memory in rapidsmpf.
Threshold for spilling data from device memory.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Why is client in this name?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's only used for the "single" cluster case and can be removed when we remove that code path (in favor of "spmd").

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving to unblock, some final questions.

Comment thread .github/workflows/pr.yaml Outdated
sccache --zero-stats;
build-all -j0 -DBUILD_BENCHMARKS=ON --verbose 2>&1 | tee telemetry-artifacts/build.log;
clone-rapidsmpf -j$(nproc) -v -q --branch "$(cat ~/cudf/RAPIDS_BRANCH)" --clone-upstream --depth 1 --single-branch --shallow-submodules;
if [ "$PYTHON_PACKAGE_MANAGER" = "pip" ]; then rapids-make-pip-env --force; fi;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like you would need the same thing in conda, since rapidsmpf is being cloned late. Let’s check on the dependency list and make sure it doesn’t have rapidsmpf or cudf before merging.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, fair point. In fe5502b, the final, recreated conda environment creation doesn't include rapidsmpf

Comment thread .github/workflows/pr.yaml
build-all -j0 -DBUILD_BENCHMARKS=ON --verbose 2>&1 | tee telemetry-artifacts/build.log;
clone-rapidsmpf -j$(nproc) -v -q --branch "$(cat ~/cudf/RAPIDS_BRANCH)" --clone-upstream --depth 1 --single-branch --shallow-submodules;
if [ "$PYTHON_PACKAGE_MANAGER" = "pip" ]; then rapids-make-pip-env --force; fi;
build-all -j0 -DBUILD_BENCHMARKS=OFF -DBUILD_NUMA_SUPPORT=OFF -DBUILD_TESTS=OFF -DCUDF_BUILD_TESTUTIL=OFF --verbose 2>&1 | tee telemetry-artifacts/build.log;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We might need to call rapids-generate-scripts. I am not sure if build-all will do everything unless the scripts were regenerated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure I added it, but in fe5502b I can't tell if it materially made a difference

@mroeschke

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 563021f into NVIDIA:main May 7, 2026
133 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python May 7, 2026
@mroeschke
mroeschke deleted the enh/cudf_polars/rapidsmpf_default branch May 7, 2026 01:55
galipremsagar pushed a commit to galipremsagar/cudf that referenced this pull request May 8, 2026
…NVIDIA#22281)

closes NVIDIA#21466
closes NVIDIA#21767

Waiting for NVIDIA#22212

* Makes rapidsmpf a required dependency of cudf_polars
* Removes the following `StreamingExecutor` options as they were "experimental" with associated code paths
    * `StreamingExecutor.runtime`
    * `StreamingExecutor.shuffle_method`
    * `StreamingExecutor.unique_fraction`
    * `StreamingExecutor.groupby_n_ary`
    * `StreamingExecutor.rapidsmpf_spill`
* Removes the task runtime and associated tests
* Some tests we modified to only test 1 specific test configuration because of NVIDIA#22346 to pass these tests for now. Planning on revisiting this once rapidsmpf becomes the default

Ops-Bot-Merge-Barrier: true

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

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - Bradley Dice (https://github.com/bdice)
  - Matthew Murray (https://github.com/Matt711)
  - Lawrence Mitchell (https://github.com/wence-)

URL: NVIDIA#22281
rapids-bot Bot pushed a commit that referenced this pull request May 11, 2026
…lars (#22417)

* Address the follow up review #22281 (comment)

> In a followup we should merge evaluate_rapidsmpf and evaluate_streaming...

* Removes a temporary iterable in `select` that I had from a leftover branch 
* Removes the `devcontainers` key in `dependencies.yml` that was leftover from troubleshooting the devcontainer CI job

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

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - James Lamb (https://github.com/jameslamb)
  - Peter Andreas Entschev (https://github.com/pentschev)

URL: #22417
shrshi pushed a commit to shrshi/cudf that referenced this pull request May 12, 2026
…NVIDIA#22281)

closes NVIDIA#21466
closes NVIDIA#21767

Waiting for NVIDIA#22212

* Makes rapidsmpf a required dependency of cudf_polars
* Removes the following `StreamingExecutor` options as they were "experimental" with associated code paths
    * `StreamingExecutor.runtime`
    * `StreamingExecutor.shuffle_method`
    * `StreamingExecutor.unique_fraction`
    * `StreamingExecutor.groupby_n_ary`
    * `StreamingExecutor.rapidsmpf_spill`
* Removes the task runtime and associated tests
* Some tests we modified to only test 1 specific test configuration because of NVIDIA#22346 to pass these tests for now. Planning on revisiting this once rapidsmpf becomes the default

Ops-Bot-Merge-Barrier: true

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

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - Bradley Dice (https://github.com/bdice)
  - Matthew Murray (https://github.com/Matt711)
  - Lawrence Mitchell (https://github.com/wence-)

URL: NVIDIA#22281
shrshi pushed a commit to shrshi/cudf that referenced this pull request May 12, 2026
…lars (NVIDIA#22417)

* Address the follow up review NVIDIA#22281 (comment)

> In a followup we should merge evaluate_rapidsmpf and evaluate_streaming...

* Removes a temporary iterable in `select` that I had from a leftover branch 
* Removes the `devcontainers` key in `dependencies.yml` that was leftover from troubleshooting the devcontainer CI job

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

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - James Lamb (https://github.com/jameslamb)
  - Peter Andreas Entschev (https://github.com/pentschev)

URL: NVIDIA#22417
rapids-bot Bot pushed a commit that referenced this pull request May 13, 2026
…y; add [dask] pip extra (#22480)

* Follow up to making rapidsmpf a required dependency of cudf-polars, #22281, we no longer need a `cudf-polars[rapidsmpf]` pip extra
* Removes `numpy` as a testing dependency as it was only used for `np.random` and `np.full`
* Removes `"nvidia-ml-py>=12"` from the `experimental` extra as it's already a required dependency of cudf_polars
* Adds `cudf-polars[dask]` as an alias for `cudf-polars[experimental]`

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

Approvers:
  - Tom Augspurger (https://github.com/TomAugspurger)
  - Bradley Dice (https://github.com/bdice)
  - Matthew Murray (https://github.com/Matt711)
  - Gil Forsyth (https://github.com/gforsyth)

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

Labels

breaking Breaking change cudf-polars Issues specific to cudf-polars improvement Improvement / enhancement to an existing function Python Affects Python cuDF API.

Projects

None yet

8 participants