Repository: scikit-learn/scikit-learn
Pull request: FIX accept multilabel-indicator in _get_response_values
closes #26817 

Accept "multilabel-indicator" in `_get_response_values`.



Problem statement:
Scorer not working on ClassifierChain
### Describe the bug

ClassifeirChain fails when trying to make scoring predictions. The reason is: ClassifeirChain's attribute `classes_` is a list and not a np.array as the `_get_response_values` function expects. 

### Steps/Code to Reproduce

```python
from sklearn.multioutput import ClassifierChain
from sklearn.metrics._scorer import get_scorer
from sklearn.datasets import make_multilabel_classification
from sklearn.linear_model import LogisticRegression
from sklearn.model_selection import train_test_split
from sklearn.multioutput import ClassifierChain


X, Y = make_multilabel_classification(n_samples=72, n_classes=3, random_state=0)
X_train, X_test, Y_train, Y_test = train_test_split(X, Y, random_state=0)

base_lr = LogisticRegression(solver='lbfgs', random_state=0)
chain = ClassifierChain(base_lr, order='random', random_state=0)
y_pred = chain.fit(X_train, Y_train).predict(X_test)

print(get_scorer("average_precision")(chain, X_test, Y_test))
```

### Expected Results

No error thrown.

### Actual Results

```
  File "C:\Users\Mavs\Documents\Python\ATOM\test.py", line 204, in <module>
    print(get_scorer("average_precision")(chain, Y_test, y_pred))
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\metrics\_scorer.py", line 266, in __call__
    return self._score(partial(_cached_call, None), estimator, X, y_true, **_kwargs)
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\metrics\_scorer.py", line 466, in _score
    y_pred = method_caller(clf, "predict_proba", X, pos_label=pos_label)
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\metrics\_scorer.py", line 86, in _cached_call
    result, _ = _get_response_values(
  File "C:\Users\Mavs\Documents\Python\ATOM\venv310\lib\site-packages\sklearn\utils\_response.py", line 77, in _get_response_values
    if pos_label is not None and pos_label not in classes.tolist():
AttributeError: 'list' object has no attribute 'tolist'
```
### Versions

```shell
System:
    python: 3.10.2 (tags/v3.10.2:a58ebcc, Jan 17 2022, 14:12:15) [MSC v.1929 64 bit (AMD64)]
executable: C:\Users\Mavs\Documents\Python\ATOM\venv310\Scripts\python.exe
   machine: Windows-10-10.0.19045-SP0
Python dependencies:
      sklearn: 1.3.0
          pip: 23.1.2
   setuptools: 59.8.0
        numpy: 1.23.5
        scipy: 1.10.1
       Cython: 0.29.30
       pandas: 1.5.3
   matplotlib: 3.7.2
       joblib: 1.3.1
threadpoolctl: 3.1.0
Built with OpenMP: True
threadpoolctl info:
       user_api: blas
   internal_api: openblas
         prefix: libopenblas
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\numpy\.libs\libopenblas.FB5AE2TYXYH2IJRDKGDGQ3XBKLKTF43H.gfortran-win_amd64.dll
        version: 0.3.20
threading_layer: pthreads
   architecture: Zen
    num_threads: 16
       user_api: openmp
   internal_api: openmp
         prefix: vcomp
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\sklearn\.libs\vcomp140.dll
        version: None
    num_threads: 16
       user_api: blas
   internal_api: openblas
         prefix: libopenblas
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\scipy.libs\libopenblas-802f9ed1179cb9c9b03d67ff79f48187.dll
        version: 0.3.18
threading_layer: pthreads
   architecture: Zen
    num_threads: 16
       user_api: openmp
   internal_api: openmp
         prefix: libiomp
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\torch\lib\libiomp5md.dll
        version: None
    num_threads: 8
       user_api: openmp
   internal_api: openmp
         prefix: libiomp
       filepath: C:\Users\Mavs\Documents\Python\ATOM\venv310\Lib\site-packages\torch\lib\libiompstubs5md.dll
        version: None
    num_threads: 1
```

Hints from issue discussion:
Thanks for reporting this. I can reproduce the problem.

A slight complication is that `ClassifierChain.classes_` is defined as a list of the classes for each estimator in the chain. This is a bit different from what `classes_` should be IMHO. I think it makes sense to fix this and stick to the uniform interface for `classes_`. This might need some deprecation notice/warning. Maybe we need a new attribute that stores the individual classes seen by each estimator?

Would you be interested in submitting a Pull Request for this?
Hi @betatim ,

I would love to work on the proposed solution.
I am willing to give it a try as well, although I'll probably need some guidance about the deprecation/warning part.

I also noticed that `MultiOutputClassifier` experiences the same issue, and the docstring for the `classes_` attribute is wrong. It says `classes_: ndarray of shape (n_classes,)` but the code is the same as ClassifierChain.

https://github.com/scikit-learn/scikit-learn/blob/7f9bad99d6e0a3e8ddf92a7e5561245224dab102/sklearn/multioutput.py#L539

Is it possible for the base estimators to have different `classes_`? If not, it would make sense to let the attribute of the meta-estimator return the classes from the first base estimator.
> Is it possible for the base estimators to have different `classes_`? If not, it would make sense to let the attribute of the meta-estimator return the classes from the first base estimator.

It seems unlikely that they'd have different classes, but not impossible? I think the same data is passed to all estimators in the chain, but maybe if the estimator in the chain does some splitting of the data at random during its learning it would see different classes? I'm not sure if this is realistic or not. I think I'd go for the chain's `classes_` attribute being the union of all the individual estimator's `classes_`?

I'm not very familiar with the `ClassifierChain` methodology, so for me this would be a case where doing the research takes more time than making the change.

@tvdboom and @sqali regarding making a PR: maybe one of you can make a PR and the other can help review it?

ps. it might be worth making one PR for `ClassifierChain` and in it mentioning that `MultiOutputClassifier` has the same problem, but holding off with changing its code. Once there is consensus around the solution for `ClassifierChain` you can then add on the `MultiOutputClassifier` or make a second PR to deal with it.
Hi @betatim ,

Sure, I and @tvdboom would love to work on it together.

I will do my research and update it here.
So there are some puzzling stuff here.

First, it seems that the `ClassifierChain` accepts multiclass-multioutput while it should not. This is not the purpose of the original paper. The idea is to have a set of binary classifiers that can learn a label correlation. So I would think that we should run a `type_of_target` and reject if we are not in the `multilabel-indicator` case.

Secondly, for the classes, we expect each binary classifier to be trained on `[0, 1]`. The current version of the `classes_` is actually not informative at all. Making the union would already be more aligned with our own API but I don't think this is informative as well.

As a user, I would expect `classes_` to refer to the label's name which in the simple case would be `range(Y.shape[1])` (that corresponds as well to `np.unique(LabelBinarizer().inverse_transform(Y))`. In a world that `Y`, could be a dataframe, I would therefore expect `Y.columns`.

And I lost track of the original problem: `_get_response_values` is not written to handle multilabel classification. We can decide if we want to change the API of `classes_` for the multilabel but otherwise we should fix:

https://github.com/scikit-learn/scikit-learn/blob/fa3ca7ab9f4e1363c9f67a12652c362d53afb9cb/sklearn/utils/_response.py#L73-L75

If `estimator.classes_` is actually a list, then we can fork in a branch to handle this case. There is no problem with `pos_label` but I would need to check if there are any metrics supporting the output of `decision_function` or `predict_proba` in this case. `predict` should work like a charm just by returning directly the output.
Feel free to make the pr @sqali and add me to the reviewers
Hi @glemaitre @betatim ,

I can confirm that the estimator.classes_ is actually a list. I ran the below code and got the output.

```
X, Y = make_multilabel_classification(n_samples=72, n_classes=3, random_state=0)
X_train, X_test, Y_train, Y_test = train_test_split(X, Y, random_state=0)

base_lr = LogisticRegression(solver='lbfgs', random_state=0)
chain = ClassifierChain(base_lr, order='random', random_state=0)
y_pred = chain.fit(X_train, Y_train).predict(X_test)

print(chain.classes_)
print(type(chain.classes_))
```

Ouput:-
```
[array([0, 1]), array([0, 1]), array([0, 1])]
<class 'list'>
```

I am new to open source. I require some guidance here. I understand that I need to make changes in the multioutput.py file in the fit function of classes **MultiOutputClassifier** and **ClassifierChain** by introducing a new attribute to store the individual classes seen by each estimator. Kindly confirm if this would be the right approach.

If you are new to open-source @sqali it might be better to start with a [good first issue](https://github.com/scikit-learn/scikit-learn/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3A%22good+first+issue%22). Those are meant for newcomers compared to issues like this.

Commit message: Apply suggestions from code review

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Patch to review:
diff --git a/doc/whats_new/v1.3.rst b/doc/whats_new/v1.3.rst
index d486b177b0f2c..0a1778a3277b9 100644
--- a/doc/whats_new/v1.3.rst
+++ b/doc/whats_new/v1.3.rst
@@ -12,6 +12,13 @@ Version 1.3.1
 Changelog
 ---------
 
+:mod:`sklearn.metrics`
+.......................
+
+- |Fix| scorers used with :func:`metrics.get_scorer` handles properly
+  multilabel-indicator matrix.
+  :pr:`27002` by :user:`Guillaume Lemaitre <glemaitre>`.
+
 :mod:`sklearn.neighbors`
 ........................
 
diff --git a/sklearn/metrics/tests/test_score_objects.py b/sklearn/metrics/tests/test_score_objects.py
index 45bd271ad7644..dcfcd43341989 100644
--- a/sklearn/metrics/tests/test_score_objects.py
+++ b/sklearn/metrics/tests/test_score_objects.py
@@ -52,6 +52,7 @@
 )
 from sklearn.model_selection import GridSearchCV, cross_val_score, train_test_split
 from sklearn.multiclass import OneVsRestClassifier
+from sklearn.multioutput import ClassifierChain
 from sklearn.neighbors import KNeighborsClassifier
 from sklearn.pipeline import make_pipeline
 from sklearn.svm import LinearSVC
@@ -541,12 +542,15 @@ def test_thresholded_scorers_multilabel_indicator_data():
 
     # Multi-output multi-class decision_function
     # TODO Is there any yet?
-    clf = DecisionTreeClassifier()
-    clf.fit(X_train, y_train)
-    clf._predict_proba = clf.predict_proba
-    clf.predict_proba = None
-    clf.decision_function = lambda X: [p[:, 1] for p in clf._predict_proba(X)]
+    class TreeWithDecisionFunction(DecisionTreeClassifier):
+        # disable predict_proba
+        predict_proba = None
+
+        def decision_function(self, X):
+            return [p[:, 1] for p in DecisionTreeClassifier.predict_proba(self, X)]
 
+    clf = TreeWithDecisionFunction()
+    clf.fit(X_train, y_train)
     y_proba = clf.decision_function(X_test)
     score1 = get_scorer("roc_auc")(clf, X_test, y_test)
     score2 = roc_auc_score(y_test, np.vstack([p for p in y_proba]).T)
@@ -799,7 +803,19 @@ def test_multimetric_scorer_calls_method_once(
     assert decision_function_func.call_count == expected_decision_func_count
 
 
-def test_multimetric_scorer_calls_method_once_classifier_no_decision():
+@pytest.mark.parametrize(
+    "scorers",
+    [
+        (["roc_auc", "neg_log_loss"]),
+        (
+            {
+                "roc_auc": make_scorer(roc_auc_score, needs_threshold=True),
+                "neg_log_loss": make_scorer(log_loss, needs_proba=True),
+            }
+        ),
+    ],
+)
+def test_multimetric_scorer_calls_method_once_classifier_no_decision(scorers):
     predict_proba_call_cnt = 0
 
     class MockKNeighborsClassifier(KNeighborsClassifier):
@@ -814,7 +830,6 @@ def predict_proba(self, X):
     clf = MockKNeighborsClassifier(n_neighbors=1)
     clf.fit(X, y)
 
-    scorers = ["roc_auc", "neg_log_loss"]
     scorer_dict = _check_multimetric_scoring(clf, scorers)
     scorer = _MultimetricScorer(scorers=scorer_dict)
     scorer(clf, X, y)
@@ -837,7 +852,7 @@ def predict(self, X):
     clf = MockDecisionTreeRegressor()
     clf.fit(X, y)
 
-    scorers = {"neg_mse": "neg_mean_squared_error", "r2": "roc_auc"}
+    scorers = {"neg_mse": "neg_mean_squared_error", "r2": "r2"}
     scorer_dict = _check_multimetric_scoring(clf, scorers)
     scorer = _MultimetricScorer(scorers=scorer_dict)
     scorer(clf, X, y)
@@ -1356,3 +1371,20 @@ def score(y_true, y_pred, param=None):
             ValueError, match="is only supported if enable_metadata_routing=True"
         ):
             scorer(clf, X, y, param="blah")
+
+
+def test_get_scorer_multilabel_indicator():
+    """Check that our scorer deal with multi-label indicator matrices.
+
+    Non-regression test for:
+    https://github.com/scikit-learn/scikit-learn/issues/26817
+    """
+    X, Y = make_multilabel_classification(n_samples=72, n_classes=3, random_state=0)
+    X_train, X_test, Y_train, Y_test = train_test_split(X, Y, random_state=0)
+
+    base_lr = LogisticRegression(solver="lbfgs", random_state=0)
+    chain = ClassifierChain(base_lr, order="random", random_state=0)
+    chain.fit(X_train, Y_train)
+
+    score = get_scorer("average_precision")(chain, X_test, Y_test)
+    assert score > 0.8
diff --git a/sklearn/utils/_response.py b/sklearn/utils/_response.py
index e753ced045e1e..f078ab689cd96 100644
--- a/sklearn/utils/_response.py
+++ b/sklearn/utils/_response.py
@@ -5,6 +5,7 @@
 import numpy as np
 
 from ..base import is_classifier
+from .multiclass import type_of_target
 from .validation import _check_response_method, check_is_fitted
 
 
@@ -72,15 +73,15 @@ def _get_response_values(
     if is_classifier(estimator):
         prediction_method = _check_response_method(estimator, response_method)
         classes = estimator.classes_
-        target_type = "binary" if len(classes) <= 2 else "multiclass"
-
-        if pos_label is not None and pos_label not in classes.tolist():
-            raise ValueError(
-                f"pos_label={pos_label} is not a valid label: It should be "
-                f"one of {classes}"
-            )
-        elif pos_label is None and target_type == "binary":
-            pos_label = pos_label if pos_label is not None else classes[-1]
+        target_type = type_of_target(classes)
+        if target_type in ("binary", "multiclass"):
+            if pos_label is not None and pos_label not in classes.tolist():
+                raise ValueError(
+                    f"pos_label={pos_label} is not a valid label: It should be "
+                    f"one of {classes}"
+                )
+            elif pos_label is None and target_type == "binary":
+                pos_label = classes[-1]
 
         y_pred = prediction_method(X)
         if prediction_method.__name__ == "predict_proba":
@@ -94,10 +95,14 @@ def _get_response_values(
                         "classifier with two classes."
                     )
                     raise ValueError(err_msg)
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
+                y_pred = np.vstack([p[:, -1] for p in y_pred]).T
         elif prediction_method.__name__ == "decision_function":
             if target_type == "binary":
                 if pos_label == classes[0]:
                     y_pred *= -1
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
+                y_pred = np.vstack([p for p in y_pred]).T
     else:  # estimator is a regressor
         if response_method != "predict":
             raise ValueError(
diff --git a/sklearn/utils/tests/test_response.py b/sklearn/utils/tests/test_response.py
index a67346e5697ec..e715e94cc60b1 100644
--- a/sklearn/utils/tests/test_response.py
+++ b/sklearn/utils/tests/test_response.py
@@ -1,11 +1,17 @@
 import numpy as np
 import pytest
 
-from sklearn.datasets import load_iris, make_classification, make_regression
+from sklearn.datasets import (
+    load_iris,
+    make_classification,
+    make_multilabel_classification,
+    make_regression,
+)
 from sklearn.linear_model import (
     LinearRegression,
     LogisticRegression,
 )
+from sklearn.multioutput import ClassifierChain
 from sklearn.preprocessing import scale
 from sklearn.tree import DecisionTreeClassifier, DecisionTreeRegressor
 from sklearn.utils._mocking import _MockEstimatorOnOffPrediction
@@ -230,3 +236,26 @@ def test_get_response_values_multiclass(estimator, response_method):
     assert predictions.shape == (X.shape[0], len(estimator.classes_))
     if response_method == "predict_proba":
         assert np.logical_and(predictions >= 0, predictions <= 1).all()
+
+
+@pytest.mark.parametrize(
+    "response_method", ["predict_proba", "decision_function", "predict"]
+)
+def test_get_response_values_multilabel_indicator(response_method):
+    X, Y = make_multilabel_classification(random_state=0)
+    estimator = ClassifierChain(LogisticRegression()).fit(X, Y)
+
+    y_pred, pos_label = _get_response_values(
+        estimator, X, response_method=response_method
+    )
+    assert pos_label is None
+    assert y_pred.shape == Y.shape
+
+    if response_method == "predict_proba":
+        assert np.logical_and(y_pred >= 0, y_pred <= 1).all()
+    elif response_method == "decision_function":
+        # values returned by `decision_function` are not bounded in [0, 1]
+        assert (y_pred < 0).sum() > 0
+        assert (y_pred > 1).sum() > 0
+    else:  # response_method == "predict"
+        assert np.logical_or(y_pred == 0, y_pred == 1).all()


Code review comments:
diff_hunk: @@ -94,10 +95,14 @@ def _get_response_values(
                         "classifier with two classes."
                     )
                     raise ValueError(err_msg)
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
+                y_pred = np.vstack([p[:, -1] for p in y_pred]).T
         elif prediction_method.__name__ == "decision_function":
             if target_type == "binary":
                 if pos_label == classes[0]:
                     y_pred *= -1
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
+                y_pred = np.vstack([p for p in y_pred]).T
path: sklearn/utils/_response.py
text: @author:
In this case, this is the same story. However, I don't recall any estimator that would return a list currently.

diff_hunk: @@ -94,10 +95,14 @@ def _get_response_values(
                         "classifier with two classes."
                     )
                     raise ValueError(err_msg)
+            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
path: sklearn/utils/_response.py
text: @author:
```suggestion
            elif target_type == "multilabel-indicator" and isinstance(y_pred, list):
            # Use a compress format of shape `(n_samples, n_output)`.
            # Only `MLPClassifier` and `RidgeClassifier` already return
            # such output in a form of a NumPy array.
```