Repository: scikit-learn/scikit-learn
Pull request: ENH Raises error in hinge_loss when 'pred_decision' is invalid

<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/main/CONTRIBUTING.md
-->

#### Reference Issues/PRs
Fixes #19638 


#### What does this implement/fix? Explain your changes.
With multiclass target (more than 2 classes), the `pred_decision` argument must have a shape of type : 
`(n_sample, n_classes)` like written in the doc. 
If it is not the case, an arror is raised with a clear message. 

#### Any other comments?

A new test has been added in test_classification in order to check that situation. 

<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
[BUG] hinge_loss multi
```python
>>> sklearn.metrics.hinge_loss(y_true=[2,1,0,1,0,1,1], pred_decision=[0,1,2,1,0,2,1])

  2378         y_true = le.transform(y_true)
   2379         mask = np.ones_like(pred_decision, dtype=bool)
-> 2380         mask[np.arange(y_true.shape[0]), y_true] = False
   2381         margin = pred_decision[~mask]
   2382         margin -= np.max(pred_decision[mask].reshape(y_true.shape[0], -1),

IndexError: too many indices for array: array is 1-dimensional, but 2 were indexed
```

Hints from issue discussion:
In this case `y_true` has three classes, while the `pred_decision` is 1d which is for binary classification. We most likely want to error when the shape of `pred_decision` is not consistent with `y_true` (or `labels`).
Also `pred_decision` should be the output of `decision_function`. It looks like the output of `predict` in the snippet above
Could we not transform `[0,1,2,1,0,2,1]` in the right format : `[[0,0,1], [0,1,0], [0,0,1], [0,1,0], [2,0,0], [0,0,1], [0,1,0]]` inside the `_classification.hinge_loss` method ?

If we detect that `y_true` is not a binary classification and that `pred_decision` is 1d.



No, the hinge loss requires the real-valued decision function, not the predicted classes


```py
    In the multiclass case:

    >>> import numpy as np
    >>> X = np.array([[0], [1], [2], [3]])
    >>> Y = np.array([0, 1, 2, 3])
    >>> labels = np.array([0, 1, 2, 3])
    >>> est = svm.LinearSVC()
    >>> est.fit(X, Y)
    LinearSVC()
    >>> pred_decision = est.decision_function([[-1], [2], [3]])
    >>> y_true = [0, 2, 3]
    >>> hinge_loss(y_true, pred_decision, labels=labels)
    0.56...
```

where pred_decision is

```py
In [3]: pred_decision
Out[3]:
array([[ 1.27271904,  0.03418064, -0.68377391, -1.40170867],
       [-1.45454381, -0.58121055, -0.37602669, -0.17093709],
       [-2.36363142, -0.78634095, -0.27344428,  0.23932011]]
```
Ok, so I take and raise an Error if the shape of `pred_decision` is not consistent with `y_true` (or `labels`) like said @thomasjpfan.

Commit message: Add precision to error messages.

Patch to review:
diff --git a/doc/whats_new/v1.0.rst b/doc/whats_new/v1.0.rst
index 3e36438dda095..06764d2be6003 100644
--- a/doc/whats_new/v1.0.rst
+++ b/doc/whats_new/v1.0.rst
@@ -165,6 +165,11 @@ Changelog
   class methods and will be removed in 1.2.
   :pr:`18543` by `Guillaume Lemaitre`_.
 
+- |Enhancement| A fix to raise an error in :func:`metrics.hinge_loss` when
+  ``pred_decision`` is 1d whereas it is a multiclass classification or when
+  ``pred_decision`` parameter is not consistent with the ``labels`` parameter.
+  :pr:`19643` by :user:`Pierre Attard <PierreAttard>`.
+
 - |Feature| :func:`metrics.mean_pinball_loss` exposes the pinball loss for
   quantile regression. :pr:`19415` by :user:`Xavier Dupré <sdpython>`
   and :user:`Oliver Grisel <ogrisel>`.
diff --git a/sklearn/metrics/_classification.py b/sklearn/metrics/_classification.py
index 708bde662e765..97ee5a2e01340 100644
--- a/sklearn/metrics/_classification.py
+++ b/sklearn/metrics/_classification.py
@@ -2370,11 +2370,29 @@ def hinge_loss(y_true, pred_decision, *, labels=None, sample_weight=None):
     pred_decision = check_array(pred_decision, ensure_2d=False)
     y_true = column_or_1d(y_true)
     y_true_unique = np.unique(labels if labels is not None else y_true)
+
     if y_true_unique.size > 2:
-        if (labels is None and pred_decision.ndim > 1 and
-                (np.size(y_true_unique) != pred_decision.shape[1])):
-            raise ValueError("Please include all labels in y_true "
-                             "or pass labels as third argument")
+
+        if pred_decision.ndim <= 1:
+            raise ValueError("The shape of pred_decision cannot be 1d array"
+                             "with a multiclass target. pred_decision shape "
+                             "must be (n_samples, n_classes), that is "
+                             f"({y_true.shape[0]}, {y_true_unique.size})."
+                             f" Got: {pred_decision.shape}")
+
+        # pred_decision.ndim > 1 is true
+        if y_true_unique.size != pred_decision.shape[1]:
+            if labels is None:
+                raise ValueError("Please include all labels in y_true "
+                                 "or pass labels as third argument")
+            else:
+                raise ValueError("The shape of pred_decision is not "
+                                 "consistent with the number of classes. "
+                                 "With a multiclass target, pred_decision "
+                                 "shape must be "
+                                 "(n_samples, n_classes), that is "
+                                 f"({y_true.shape[0]}, {y_true_unique.size}). "
+                                 f"Got: {pred_decision.shape}")
         if labels is None:
             labels = y_true_unique
         le = LabelEncoder()
diff --git a/sklearn/metrics/tests/test_classification.py b/sklearn/metrics/tests/test_classification.py
index c32e9c89ada47..2925bcdc6bad4 100644
--- a/sklearn/metrics/tests/test_classification.py
+++ b/sklearn/metrics/tests/test_classification.py
@@ -4,6 +4,7 @@
 from itertools import chain
 from itertools import permutations
 import warnings
+import re
 
 import numpy as np
 from scipy import linalg
@@ -2135,6 +2136,33 @@ def test_hinge_loss_multiclass_missing_labels_with_labels_none():
         hinge_loss(y_true, pred_decision)
 
 
+def test_hinge_loss_multiclass_no_consistent_pred_decision_shape():
+    # test for inconsistency between multiclass problem and pred_decision
+    # argument
+    y_true = np.array([2, 1, 0, 1, 0, 1, 1])
+    pred_decision = np.array([0, 1, 2, 1, 0, 2, 1])
+    error_message = ("The shape of pred_decision cannot be 1d array"
+                     "with a multiclass target. pred_decision shape "
+                     "must be (n_samples, n_classes), that is "
+                     f"({y_true.shape[0]}, {np.unique(y_true).size}). "
+                     f"Got: ({y_true.shape[0]},)")
+    with pytest.raises(ValueError, match=re.escape(error_message)):
+        hinge_loss(y_true=y_true, pred_decision=pred_decision)
+
+    # test for inconsistency between pred_decision shape and labels number
+    pred_decision = np.array([[0, 1], [0, 1], [0, 1], [0, 1],
+                              [2, 0], [0, 1], [1, 0]])
+    labels = [0, 1, 2]
+    error_message = ("The shape of pred_decision is not "
+                     "consistent with the number of classes. "
+                     "With a multiclass target, pred_decision "
+                     "shape must be (n_samples, n_classes), that is "
+                     f"({y_true.shape[0]}, {np.unique(y_true).size}). "
+                     f"Got: ({y_true.shape[0]}, {pred_decision.shape[1]})")
+    with pytest.raises(ValueError, match=re.escape(error_message)):
+        hinge_loss(y_true=y_true, pred_decision=pred_decision, labels=labels)
+
+
 def test_hinge_loss_multiclass_with_missing_labels():
     pred_decision = np.array([
         [+0.36, -0.17, -0.58, -0.99],


Code review comments:
diff_hunk: @@ -2135,6 +2136,33 @@ def test_hinge_loss_multiclass_missing_labels_with_labels_none():
         hinge_loss(y_true, pred_decision)
 
 
+def test_hinge_loss_multiclass_no_consistent_pred_decision_shape():
+    # test for inconsistency between multiclass problem and pred_decision
+    # argument
+    y_true = np.array([2, 1, 0, 1, 0, 1, 1])
+    pred_decision = np.array([0, 1, 2, 1, 0, 2, 1])
+    error_message = ("The shape of pred_decision cannot be 1d array"
+                     "with a multiclass target. pred_decision shape "
+                     "must be (n_samples, n_classes), that is "
+                     f"({y_true.shape[0]}, {np.unique(y_true).size}). "
+                     f"Got: ({y_true.shape[0]},)")
path: sklearn/metrics/tests/test_classification.py
text: @user1:
The following would make more sense:

```suggestion
                     f"Got: {pred_decision.shape}")
```

Alternatively, just hard code the numbers in the test: " that is (7, 3). Got: (7,).

diff_hunk: @@ -2135,6 +2136,33 @@ def test_hinge_loss_multiclass_missing_labels_with_labels_none():
         hinge_loss(y_true, pred_decision)
 
 
+def test_hinge_loss_multiclass_no_consistent_pred_decision_shape():
+    # test for inconsistency between multiclass problem and pred_decision
+    # argument
+    y_true = np.array([2, 1, 0, 1, 0, 1, 1])
+    pred_decision = np.array([0, 1, 2, 1, 0, 2, 1])
+    error_message = ("The shape of pred_decision cannot be 1d array"
+                     "with a multiclass target. pred_decision shape "
+                     "must be (n_samples, n_classes), that is "
+                     f"({y_true.shape[0]}, {np.unique(y_true).size}). "
+                     f"Got: ({y_true.shape[0]},)")
+    with pytest.raises(ValueError, match=re.escape(error_message)):
+        hinge_loss(y_true=y_true, pred_decision=pred_decision)
+
+    # test for inconsistency between pred_decision shape and labels number
+    pred_decision = np.array([[0, 1], [0, 1], [0, 1], [0, 1],
+                              [2, 0], [0, 1], [1, 0]])
+    labels = [0, 1, 2]
+    error_message = ("The shape of pred_decision is not "
+                     "consistent with the number of classes. "
+                     "With a multiclass target, pred_decision "
+                     "shape must be (n_samples, n_classes), that is "
+                     f"({y_true.shape[0]}, {np.unique(y_true).size}). "
+                     f"Got: ({y_true.shape[0]}, {pred_decision.shape[1]})")
path: sklearn/metrics/tests/test_classification.py
text: @user1:
Same remark, just hard-code the expected and observed shapes.