Apply new input validation to metrics.regression - #8044
Merged
rapids-bot[bot] merged 5 commits intoMay 6, 2026
Merged
Conversation
|
Caution Review failedFailed to post review comments Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplace ad-hoc array conversion with shared validation helpers in regression metrics: ChangesRegression metric input validation + tests
scikit-learn xfail list tweak
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
python/cuml/tests/test_metrics.py (1)
790-805: ⚡ Quick winAdd a mixed
(n,)vs(n, 1)regression case to this error suiteThis block now checks length and sample-weight errors, but it doesn’t guard the single-output mixed-dimension normalization path. Adding that case here will prevent regressions in
_normalize_regression_metric_args.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cuml/tests/test_metrics.py` around lines 790 - 805, Add a test for the single-output mixed-dimension case in test_regression_metrics_errors: create a (n,1) target/prediction (e.g., arr_3_col = np.array([[1.0],[2.0],[3.0]])) and call cu_metric(arr_3, arr_3_col) and cu_metric(arr_3_col, arr_3) inside pytest.raises(ValueError) to ensure _normalize_regression_metric_args rejects (n,) vs (n,1) mismatches; reference the existing test_regression_metrics_errors, _REGRESSION_FUNCS and cu_metric when adding these assertions.
🤖 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/metrics/regression.py`:
- Around line 33-49: The shape-checking rejects valid cases like y_true.ndim==1
vs y_pred.shape==(n,1); instead, first normalize 1D targets by promoting y_true
and y_pred to column vectors (the reshape logic currently in the block using
y_true.reshape and y_pred.reshape) before performing the dimensionality/shape
consistency check, then run sample_weight = check_sample_weight(...) and
check_consistent_length(...) against the normalized y_true; update references to
y_true/y_pred in the initial validation to use the promoted versions so
functions like check_sample_weight and check_consistent_length operate on the
normalized arrays.
---
Nitpick comments:
In `@python/cuml/tests/test_metrics.py`:
- Around line 790-805: Add a test for the single-output mixed-dimension case in
test_regression_metrics_errors: create a (n,1) target/prediction (e.g.,
arr_3_col = np.array([[1.0],[2.0],[3.0]])) and call cu_metric(arr_3, arr_3_col)
and cu_metric(arr_3_col, arr_3) inside pytest.raises(ValueError) to ensure
_normalize_regression_metric_args rejects (n,) vs (n,1) mismatches; reference
the existing test_regression_metrics_errors, _REGRESSION_FUNCS and cu_metric
when adding these assertions.
🪄 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: 86eaec2a-7c37-4066-890c-6685893d0d97
📒 Files selected for processing (2)
python/cuml/cuml/metrics/regression.pypython/cuml/tests/test_metrics.py
csadorf
added
improvement
csadorf
force-pushed
the
apply-new-validation-to-metrics.regression
branch
from
May 4, 2026 22:02
2801d16 to
e934ce2
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/cuml/cuml/metrics/regression.py (1)
33-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize 1D targets before shape mismatch checks (mixed
(n,)/(n,1)should be accepted).Current ordering raises on valid single-output mixed shapes before normalization, which breaks sklearn-compatible behavior.
Proposed fix
- if y_true.ndim != y_pred.ndim or ( - y_true.ndim == 2 and y_true.shape[1] != y_pred.shape[1] - ): - raise ValueError( - f"y_true and y_pred have different shapes: " - f"{y_true.shape} vs {y_pred.shape}" - ) - - if ( - sample_weight := check_sample_weight(sample_weight, dtype=np.float64) - ) is not None: - check_consistent_length(y_true, sample_weight) - - # Promote 1D inputs to column vectors - if y_true.ndim == 1: - y_true = y_true.reshape((-1, 1)) - y_pred = y_pred.reshape((-1, 1)) + # Promote 1D inputs to column vectors + if y_true.ndim == 1: + y_true = y_true.reshape((-1, 1)) + if y_pred.ndim == 1: + y_pred = y_pred.reshape((-1, 1)) + + if y_true.ndim != y_pred.ndim or y_true.shape[1] != y_pred.shape[1]: + raise ValueError( + f"y_true and y_pred have different shapes: " + f"{y_true.shape} vs {y_pred.shape}" + ) + + if ( + sample_weight := check_sample_weight(sample_weight, dtype=np.float64) + ) is not None: + check_consistent_length(y_true, sample_weight)In current scikit-learn regression metrics input validation, are y_true.shape == (n_samples,) and y_pred.shape == (n_samples, 1) treated as valid by promoting 1D arrays to column vectors before shape comparison?As per coding guidelines: “Function and parameter names or defaults must match scikit-learn without justification; behavior for edge cases (empty arrays, single sample) must match scikit-learn.”
🤖 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/cuml/metrics/regression.py` around lines 33 - 49, The current validation raises on mixed single-output shapes because the 1D-to-column promotion occurs after the shape mismatch check; move the "Promote 1D inputs to column vectors" step so y_true and y_pred are reshaped to (-1,1) when ndim == 1 before any shape equality check, then run the existing shape validation (the block that raises ValueError) and the sample_weight handling (check_sample_weight and check_consistent_length) afterwards; update references in this function to ensure y_true, y_pred are normalized prior to comparing shapes so inputs like (n,) and (n,1) are accepted.
🤖 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.
Duplicate comments:
In `@python/cuml/cuml/metrics/regression.py`:
- Around line 33-49: The current validation raises on mixed single-output shapes
because the 1D-to-column promotion occurs after the shape mismatch check; move
the "Promote 1D inputs to column vectors" step so y_true and y_pred are reshaped
to (-1,1) when ndim == 1 before any shape equality check, then run the existing
shape validation (the block that raises ValueError) and the sample_weight
handling (check_sample_weight and check_consistent_length) afterwards; update
references in this function to ensure y_true, y_pred are normalized prior to
comparing shapes so inputs like (n,) and (n,1) are accepted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fcc90403-2bf0-4416-8e27-95be0a7bcefe
📒 Files selected for processing (2)
python/cuml/cuml/metrics/regression.pypython/cuml/tests/test_metrics.py
🚧 Files skipped from review as they are similar to previous changes (1)
- python/cuml/tests/test_metrics.py
csadorf
force-pushed
the
apply-new-validation-to-metrics.regression
branch
from
May 5, 2026 15:37
e934ce2 to
af0c584
Compare
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/tests/test_metrics.py`:
- Around line 786-787: Replace the brittle exact-equality asserts in
test_metrics that compare floating outputs (the two lines calling
cu_metric(y_true, y_pred, sample_weight=...) == unweighted) with tolerant float
comparisons: use a numeric-approx helper such as numpy.testing.assert_allclose
or pytest.approx to compare cu_metric(y_true, y_pred, sample_weight=...) to
unweighted with an appropriate tolerance. Update the two assertions that
reference cu_metric, y_true, y_pred, sample_weight and unweighted to use the
chosen tolerant assertion helper.
- Around line 778-827: Update the three tests to compare cuML outputs/errors
directly to scikit-learn: in test_regression_metrics_scalar_sample_weight, get
skl_metric = getattr(sklearn.metrics, func) and assert skl_metric(y_true,
y_pred) equals cu_metric(y_true, y_pred, sample_weight=1.0) and
sample_weight=2.5 (use exact equality or np.testing.assert_allclose as
appropriate); in test_regression_metrics_1d_2d_equivalence, compute expected =
skl_metric(y_true_1d, y_pred_1d) and assert got (from different shapes) matches
expected using np.testing.assert_allclose; in test_regression_metrics_errors,
call skl_metric on the same bad inputs to confirm it raises the same ValueError
patterns and use pytest.raises with similar match strings to assert parity
between cu_metric and skl_metric for inconsistent sample counts and non-1D
sample_weight. Ensure you import sklearn.metrics at top and reuse func to locate
corresponding skl function.
🪄 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: ddce1068-caf9-4040-ac25-e4c0497ce4a6
📒 Files selected for processing (3)
python/cuml/cuml/metrics/regression.pypython/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yamlpython/cuml/tests/test_metrics.py
💤 Files with no reviewable changes (1)
- python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- python/cuml/cuml/metrics/regression.py
csadorf
force-pushed
the
apply-new-validation-to-metrics.regression
branch
from
May 5, 2026 17:59
af0c584 to
163b214
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/cuml/tests/test_metrics.py (1)
787-788:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse tolerant float assertions for metric outputs.
At Line 787 and Line 788, exact
==on floating outputs can make this test flaky across backends/dtypes. Useassert_allclose(orassert_almost_equal) for both checks.Suggested patch
- assert cu_metric(y_true, y_pred, sample_weight=1.0) == unweighted - assert cu_metric(y_true, y_pred, sample_weight=2.5) == unweighted + np.testing.assert_allclose( + cu_metric(y_true, y_pred, sample_weight=1.0), unweighted + ) + np.testing.assert_allclose( + cu_metric(y_true, y_pred, sample_weight=2.5), unweighted + )🤖 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_metrics.py` around lines 787 - 788, Replace the exact equality checks on floating metric outputs with tolerant assertions: change the two assertions that call cu_metric(y_true, y_pred, sample_weight=1.0) and cu_metric(y_true, y_pred, sample_weight=2.5) to use a float-tolerant check (e.g., numpy.testing.assert_allclose or pytest.approx). Update the test to call assert_allclose(actual, unweighted, rtol=..., atol=...) for both sample_weight cases so small backend/dtype differences don't make the test flaky; keep references to the cu_metric calls and the same y_true/y_pred inputs.
🤖 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.
Duplicate comments:
In `@python/cuml/tests/test_metrics.py`:
- Around line 787-788: Replace the exact equality checks on floating metric
outputs with tolerant assertions: change the two assertions that call
cu_metric(y_true, y_pred, sample_weight=1.0) and cu_metric(y_true, y_pred,
sample_weight=2.5) to use a float-tolerant check (e.g.,
numpy.testing.assert_allclose or pytest.approx). Update the test to call
assert_allclose(actual, unweighted, rtol=..., atol=...) for both sample_weight
cases so small backend/dtype differences don't make the test flaky; keep
references to the cu_metric calls and the same y_true/y_pred inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b1fc170f-8931-435c-a7c6-9393c08d95dd
📒 Files selected for processing (2)
python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yamlpython/cuml/tests/test_metrics.py
💤 Files with no reviewable changes (1)
- python/cuml/cuml_accel_tests/upstream/scikit-learn/xfail-list.yaml
…regression metrics test_regression_metrics_scalar_sample_weight: Compare cuML outputs to sklearn for unweighted and sample_weight=1.0 using np.testing.assert_allclose (exact == fails due to float drift) Use 1D array for sklearn sample_weight since it rejects scalar input test_regression_metrics_1d_2d_equivalence: Use sklearn as expected reference instead of cuML(1d) to verify all shape permutations produce identical results test_regression_metrics_errors: Assert both cuML and sklearn raise the same ValueError for inconsistent sample counts and 2D sample_weight Narrow regex from 'inconsistent number of samples' to 'inconsistent' since sklearn uses 'inconsistent numbers of samples' (plural)
sklearn standardized the error messages for invalid sample_weight
inputs in 1.7. On 1.5.x and 1.6.x the five parameterizations of
test_regression_metrics_errors fail because the match= regexes
("Sample weights must be 1D" / "inconsistent") don't match the
older messages raised by numpy.average or column_or_1d.
Mark the test xfail with strict=True when sklearn < 1.7 so the
oldest-deps CI matrix (sklearn==1.5.0) reports XFAIL instead of
FAILED, while 1.7+ continues to run and pass.
csadorf
force-pushed
the
apply-new-validation-to-metrics.regression
branch
from
May 6, 2026 15:57
163b214 to
8cd284e
Compare
jcrist
approved these changes
May 6, 2026
| if ( | ||
| sample_weight := check_sample_weight(sample_weight, dtype=np.float64) | ||
| ) is not None: | ||
| check_consistent_length(y_true, sample_weight) |
Member
There was a problem hiding this comment.
Not a problem, but note that check_consistent_length works fine even if some of the inputs are None. So you can unconditionally call it on all input arrays, including optional ones.
Member
|
/merge |
This was referenced May 6, 2026
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
3 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.
Applies new input validation system to
metrics.regressionPart of #7998