Forward merge release/26.06 into main - #8119
Closed
jcrist wants to merge 7 commits into
Closed
Conversation
This is required to import `libcuml`, but wasn't explicitly listed as a dependency. The parent main user-facing package (`cuml`) does have a `rmm` dependency, which in turn depends on `librmm`, so everything would be installed properly normally anyway. Just adding an explicit link here for tidyness. Part of NVIDIA#7845. Authors: - Jim Crist-Harif (https://github.com/jcrist) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: NVIDIA#8110
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds librmm to libcuml runtime and raises NumPy minimum to 1.26 across packaging and CI; patches cuml.accel to handle sklearn ColumnTransformer/FeatureUnion/Pipeline with numpy output, adds docs and tests for third‑party acceleration, and updates internal CuPy PTDS, validation, materialization, and related tests. ChangesDependencies and NumPy pins
cuml.accel: sklearn patches, docs, and tests
Internals: PTDS, validation, materialization, and tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Our pipeline data transfer optimization didn't work if any of the steps were other compositional metaestimators that wrapped accelerated estimators (since these could then accidentally use the accelerated versions, resulting in a mix of `cupy` and `numpy` results). This PR patches the other two compositional estimators (`FeatureUnion` and `ColumnTransformer`) so they always run within a `numpy` output-type context. Fixes NVIDIA#8112. Fixes a few sklearn examples as well (yay!) Authors: - Jim Crist-Harif (https://github.com/jcrist) Approvers: - Simon Adorf (https://github.com/csadorf) URL: NVIDIA#8115
This configures cupy to use the per-thread default stream (PTDS) for _most_ operations. This avoids usage of the default legacy stream in more of the codebase, allowing for improved parallelism when running across multiple threads. **This is a breaking change.** Previously any cupy operations in `cuml` ran in cupy's default stream (the legacy stream). We didn't synchronize the stream before returning, but that didn't matter due to the synchronization behavior of the legacy stream. With this PR we've moved to running (most) cupy operations in the PTDS. Depending on the operation, we may not synchronize the PTDS before returning. **Most users shouldn't notice a difference and should have no issues.** Users not using threads, custom streams, or only working with host memory (e.g. numpy in/numpy out) should see no difference. Likewise any users that only use cupy's default stream (the legacy stream) in their code should see no issues. Users doing tricky things with custom streams or threads may run into issues and require a manual sync of the PTDS (can be done with `cupy.cuda.Stream.ptds.synchronize()`. For example, the following workflow _may_ run into issues: - Run a cuml operation based on cupy in thread A, returning a cupy array - Consume that output in thread B as a cupy array using a stream other than the legacy stream (e.g. a different PTDS or a custom stream) For safety, you probably want to add a call to `cupy.cuda.Stream.ptds.synchronize()` in thread A before returning to ensure the output array is fully populated before consuming it in thread B. Fixes NVIDIA#7909. Authors: - Jim Crist-Harif (https://github.com/jcrist) Approvers: - Simon Adorf (https://github.com/csadorf) - Dante Gama Dessavre (https://github.com/dantegd) URL: NVIDIA#8086
This adds to the cuml.accel documentation and documents the "third party application" use-case. Authors: - Tim Head (https://github.com/betatim) Approvers: - Jim Crist-Harif (https://github.com/jcrist) URL: NVIDIA#8094
This: - Bumps our minimum supported `numpy` version to 1.26, to match that of `cudf`. Since `cudf` is a required dependency, we were effectively pinned at that already. - Adds `numpy` to our oldest deps test runs. This also effectively adds `cupy==13.6`, since `cupy==14` requires `numpy>=2.0`. Explicitly specifying `cupy==13.6` in an oldest-deps run is tricky since the pypi packages require cuda suffixes as well. I'm skipping that for now. - Fixes a few incompatibilities with numpy 1.x Authors: - Jim Crist-Harif (https://github.com/jcrist) Approvers: - Gil Forsyth (https://github.com/gforsyth) - Simon Adorf (https://github.com/csadorf) URL: NVIDIA#8118
Closes NVIDIA#8121 Proposes slightly reducing the threshold in the R-squared check for `MBSGDRegressor` tests. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Simon Adorf (https://github.com/csadorf) URL: NVIDIA#8122
jcrist
force-pushed
the
main-merge-release/26.06
branch
from
May 18, 2026 14:31
0b07bab to
5a20033
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@python/cuml/cuml_accel_tests/test_pipeline.py`:
- Around line 363-409: Both tests currently only call pipe.fit; add explicit
assertions that transformed outputs are numpy arrays to enforce the numpy-output
contract. In test_column_transfomer_in_pipeline_works call ct.fit_transform(X,
y) and/or pipe.named_steps["ct"].transform(X) and assert the result is an
instance of numpy.ndarray and has the expected shape, and also assert
pipe.predict(X) (or pipe.named_steps["ridge"].predict after fit) returns a
numpy.ndarray. Do the analogous checks in test_feature_union_in_pipeline_works
using union.fit_transform(X) and pipe.predict(X) (or union.transform and
ridge.predict), asserting numpy.ndarray types and appropriate shapes for both
intermediate transformer outputs and final pipeline predictions.
In `@python/cuml/cuml/accel/_patches/sklearn/pipeline.py`:
- Line 15: The exported names in the module's __all__ are not lexicographically
ordered; update the __all__ tuple (currently containing "Pipeline" and
"FeatureUnion") so entries are sorted alphabetically (e.g., place "FeatureUnion"
before "Pipeline") to satisfy the RUF022 lint rule.
🪄 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: b2d78a56-dced-4aee-a81c-2010416b9f05
📒 Files selected for processing (26)
conda/environments/all_cuda-129_arch-aarch64.yamlconda/environments/all_cuda-129_arch-x86_64.yamlconda/environments/all_cuda-132_arch-aarch64.yamlconda/environments/all_cuda-132_arch-x86_64.yamlconda/recipes/cuml/recipe.yamldependencies.yamldocs/source/cuml-accel/examples/index.rstdocs/source/cuml-accel/examples/third-party-apps.rstdocs/source/cuml-accel/index.rstpython/cuml/cuml/accel/_patches/sklearn/compose.pypython/cuml/cuml/accel/_patches/sklearn/pipeline.pypython/cuml/cuml/accel/core.pypython/cuml/cuml/internals/outputs.pypython/cuml/cuml/internals/validation.pypython/cuml/cuml/linear_model/linear_regression.pyxpython/cuml/cuml/linear_model/ridge.pyxpython/cuml/cuml/preprocessing/label.pypython/cuml/cuml_accel_tests/test_pipeline.pypython/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-examples.yamlpython/cuml/pyproject.tomlpython/cuml/tests/explainer/test_gpu_treeshap.pypython/cuml/tests/test_mbsgd_regressor.pypython/cuml/tests/test_prims.pypython/cuml/tests/test_reflection.pypython/cuml/tests/test_validation.pypython/libcuml/pyproject.toml
💤 Files with no reviewable changes (3)
- python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-examples.yaml
- python/cuml/tests/test_prims.py
- python/cuml/cuml/preprocessing/label.py
✅ Files skipped from review due to trivial changes (6)
- conda/environments/all_cuda-132_arch-x86_64.yaml
- conda/environments/all_cuda-132_arch-aarch64.yaml
- conda/environments/all_cuda-129_arch-x86_64.yaml
- conda/environments/all_cuda-129_arch-aarch64.yaml
- docs/source/cuml-accel/examples/index.rst
- python/cuml/cuml/accel/core.py
🚧 Files skipped from review as they are similar to previous changes (1)
- python/libcuml/pyproject.toml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.