Fix sample_weight handling in KernelRidge - #8040
Conversation
This fixes a bug introduced in the validation port to `KernelRidge`. This solver needs to special case scalar sample weights; the naive application of `check_inputs` here resulted in them being treated differently than before. This PR fixes the validation flow to treat scalar weights as they were before. It also adjusts the hypothesis test to hit this case in an example.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughKernelRidge.fit now conditionally validates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
There was a problem hiding this comment.
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/kernel_ridge/kernel_ridge.py`:
- Around line 307-314: The scalar fast-path for sample_weight bypasses
validation and allows invalid scalars (0, NaN, Inf) into _solve_cholesky_kernel;
change the cp.isscalar(sample_weight) branch in kernel_ridge.fit (around the
sample_weight handling) to validate scalars before skipping array checks: either
call check_sample_weight on a 1-element array-wrapped scalar or perform the same
checks manually (finite, non-negative, non-zero if required) and raise the same
errors as check_sample_weight, so that invalid scalar values are rejected
consistently with the non-scalar path and with scikit-learn behavior.
🪄 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: 93b3bf5d-e721-4ba5-aca5-5d0c0195630b
📒 Files selected for processing (2)
python/cuml/cuml/kernel_ridge/kernel_ridge.pypython/cuml/tests/test_kernel_ridge.py
This fixes a bug introduced in the validation port to
KernelRidge. This solver needs to special case scalar sample weights; the naive application ofcheck_inputshere resulted in them being treated differently than before.This PR fixes the validation flow to treat scalar weights as they were before. It also adjusts the hypothesis test to hit this case in an example.