Add 26.04 to 26.06 migration guide#2344
Conversation
|
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. |
## Summary - Remove `device_memory_resource` base class usage, de-template all resource and adaptor types, replace pointer-based per-device resource APIs with ref-based equivalents - Part of rapidsai/rmm#2011. Migration guide: rapidsai/rmm#2344. - Supersedes #2917 and #2920 Depends on rapidsai/rmm#2361. Depends on rapidsai/ucxx#636. ## Changes ### Core resource infrastructure - **`device_memory_resource.hpp`**: Remove `any_resource_bridge` (which inherited from `rmm::mr::device_memory_resource`), remove all `shared_ptr<device_memory_resource>` constructor overloads, consolidate to `any_resource`-only path - **`device_resources.hpp`**: Remove deprecated constructor taking `shared_ptr<device_memory_resource>`, update `get_workspace_resource()` return type (de-templated `limiting_resource_adaptor`) - **`device_resources_snmg.hpp`**: Remove stale include, de-template `pool_memory_resource` - **`handle.hpp`**: Remove deprecated constructors taking `shared_ptr<device_memory_resource>` - **`device_resources_manager.hpp`**: Retype `workspace_mrs` vector from `shared_ptr<device_memory_resource>` to `raft::mr::device_resource`, update `set_workspace_memory_resource()` signature accordingly, de-template `pool_mr_` to `optional<pool_memory_resource>`, remove `dynamic_cast` for upstream type detection, replace `get/set_current_device_resource()` with `_ref` variants ### Memory tracking - **`memory_tracking_resources.hpp`**: Remove `device_tracking_bridge` (inherited from `device_memory_resource`), use `set_current_device_resource_ref()` directly ### Call sites using `get_workspace_resource()` → `get_workspace_resource_ref()` - `select_k-inl.cuh`, `select_radix.cuh`, `select_warpsort.cuh`, `sparse/select_k-inl.cuh`, `bitmap_to_csr.cuh`, `bitset_to_csr.cuh` ### Benchmarks - **`benchmark.hpp`**: De-template `pool_memory_resource`, use `any_resource` for RAII restore - **`gather.cu`**, **`subsample.cu`**: Same pattern ### Tests - **`handle.cpp`**: Dereference `limiting_resource_adaptor*` for `device_buffer` constructor - **`device_resources_manager.cpp`**: Remove workspace-related test code for removed APIs - **`mdarray.cu`**: Remove `test_device_resource_bridge_unwrap` (bridge no longer exists) - **`multi_variable_gaussian.cu`**: `get_current_device_resource()` → `get_current_device_resource_ref()` Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Divye Gala (https://github.com/divyegala) URL: #2996
Covers the removal of device_memory_resource, de-templating of all resources/adaptors, CCCL resource concept requirements (including allocate_sync/deallocate_sync, copyability for any_resource, and get_property friend restrictions), and downstream Cython migration patterns for .pxd and .pyx files.
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
30f2784 to
d0914a6
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds a comprehensive migration guide for RMM 26.04 → 26.06, documenting breaking C++ and Python/Cython API changes including resource model de-templating, CCCL concept adoption, ownership model changes, and binding updates. The guide is integrated into Sphinx navigation and includes implementation patterns, constraints, and troubleshooting guidance. ChangesRMM 26.04 → 26.06 Migration Guide
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/migration_guide_2604_to_2606.md`:
- Around line 577-580: The doc currently tells Cython authors to keep using
self.get_mr() in .pyx code, but the migration requires switching to
mr.c_ref.value(); update the migration text (the lines referencing
self.c_obj.get(), self.get_mr(), and device_async_resource_ref) to instruct
authors to replace uses of self.c_obj.get() / self.get_mr() with
mr.c_ref.value() when passing a resource ref from Cython to C++, and adjust
examples and the other affected section (lines ~676-690) to consistently show
mr.c_ref.value() as the correct replacement.
- Around line 209-214: The migration guide currently conflicts: the prose states
`_ref` reset APIs are deprecated-but-still-present in 26.06, but the per-device
table marks reset_per_device_resource_ref and reset_current_device_resource_ref
as removed; update the table rows (and the similar entries at lines referenced
219-220) to explicitly mark those functions as "Deprecated (still available in
26.06; removed in 26.08)" or similar wording, and ensure the surrounding prose
references the exact symbols reset_per_device_resource_ref and
reset_current_device_resource_ref (and `_ref` setters/reset functions) so
readers clearly understand they remain in 26.06 as deprecated compatibility APIs
and will be removed in 26.08.
🪄 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: 8d5fb5c0-d933-4799-b8c8-6d054af9ad04
📒 Files selected for processing (1)
docs/migration_guide_2604_to_2606.md
| owning `any_resource` setters instead. The `_ref` setters and reset functions | ||
| still exist in 26.06 as deprecated compatibility APIs, but will be removed in | ||
| 26.08: | ||
|
|
||
| | Removed (26.04) | Replacement (26.06) | | ||
| |-----------------|---------------------| |
There was a problem hiding this comment.
Clarify contradictory _ref reset API status in the per-device table.
The prose says _ref reset APIs still exist in 26.06 (deprecated), but the table marks reset_per_device_resource_ref / reset_current_device_resource_ref as removed. Please make the status explicit (deprecated-but-still-available vs removed) to avoid migration ambiguity.
As per coding guidelines: "Clarity: Flag confusing explanations, missing prerequisites, or unclear examples" and "Consistency: Version numbers, parameter types, and terminology match code".
Also applies to: 219-220
🤖 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 `@docs/migration_guide_2604_to_2606.md` around lines 209 - 214, The migration
guide currently conflicts: the prose states `_ref` reset APIs are
deprecated-but-still-present in 26.06, but the per-device table marks
reset_per_device_resource_ref and reset_current_device_resource_ref as removed;
update the table rows (and the similar entries at lines referenced 219-220) to
explicitly mark those functions as "Deprecated (still available in 26.06;
removed in 26.08)" or similar wording, and ensure the surrounding prose
references the exact symbols reset_per_device_resource_ref and
reset_current_device_resource_ref (and `_ref` setters/reset functions) so
readers clearly understand they remain in 26.06 as deprecated compatibility APIs
and will be removed in 26.08.
| **Cython bindings authors:** | ||
| - Replace `self.c_obj.get()` (which returned `device_memory_resource*`) with | ||
| `self.get_mr()` (which returns `device_async_resource_ref`). | ||
| - Keep using `get_mr()` in `.pyx` code when passing a resource ref to C++. |
There was a problem hiding this comment.
Fix conflicting Python/Cython migration guidance (get_mr() vs c_ref.value()).
These lines instruct downstreams to keep using get_mr(), but the PR objective explicitly states migration in .pyx should move from mr.get_mr() to mr.c_ref.value(). This mismatch can cause incorrect downstream migrations.
Suggested doc edit
- - Replace `self.c_obj.get()` (which returned `device_memory_resource*`) with
- `self.get_mr()` (which returns `device_async_resource_ref`).
- - Keep using `get_mr()` in `.pyx` code when passing a resource ref to C++.
+ - Replace `self.c_obj.get()` (which returned `device_memory_resource*`) with
+ `self.c_ref.value()` (which yields `device_async_resource_ref`).
+ - In `.pyx`, use `mr.c_ref.value()` when passing a resource ref to C++.-In `.pyx` files that call C++ functions, keep using `mr.get_mr()` when the
-target C++ function takes `device_async_resource_ref`. `get_mr()` returns a
-`device_async_resource_ref` and can be used directly inside `with nogil:` blocks:
+In `.pyx` files that call C++ functions, use `mr.c_ref.value()` when the
+target C++ function takes `device_async_resource_ref`. It can be used directly
+inside `with nogil:` blocks:
...
- result = cpp_my_function(input.view(), mr.get_mr())
+ result = cpp_my_function(input.view(), mr.c_ref.value())
...
- result = cpp_my_function(input.view(), mr.get_mr())
+ result = cpp_my_function(input.view(), mr.c_ref.value())
...
- result = cpp_my_function(input.view(), make_any_device_resource(mr.get_mr()))
+ result = cpp_my_function(input.view(), make_any_device_resource(mr.c_ref.value()))As per coding guidelines: "Accuracy: Verify code examples compile and run correctly" and "Consistency: Version numbers, parameter types, and terminology match code".
Also applies to: 676-690
🤖 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 `@docs/migration_guide_2604_to_2606.md` around lines 577 - 580, The doc
currently tells Cython authors to keep using self.get_mr() in .pyx code, but the
migration requires switching to mr.c_ref.value(); update the migration text (the
lines referencing self.c_obj.get(), self.get_mr(), and
device_async_resource_ref) to instruct authors to replace uses of
self.c_obj.get() / self.get_mr() with mr.c_ref.value() when passing a resource
ref from Cython to C++, and adjust examples and the other affected section
(lines ~676-690) to consistently show mr.c_ref.value() as the correct
replacement.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/migration_guide_2606.md (1)
650-653:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve conflicting
.pyxmigration guidance (get_mr()vsc_ref.value()).These sections still instruct downstream
.pyxcode to useget_mr(), but this conflicts with the stated migration contract to move call sites tomr.c_ref.value(). Please align both prose and examples to a single migration path to avoid incorrect downstream updates.As per coding guidelines: "Accuracy: Verify code examples compile and run correctly" and "Consistency: Version numbers, parameter types, and terminology match code".
Also applies to: 754-777
🤖 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 `@docs/migration_guide_2606.md` around lines 650 - 653, The migration guide currently gives conflicting advice for Cython `.pyx` bindings—some places say replace self.c_obj.get() with self.get_mr() while others instruct moving call sites to mr.c_ref.value(); pick one consistent migration path and update the prose and examples accordingly: either (A) change every occurrence and example that mentions self.get_mr() to instead call mr.c_ref.value() where a raw device_memory_resource* is required, or (B) change all references that suggest using mr.c_ref.value() to consistently show using self.get_mr() and how to extract the underlying pointer from that object; update the paragraphs around the examples and the example code blocks (references: self.c_obj.get(), self.get_mr(), mr.c_ref.value(), and any .pyx snippets) so the guide shows one coherent, compile-able approach throughout.
🤖 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.
Duplicate comments:
In `@docs/migration_guide_2606.md`:
- Around line 650-653: The migration guide currently gives conflicting advice
for Cython `.pyx` bindings—some places say replace self.c_obj.get() with
self.get_mr() while others instruct moving call sites to mr.c_ref.value(); pick
one consistent migration path and update the prose and examples accordingly:
either (A) change every occurrence and example that mentions self.get_mr() to
instead call mr.c_ref.value() where a raw device_memory_resource* is required,
or (B) change all references that suggest using mr.c_ref.value() to consistently
show using self.get_mr() and how to extract the underlying pointer from that
object; update the paragraphs around the examples and the example code blocks
(references: self.c_obj.get(), self.get_mr(), mr.c_ref.value(), and any .pyx
snippets) so the guide shows one coherent, compile-able approach throughout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0d1895ed-64e4-4608-9d17-d6e8a83c9469
📒 Files selected for processing (2)
docs/migration_guide_2606.mddocs/user_guide/guide.md
✅ Files skipped from review due to trivial changes (1)
- docs/user_guide/guide.md
Description
Adds a migration guide covering all breaking changes from RMM 26.04 to 26.06, and adds it to the docs toctree.
Closes #2345
The guide covers:
device_memory_resourcebase class andowning_wrapperUpstreamparameter removed)operator==,get_propertyfriendany_resource(wrapping non-copyable state inshared_ptr)get_propertyfriend restriction in function-scoped classes_refvariants).pxdpointer-to-value-type,.pyxget_mr()toc_ref.value())Written and refined during the cudf migration (rapidsai/cudf#21018).
Checklist