Remove deprecated probability argument to SVC - #8223
Merged
Conversation
jcrist
force-pushed
the
remove-deprecated-probability
branch
from
June 3, 2026 16:55
b2e6871 to
3c364bb
Compare
jcrist
commented
Jun 3, 2026
jcrist
left a comment
jcrist
left a comment
Member
Author
There was a problem hiding this comment.
Annotating the diff for review.
I'll open a stacked PR to remove the parameter from LinearSVC. That one can also rip out some C++, so keeping it separate to ease review makes mores sense to me.
| assert model.n_iter_ == 10 | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("explicit_value", [True, False]) |
Member
Author
There was a problem hiding this comment.
These tests were in test_svm.py even though they tested LinearSVC. I've temporarily moved them here so I could be more certain I'd fixed all the warnings in SVC in this PR. In the follow-up removing probability from LinearSVC I'll rip these out.
| assert intersection_len > average_len / 8 | ||
|
|
||
|
|
||
| def compare_probabilistic_svm( |
|
|
||
| # rapids-pre-commit-hooks: disable-next-line | ||
| # TODO(26.08): Remove once `probability` is removed from cuml.svm.SVC. | ||
| # TODO: remove once we only support sklearn >= 1.9 |
Member
Author
There was a problem hiding this comment.
We do want to keep this test around to check plumbing for the fallback, the TODO to remove has been updated.
The filterwarnings below are to ignore warnings generated from sklearn on versions where they've deprecated probability.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRemoves SVC's deprecated probability support: constructor flag and internal state, _fit_proba, predict_proba/predict_log_proba. CPU↔GPU attribute export/import updated to omit probability arrays. Tests refactored to remove probability parametrization and add precomputed-kernel coverage. ChangesSVC probability removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/cuml/tests/test_svm.py (1)
891-912:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDon't bake the default multiclass precomputed path in as a known failure.
This test codifies that
SVC(kernel="precomputed")still breaks for multiclass under the defaultdecision_function_shape="ovo". Since the public API inpython/cuml/cuml/svm/svc.pystill advertiseskernel="precomputed"and still defaultsdecision_function_shapeto"ovo", users will hit this on the default path. Please either make the OvO precomputed flow work, or reject that combination up front with a targeted validation error instead of the current late "square" failure.🤖 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 `@python/cuml/tests/test_svm.py` around lines 891 - 912, The test reveals that SVC(kernel="precomputed") with the default decision_function_shape="ovo" results in a late "square" error; fix by adding an explicit validation in the SVC implementation (e.g., in SVC.__init__ or SVC.fit in python/cuml/cuml/svm/svc.py) that checks if kernel=="precomputed" and decision_function_shape=="ovo" and immediately raises a clear ValueError informing the user that OvO is not supported with precomputed kernels (or alternatively implement proper OvO handling for precomputed kernels), so callers like test_svc_precomputed_multiclass_ovo_fails and runtime users get an early, targeted error instead of the cryptic matrix "square" failure.
🤖 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.
Outside diff comments:
In `@python/cuml/tests/test_svm.py`:
- Around line 891-912: The test reveals that SVC(kernel="precomputed") with the
default decision_function_shape="ovo" results in a late "square" error; fix by
adding an explicit validation in the SVC implementation (e.g., in SVC.__init__
or SVC.fit in python/cuml/cuml/svm/svc.py) that checks if kernel=="precomputed"
and decision_function_shape=="ovo" and immediately raises a clear ValueError
informing the user that OvO is not supported with precomputed kernels (or
alternatively implement proper OvO handling for precomputed kernels), so callers
like test_svc_precomputed_multiclass_ovo_fails and runtime users get an early,
targeted error instead of the cryptic matrix "square" failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5f1a6f04-b3c0-4992-8034-10870ebac856
📒 Files selected for processing (10)
python/cuml/cuml/svm/svc.pypython/cuml/cuml/svm/svm_base.pyxpython/cuml/cuml/testing/utils.pypython/cuml/tests/explainer/test_explainer_kernel_shap.pypython/cuml/tests/explainer/test_explainer_permutation_shap.pypython/cuml/tests/test_base.pypython/cuml/tests/test_linear_svm.pypython/cuml/tests/test_pickle.pypython/cuml/tests/test_sklearn_import_export.pypython/cuml/tests/test_svm.py
💤 Files with no reviewable changes (1)
- python/cuml/cuml/testing/utils.py
Member
Author
|
/merge |
rapids-bot Bot
pushed a commit
that referenced
this pull request
Jun 4, 2026
This removes the deprecated `probability` argument to `LinearSVC`, along with all supporting code (python and C++). Currently stacked on #8223. Last part of #8220. Fixes #8220. Authors: - Jim Crist-Harif (https://github.com/jcrist) Approvers: - Dante Gama Dessavre (https://github.com/dantegd) URL: #8224
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
4 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.
This removes the deprecated
probabilityargument toSVC.Part of #8220.