Add 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
30f2784 to
d0914a6
Compare
|
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:
📝 WalkthroughWalkthroughAdds a new RMM 26.06 migration guide and inserts it into the user guide toctree. The guide documents C++ API/signature changes, custom resource implementation rules, CCCL constraints, and detailed Python/Cython binding migration steps. ChangesMigration Guide Documentation
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.
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
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>
df90e63 to
483c1a7
Compare
davidwendt
left a comment
There was a problem hiding this comment.
This is amazing.
I know this is just documentation but I only really read through the C++ parts.
Description
Adds an RMM 26.06 migration guide covering library API changes and memory resource migration guidance.
Closes #2345
Checklist