Skip to content

Use (un)spill_partitions from RapidsMPF - #22837

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:mainfrom
pentschev:use-spill-partitions-from-rapidsmpf
Jun 10, 2026
Merged

Use (un)spill_partitions from RapidsMPF#22837
rapids-bot[bot] merged 3 commits into
NVIDIA:mainfrom
pentschev:use-spill-partitions-from-rapidsmpf

Conversation

@pentschev

Copy link
Copy Markdown
Contributor

#22810 was merged with changes to remove the (un)spill_partition functions from cudf-streaming in favor of using them from RapidsMPF. However, simultaneously #22814 got merged introducing some new use cases for those functions, that allowed
#22810 to pass CI and be merged. This change fixes coverage.

NVIDIA#22810 was merged with changes to
remove the (un)spill_partition functions from cudf-streaming in favor of
using them from RapidsMPF. However, simultaneously
NVIDIA#22814 got merged introducing some
new use cases for those functions, that allowed
NVIDIA#22810 to pass CI and be merged.
This change fixes coverage.
@pentschev
pentschev requested review from a team as code owners June 10, 2026 16:01
@pentschev pentschev self-assigned this Jun 10, 2026
@pentschev pentschev added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change cudf-polars Issues specific to cudf-polars labels Jun 10, 2026
@github-actions github-actions Bot added libcudf Affects libcudf (C++/CUDA) code. Python Affects Python cuDF API. labels Jun 10, 2026
@GPUtester GPUtester moved this to In Progress in cuDF Python Jun 10, 2026

@madsbk madsbk 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.

Thanks @pentschev

@coderabbitai

coderabbitai Bot commented Jun 10, 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: bf5f5938-402a-4b54-9bb8-38de6fd119f6

📥 Commits

Reviewing files that changed from the base of the PR and between 502bdad and 33992c0.

📒 Files selected for processing (10)
  • ci/cpp_linters.sh
  • cpp/libcudf_streaming/benchmarks/bench_shuffle.cpp
  • cpp/libcudf_streaming/benchmarks/streaming/ndsh/join.cpp
  • cpp/libcudf_streaming/examples/example_shuffle.cpp
  • cpp/libcudf_streaming/tests/test_shuffler.cpp
  • python/cudf_streaming/cudf_streaming/examples/bulk_mpi_shuffle.py
  • python/cudf_streaming/cudf_streaming/examples/bulk_ray_shuffle.py
  • python/cudf_streaming/cudf_streaming/examples/ray_shuffle_example.py
  • python/cudf_streaming/cudf_streaming/tests/test_integration_partition.py
  • python/cudf_streaming/cudf_streaming/tests/test_shuffler.py

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Switched spill/unspill handling to use an external memory spill API across benchmarks, examples, and tests to standardize memory operations.
  • Chores
    • CI linting script now normalizes compiler flags before running checks, removing incompatible GCC-only flags when present.

Walkthrough

This PR moves unspill_partitions calls and imports from cudf_streaming.integrations/partition to rapidsmpf (C++ and Python), adds rapidsmpf spill includes where needed, and normalizes some CI linter flags.

Changes

Unspill Function Namespace Migration

Layer / File(s) Summary
C++ header dependencies for rapidsmpf spill
cpp/libcudf_streaming/benchmarks/bench_shuffle.cpp, cpp/libcudf_streaming/benchmarks/streaming/ndsh/join.cpp, cpp/libcudf_streaming/examples/example_shuffle.cpp, cpp/libcudf_streaming/tests/test_shuffler.cpp
Adds rapidsmpf/memory/spill.hpp include to C++ translation units that call unspill_partitions.
C++ join benchmark namespace migration
cpp/libcudf_streaming/benchmarks/streaming/ndsh/join.cpp
Removes the using cudf_streaming::integrations::unspill_partitions alias and updates both broadcast and shuffle unpack_and_concat call sites to use rapidsmpf::unspill_partitions.
C++ bench & example unspill updates
cpp/libcudf_streaming/benchmarks/bench_shuffle.cpp, cpp/libcudf_streaming/examples/example_shuffle.cpp
Replaces per-partition/packed-data unspill calls with rapidsmpf::unspill_partitions at the unpack/unpack-and-concat sites.
C++ shuffler test unspill calls
cpp/libcudf_streaming/tests/test_shuffler.cpp
Updates multiple test helper paths and TEST cases to call rapidsmpf::unspill_partitions instead of the prior cudf_streaming::integrations namespaced function.
Python import rewiring for rapidsmpf
python/cudf_streaming/cudf_streaming/examples/bulk_mpi_shuffle.py, python/cudf_streaming/cudf_streaming/examples/bulk_ray_shuffle.py, python/cudf_streaming/cudf_streaming/examples/ray_shuffle_example.py, python/cudf_streaming/cudf_streaming/tests/test_integration_partition.py, python/cudf_streaming/cudf_streaming/tests/test_shuffler.py
Moves spill_partitions/unspill_partitions imports to rapidsmpf.memory.spill and narrows cudf_streaming.integrations.partition imports to partition helpers only.
CI clang-tidy flag normalization
ci/cpp_linters.sh
Strips -fno-merge-constants from CFLAGS/CXXFLAGS when set and collapses/trims whitespace before running clang-tidy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

  • rapidsai/cudf#22810: Related migration PR that updates shuffle/partition call sites to use rapidsmpf::unspill_partitions.

Suggested reviewers

  • nirandaperera
  • vyasr
  • madsbk
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Use (un)spill_partitions from RapidsMPF' clearly and concisely summarizes the main change across all modified files—switching from cudf_streaming integrations to RapidsMPF implementations.
Description check ✅ Passed The description relates directly to the changeset by explaining the context (PRs 22810 and 22814) and purpose (fixing coverage gaps) for switching to RapidsMPF's (un)spill_partitions implementations.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@pentschev
pentschev requested a review from a team as a code owner June 10, 2026 16:39
@pentschev
pentschev requested a review from KyleFromNVIDIA June 10, 2026 16:39

@nirandaperera nirandaperera 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.

LGTM. Thanks @pentschev

@jameslamb

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 6f6b12c into NVIDIA:main Jun 10, 2026
199 of 203 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in cuDF Python Jun 10, 2026
@pentschev
pentschev deleted the use-spill-partitions-from-rapidsmpf branch June 11, 2026 12:23
abigalekim pushed a commit to abigalekim/cudf that referenced this pull request Jun 12, 2026
NVIDIA#22810 was merged with changes to remove the (un)spill_partition functions from cudf-streaming in favor of using them from RapidsMPF. However, simultaneously NVIDIA#22814 got merged introducing some new use cases for those functions, that allowed
NVIDIA#22810 to pass CI and be merged. This change fixes coverage.

Authors:
  - Peter Andreas Entschev (https://github.com/pentschev)
  - Niranda Perera (https://github.com/nirandaperera)

Approvers:
  - Mads R. B. Kristensen (https://github.com/madsbk)
  - Niranda Perera (https://github.com/nirandaperera)
  - Bradley Dice (https://github.com/bdice)

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

Labels

3 - Ready for Review Ready for review by team bug Something isn't working cudf-polars Issues specific to cudf-polars libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants