Skip to content

Remove PoolMemoryResource wrapper - #7878

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
jcrist:cuml-accel-rmm
Mar 12, 2026
Merged

Remove PoolMemoryResource wrapper#7878
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
jcrist:cuml-accel-rmm

Conversation

@jcrist

@jcrist jcrist commented Mar 12, 2026

Copy link
Copy Markdown
Member

I'm seeing some OOMs in CI after the PoolMemoryResource was added. Trying removing it to see if that fixes it.

I'm seeing some OOMs in CI after the `PoolMemoryResource` was added.
Trying removing it to see if that fixes it.
@jcrist jcrist self-assigned this Mar 12, 2026
@jcrist
jcrist requested a review from a team as a code owner March 12, 2026 16:26
@jcrist jcrist added the improvement Improvement / enhancement to an existing function label Mar 12, 2026
@jcrist
jcrist requested a review from dantegd March 12, 2026 16:26
@jcrist jcrist added non-breaking Non-breaking change cuml-accel Issues related to cuml.accel labels Mar 12, 2026
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Mar 12, 2026
@coderabbitai

coderabbitai Bot commented Mar 12, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0d0214eb-faea-4771-9cdf-b2bb499f2d5e

📥 Commits

Reviewing files that changed from the base of the PR and between 8c06f91 and 3d93ae9.

📒 Files selected for processing (1)
  • python/cuml/cuml/accel/core.py

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Simplified memory resource initialization by reducing intermediate resource layers for improved efficiency.

Walkthrough

Simplified memory resource configuration in CUML's acceleration core by removing an intermediate PoolMemoryResource layer from the PrefetchResourceAdaptor construction and streamlining associated type checks.

Changes

Cohort / File(s) Summary
Memory Resource Simplification
python/cuml/cuml/accel/core.py
Removed intermediate PoolMemoryResource wrapping layer; PrefetchResourceAdaptor now directly uses ManagedMemoryResource. Updated type checks to reflect the simplified resource chain structure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: removing the PoolMemoryResource wrapper, which aligns with the file changes and PR objectives.
Description check ✅ Passed The description is related to the changeset, explaining the motivation (OOM issues) for removing the PoolMemoryResource wrapper.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan for PR comments
  • Generate coding plan

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

@jcrist

jcrist commented Mar 12, 2026

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 01edd09 into NVIDIA:main Mar 12, 2026
176 of 178 checks passed
jcrist added a commit to jcrist/cuml that referenced this pull request Mar 12, 2026
I'm seeing some OOMs in CI after the `PoolMemoryResource` was added. Trying removing it to see if that fixes it.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: NVIDIA#7878
rapids-bot Bot pushed a commit that referenced this pull request Mar 12, 2026
Cherry-pick of #7878 into the release branch for 26.04.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - Simon Adorf (https://github.com/csadorf)

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

Labels

cuml-accel Issues related to cuml.accel Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants