Skip to content

Fix hinge loss decision-function shape handling - #8245

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
csadorf:fix/hinge-loss-decision-shape
Jun 9, 2026
Merged

Fix hinge loss decision-function shape handling#8245
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
csadorf:fix/hinge-loss-decision-shape

Conversation

@csadorf

@csadorf csadorf commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Remove the stale xfail from test_hinge_loss and pass LogisticRegression.decision_function output to hinge_loss without transposing it. decision_function already returns sklearn-shaped scores, so the transpose was turning multiclass scores into (n_classes, n_samples) and causing the current failure.

Closes #3569

Validation:
pytest -q python/cuml/tests/test_metrics.py -k "test_hinge_loss" -rxX

@csadorf csadorf added bug Something isn't working non-breaking Non-breaking change labels Jun 9, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 9, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the Cython / Python Cython or Python issue label Jun 9, 2026
@csadorf

This comment has been minimized.

@csadorf
csadorf marked this pull request as ready for review June 9, 2026 17:48
@csadorf
csadorf requested a review from a team as a code owner June 9, 2026 17:48
@csadorf
csadorf requested a review from betatim June 9, 2026 17:48
@coderabbitai

coderabbitai Bot commented Jun 9, 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: a758d35a-1a10-445c-a2cc-b34032cdcc3d

📥 Commits

Reviewing files that changed from the base of the PR and between 851121b and 159cc63.

📒 Files selected for processing (1)
  • python/cuml/tests/test_metrics.py

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Fixed hinge loss computation issue that was previously causing test failures.

Walkthrough

The test_hinge_loss test in python/cuml/tests/test_metrics.py is updated to remove the xfail marker and correct the handling of the decision function output. Both the cuML and scikit-learn hinge-loss computations now use the decision function directly without transposing.

Changes

Hinge Loss Test

Layer / File(s) Summary
Test xfail removal and decision function transpose fix
python/cuml/tests/test_metrics.py
The test_hinge_loss decorator removes @pytest.mark.xfail, and both cuML's cuml_hinge call and scikit-learn's sk_hinge reference computation are updated to use cu_predict_decision directly instead of the transposed cu_predict_decision.T.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • rapidsai/cuml#8060: Implements the hinge-loss decision function orientation changes that this test update validates by removing the transpose operation.

Suggested labels

improvement

Suggested reviewers

  • dantegd
  • jcrist
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing hinge loss decision-function shape handling by removing transpose operations.
Description check ✅ Passed The description clearly explains the fix: removing stale xfail, eliminating unnecessary transpose of decision_function output, and references the closed issue #3569.
Linked Issues check ✅ Passed The PR directly addresses issue #3569 by fixing the root cause of the test failure (incorrect transpose of decision_function output) and removing the xfail marker, allowing the test to pass.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the hinge loss test by removing the xfail decorator and correcting the decision_function shape handling; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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.

@jcrist jcrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@jcrist

jcrist commented Jun 9, 2026

Copy link
Copy Markdown
Member

/merge

@rapids-bot
rapids-bot Bot merged commit 7e086f7 into NVIDIA:main Jun 9, 2026
103 checks passed
@csadorf
csadorf deleted the fix/hinge-loss-decision-shape branch June 9, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Cython / Python Cython or Python issue non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] CI failures on test_hinge_loss

4 participants