Skip to content

Apply new validation to cuml.covariance - #8008

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
jcrist:validation-covariance
Apr 29, 2026
Merged

Apply new validation to cuml.covariance#8008
rapids-bot[bot] merged 2 commits into
NVIDIA:mainfrom
jcrist:validation-covariance

Conversation

@jcrist

@jcrist jcrist commented Apr 23, 2026

Copy link
Copy Markdown
Member

Fixes #7988.

@jcrist jcrist self-assigned this Apr 23, 2026
@jcrist
jcrist requested a review from a team as a code owner April 23, 2026 14:05
@jcrist
jcrist requested a review from betatim April 23, 2026 14:05
@jcrist jcrist added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change cuml-accel Issues related to cuml.accel sklearn-api-compat Issues around cuml matching sklearn API conventions/standards labels Apr 23, 2026
@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Apr 23, 2026
@jcrist
jcrist force-pushed the validation-covariance branch from 7b296fc to 9ed5a67 Compare April 23, 2026 14:10
@coderabbitai

coderabbitai Bot commented Apr 23, 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: Enterprise

Run ID: bfc50093-e10c-4263-9395-ea93044d9dfa

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed5a67 and 6c928d4.

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

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • LedoitWolf: unified input validation/casting, better handling of object-dtype and NaN/Inf inputs, single-sample fit now warns instead of error, and statistical terms correctly scale with feature count.
  • Tests

    • Removed expected-failure markers for LedoitWolf compatibility checks so those scenarios are now evaluated normally.

Walkthrough

Replaces per-method input conversion/validation in LedoitWolf with unified check_inputs casting to float32/float64; adjusts feature-count usage and single-sample warning behavior; converts stored arrays via .to_output("cupy"); changes @reflect(reset=...) to reset="type". Removes several LedoitWolf xfail entries from sklearn-compat configs.

Changes

Cohort / File(s) Summary
LedoitWolf estimator
python/cuml/cuml/covariance/ledoit_wolf.py
Replaced input_to_cupy_array + check_features with check_inputs across fit, score, error_norm, mahalanobis, and get_precision. fit now warns for single-sample inputs (no hard error), uses X.shape for feature counts and dtype, updates covariance diagonal indexing accordingly, and converts stored arrays using .to_output("cupy"). Changed @reflect reset from True to "type".
sklearn xfail mappings & tests
python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml, python/cuml/tests/test_sklearn_compatibility.py
Removed xfail entries that previously exempted LedoitWolf from checks for object-dtype handling, empty-data error messages, and NaN/Inf validation so those estimator checks will run normally.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Apply new validation to cuml.covariance' accurately summarizes the main change: updating input validation in the covariance module.
Description check ✅ Passed The description 'Fixes #7988' is minimal but directly related to the changeset, referencing the linked issue about updating covariance validation.
Linked Issues check ✅ Passed The PR implements validation updates to cuml.covariance as required by issue #7988, including check_inputs-based validation, input conversion refactoring, and removal of obsolete xfail entries.
Out of Scope Changes check ✅ Passed All changes are scoped to updating input validation in the LedoitWolf estimator and removing corresponding xfail entries; no unrelated modifications are present.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% 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

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

@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: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@python/cuml/cuml/covariance/ledoit_wolf.py`:
- Around line 255-259: The warnings.warn call inside the X.shape[0] == 1 branch
should include stacklevel=2 so the warning points at the user's caller; update
the warnings.warn(...) invocation in ledoit_wolf.py (the block that checks if
X.shape[0] == 1) to pass stacklevel=2 as an argument to warnings.warn to improve
the warning stack trace.
🪄 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: Pro Plus

Run ID: 1ce11a2e-df76-4657-af87-6f7b50d73e9a

📥 Commits

Reviewing files that changed from the base of the PR and between 42f57b5 and 7b296fc.

📒 Files selected for processing (3)
  • python/cuml/cuml/covariance/ledoit_wolf.py
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
  • python/cuml/tests/test_sklearn_compatibility.py
💤 Files with no reviewable changes (2)
  • python/cuml/tests/test_sklearn_compatibility.py
  • python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml

Comment thread python/cuml/cuml/covariance/ledoit_wolf.py
@jcrist

jcrist commented Apr 29, 2026

Copy link
Copy Markdown
Member Author

/merge

@rapids-bot
rapids-bot Bot merged commit 51af41c into NVIDIA:main Apr 29, 2026
171 of 174 checks passed
@jcrist
jcrist deleted the validation-covariance branch April 29, 2026 20:52
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 sklearn-api-compat Issues around cuml matching sklearn API conventions/standards

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update cuml.covariance to new input validation

4 participants