Skip to content

A few sklearn compat test fixes - #8366

Merged
rapids-bot[bot] merged 9 commits into
NVIDIA:mainfrom
jcrist:a-few-compat-fixes
Jul 13, 2026
Merged

A few sklearn compat test fixes#8366
rapids-bot[bot] merged 9 commits into
NVIDIA:mainfrom
jcrist:a-few-compat-fixes

Conversation

@jcrist

@jcrist jcrist commented Jul 10, 2026

Copy link
Copy Markdown
Member

A few tiny sklearn compat test fixups.

  • Update xfail reasons to be correct for all currently xfailed tests
  • Fix min samples/features for PCA/TruncatedSVD/TSNE
  • Tag KNeighborsRegressor as multi-output
  • Small fix for StandardScaler logic

At this point most of our remaining xfails are statistical issues with sample weights (sklearn has these too for several estimators, getting these correct is an ongoing process).

@jcrist jcrist self-assigned this Jul 10, 2026
@jcrist
jcrist requested a review from a team as a code owner July 10, 2026 19:11
@jcrist
jcrist requested a review from csadorf July 10, 2026 19:11
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Jul 10, 2026
@jcrist jcrist added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cb42f04b-ce3d-4600-a6c8-fdc706c62528

📥 Commits

Reviewing files that changed from the base of the PR and between ddabacc and 5488e64.

📒 Files selected for processing (1)
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
💤 Files with no reviewable changes (1)
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened input validation for PCA, TruncatedSVD, and t-SNE (rejects datasets with fewer than two samples or features).
    • Improved StandardScaler partial-fit sample-count handling for per-feature tracking.
    • Updated KNeighborsRegressor metadata to correctly advertise multi-output support.
  • Tests
    • Refreshed sklearn compatibility/estimator-check expectations, including updated xfail reasons and adjusted coverage for several estimators.

Walkthrough

The changes update sklearn-compatible input validation, StandardScaler sample-count handling, KNeighborsRegressor metadata, and estimator compatibility expectations across PCA, TruncatedSVD, TSNE, UMAP, ensemble, linear, neighbor, and preprocessing checks.

Changes

Scikit-learn compatibility

Layer / File(s) Summary
Input validation and scaler state handling
python/cuml/cuml/_thirdparty/sklearn/preprocessing/_data.py, python/cuml/cuml/decomposition/pca.pyx, python/cuml/cuml/decomposition/tsvd.pyx, python/cuml/cuml/manifold/t_sne.pyx, python/cuml/tests/test_sklearn_compatibility.py
StandardScaler uses np.isscalar and np.full for per-feature sample counts. PCA, TruncatedSVD, and TSNE require at least two samples and two features; TSNE’s corresponding one-sample and one-feature xfails are removed.
Estimator metadata and upstream checks
python/cuml/cuml/neighbors/kneighbors_regressor.pyx, python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
KNeighborsRegressor reports multi-output support. Upstream xfails remove PCA, TruncatedSVD, and estimator-cloning entries while retaining RandomForest classifier and regressor checks.
Compatibility expectation updates
python/cuml/tests/test_sklearn_compatibility.py
Xfail reasons and expected checks are updated for sample-weight equivalence, dtype tolerance, KNeighborsRegressor multi-output behavior, UMAP, Lasso, ElasticNet, and StandardScaler.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • rapidsai/cuml#8166: Updates overlapping sklearn compatibility xfail expectations for estimators including RandomForest, KNeighbors, and TSNE.

Suggested labels: sklearn-api-compat

Suggested reviewers: csadorf

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is related, but it's generic and doesn't convey the main changes beyond broad sklearn compatibility fixes. Mention the concrete changes, such as PCA/TSVD/TSNE validation, KNeighborsRegressor multi-output tags, and StandardScaler/test xfail updates.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description matches the diff: it mentions xfail cleanup, validation fixes, KNeighborsRegressor tagging, and StandardScaler logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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/_thirdparty/sklearn/preprocessing/_data.py`:
- Around line 774-777: The scalar expansion in StandardScaler.partial_fit()
currently coerces fractional n_samples_seen_ values to int64. Preserve the
original scalar’s dtype when creating the broadcast array, and add an interop
regression test covering a migrated weighted StandardScaler resumed via
partial_fit().

In `@python/cuml/cuml/decomposition/pca.pyx`:
- Around line 482-483: PCA input validation currently rejects valid
single-feature datasets through ensure_min_features=2. Update the validation
configuration in the PCA fitting path to allow one feature, while retaining the
minimum sample requirement, so PCA(n_components=1) accepts shape (n_samples, 1)
input.

In `@python/cuml/cuml/decomposition/tsvd.pyx`:
- Around line 316-317: The TruncatedSVD validation currently rejects valid
one-row inputs via ensure_min_samples=2. In the TruncatedSVD input validation
logic, remove the sample minimum while retaining ensure_min_features=2, unless
the solver explicitly requires otherwise; add a regression test covering
fit_transform on a single-sample matrix.

In `@python/cuml/cuml/manifold/t_sne.pyx`:
- Around line 598-599: The TSNE input validation currently rejects valid
single-feature data via ensure_min_features=2. In the TSNE validation call near
ensure_min_samples, remove the ensure_min_features restriction so
one-dimensional inputs are accepted, and add or update coverage for a 1-feature
TSNE input.

In `@python/cuml/tests/test_sklearn_compatibility.py`:
- Around line 212-215: Restrict the KNeighborsRegressor exception for
check_regressor_multioutput to the float32-versus-float64 dtype assertion only,
rather than xfail-ing the entire check. Preserve or add an explicit assertion
for multi-output prediction shape so shape regressions cannot be hidden.
🪄 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: 7e0e4bc3-5a1e-4db8-8fc0-635272809379

📥 Commits

Reviewing files that changed from the base of the PR and between f19ca67 and ddabacc.

📒 Files selected for processing (7)
  • python/cuml/cuml/_thirdparty/sklearn/preprocessing/_data.py
  • python/cuml/cuml/decomposition/pca.pyx
  • python/cuml/cuml/decomposition/tsvd.pyx
  • python/cuml/cuml/manifold/t_sne.pyx
  • python/cuml/cuml/neighbors/kneighbors_regressor.pyx
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
  • python/cuml/tests/test_sklearn_compatibility.py
💤 Files with no reviewable changes (1)
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml

Comment thread python/cuml/cuml/_thirdparty/sklearn/preprocessing/_data.py
Comment thread python/cuml/cuml/decomposition/pca.pyx
Comment thread python/cuml/cuml/decomposition/tsvd.pyx
Comment thread python/cuml/cuml/manifold/t_sne.pyx
Comment thread python/cuml/tests/test_sklearn_compatibility.py

@csadorf csadorf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jcrist

jcrist commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 48d52fa into NVIDIA:main Jul 13, 2026
92 of 94 checks passed
@jcrist
jcrist deleted the a-few-compat-fixes branch July 14, 2026 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants