Repository: scikit-learn/scikit-learn
Pull request: [MRG] API Replace scorer brier_score_loss with neg_brier_score
Reference Issues/PRs
Fixes #13887

What does this implement/fix? Explain your changes.
Introduced new scorer neg_brier_score to reflect the real behavior of the scorer
Added deprecation of brier_loss_score (not sure about the versions mentioned in the deprecation message 😕)
Updated documentation to reflect changes

Any other comments?
cloned from #14123

Problem statement:
Documentation section 3.3.1.1 has incorrect description of brier_score_loss
In the documentation, section 3.3.1.1. "Common cases: predefined values" includes the remark

> All scorer objects follow the convention that higher return values are better than lower return values. 

As far as I can tell, this is true for all of the listed metrics, **except** the `brier_score_loss`. In the case of `brier_score_loss`, a _lower loss value is better._ This is because `brier_score_loss` measures the mean-square difference between a predicted probability and a categorical outcome; the Brier score is _minimized_ at 0.0 because all summands are either `(0 - 0) ^ 2=0` or `(1 -1) ^ 2=0` when the model is making perfect predictions. On the other hand, the Brier score is _maximized_ at 1.0 when all predictions are **opposite** the correct label, as all summands are either `(0 - 1)^2=1` or `(1 - 0)^2=1`.

Therefore, the definition of the `brier_score_loss` is not consistent with the quotation from section 3.3.1.1. 

I suggest making 2 changes to relieve this confusion.

1. Implement a function `neg_brier_score_loss` which simply negates the value of `brier_score_loss`; this is a direct analogy to what is done in the case of `neg_log_loss`. A better model has a lower value of log-loss (categorical cross-entropy loss), therefore a larger value of the _negative_ log-loss implies a better model. Naturally, the same is true for Brier score, where it is also the case that a better model is assigned a lower loss.

2. Remove reference to `brier_score_loss` from section 3.3.1.1. Brier score is useful in lots of ways; however, because it does not have the property that a larger value implies a better model, it seems confusing to mention it in the context of section 3.3.1.1. References to `brier_score_loss` can be replaced with `neg_brier_score_loss`, which has the property that better models have large values, just like accuracy, ROC AUC and the rest of the listed metrics.

Hints from issue discussion:
Indeed this is probably the right course of action. Please feel free to open a PR if your wish.
@Sycor4x  I'll gladly work on it if you're not already doing it 
@qdeffense Thank you. I had planned to start these revisions if this suggestion were well-received; however, I've just come down with a cold and won't be able to write coherent code at the moment. If you want to take a stab at this, I support your diligence. 

It occurred to me after I wrote this that it is possible for the verbal description in 3.3.1.1 to be incorrect while the _behavior_ of the scorer objects called via the strings in 3.3.1.1 might work correctly in the sense that internally, `brier_score_loss` behaves in the same manner as `neg_log_loss` and therefore is consistent with the statement

> All scorer objects follow the convention that higher return values are better than lower return values.

If this is the case, then the _documentation_ is the only thing that needs to be tweaked: just make it explicit that some kind of reversal is applied to `brier_score_loss` such that the block quote is true.

I haven't been able to check -- I'm basically incapacitated right now.
No, we should be using `neg_brier_loss` or something. We made a similar change for other losses in https://github.com/scikit-learn/scikit-learn/compare/7e079c0fd2%5E...2ba347841.
Apologies the mistake was introduced by me.
I agree that we should introduce neg_brier_score_loss and deprecate brier_score_loss.
Hi! I would want to start working on this.
Thanks @stefan-matcovici 
I'll work on this.
@nityamd I think @stefan-matcovici is working on that already in #14123.

Commit message: remove valid_scorers method

Patch to review:
diff --git a/doc/modules/model_evaluation.rst b/doc/modules/model_evaluation.rst
index 0a22488a1dbb4..a487756036bdd 100644
--- a/doc/modules/model_evaluation.rst
+++ b/doc/modules/model_evaluation.rst
@@ -61,7 +61,7 @@ Scoring                           Function
 'accuracy'                        :func:`metrics.accuracy_score`
 'balanced_accuracy'               :func:`metrics.balanced_accuracy_score`
 'average_precision'               :func:`metrics.average_precision_score`
-'brier_score_loss'                :func:`metrics.brier_score_loss`
+'neg_brier_score_loss'            :func:`metrics.brier_score_loss`
 'f1'                              :func:`metrics.f1_score`                          for binary targets
 'f1_micro'                        :func:`metrics.f1_score`                          micro-averaged
 'f1_macro'                        :func:`metrics.f1_score`                          macro-averaged
diff --git a/doc/whats_new/v0.22.rst b/doc/whats_new/v0.22.rst
index 1c31128b4f7e4..f844162e080ac 100644
--- a/doc/whats_new/v0.22.rst
+++ b/doc/whats_new/v0.22.rst
@@ -460,6 +460,13 @@ Changelog
   with a target matrix `Y` in which the first column was constant.
   :issue:`13609` by :user:`Camila Williamson <camilaagw>`.
 
+:mod:`sklearn.metrics`
+..................................
+
+- |API| scoring="neg_brier_score_loss" should be used instead of
+  scoring="brier_score_loss" which is now deprecated.
+  :pr:`14898` by :user:`Stefan Matcovici <stefan-matcovici>`.
+
 Miscellaneous
 .............
 
diff --git a/sklearn/metrics/classification.py b/sklearn/metrics/classification.py
index 7456e0f3e4ed5..6f2224c1dfdb1 100644
--- a/sklearn/metrics/classification.py
+++ b/sklearn/metrics/classification.py
@@ -2369,7 +2369,7 @@ def brier_score_loss(y_true, y_prob, sample_weight=None, pos_label=None):
         raise ValueError("y_prob contains values less than 0.")
 
     # if pos_label=None, when y_true is in {-1, 1} or {0, 1},
-    # pos_labe is set to 1 (consistent with precision_recall_curve/roc_curve),
+    # pos_label is set to 1 (consistent with precision_recall_curve/roc_curve),
     # otherwise pos_label is set to the greater label
     # (different from precision_recall_curve/roc_curve,
     # the purpose is to keep backward compatibility).
diff --git a/sklearn/metrics/scorer.py b/sklearn/metrics/scorer.py
index 1ab0ef077458f..76111e986b868 100644
--- a/sklearn/metrics/scorer.py
+++ b/sklearn/metrics/scorer.py
@@ -18,8 +18,9 @@
 #          Arnaud Joly <arnaud.v.joly@gmail.com>
 # License: Simplified BSD
 
-from abc import ABCMeta
+from abc import ABCMeta, abstractmethod
 from collections.abc import Iterable
+import warnings
 
 import numpy as np
 
@@ -49,6 +50,16 @@ def __init__(self, score_func, sign, kwargs):
         self._kwargs = kwargs
         self._score_func = score_func
         self._sign = sign
+        # XXX After removing the deprecated scorers (v0.24) remove the
+        # XXX deprecation_msg property again and remove __call__'s body again
+        self._deprecation_msg = None
+
+    @abstractmethod
+    def __call__(self, estimator, X, y, sample_weight=None):
+        if self._deprecation_msg is not None:
+            warnings.warn(self._deprecation_msg,
+                          category=DeprecationWarning,
+                          stacklevel=2)
 
     def __repr__(self):
         kwargs_string = "".join([", %s=%s" % (str(k), str(v))
@@ -88,6 +99,9 @@ def __call__(self, estimator, X, y_true, sample_weight=None):
             Score function applied to prediction of estimator on X.
         """
 
+        super(_PredictScorer, self).__call__(estimator, X, y_true,
+                                             sample_weight=sample_weight)
+
         y_pred = estimator.predict(X)
         if sample_weight is not None:
             return self._sign * self._score_func(y_true, y_pred,
@@ -123,6 +137,10 @@ def __call__(self, clf, X, y, sample_weight=None):
         score : float
             Score function applied to prediction of estimator on X.
         """
+
+        super(_ProbaScorer, self).__call__(clf, X, y,
+                                           sample_weight=sample_weight)
+
         y_type = type_of_target(y)
         y_pred = clf.predict_proba(X)
         if y_type == "binary":
@@ -171,6 +189,10 @@ def __call__(self, clf, X, y, sample_weight=None):
         score : float
             Score function applied to prediction of estimator on X.
         """
+
+        super(_ThresholdScorer, self).__call__(clf, X, y,
+                                               sample_weight=sample_weight)
+
         y_type = type_of_target(y)
         if y_type not in ("binary", "multilabel-indicator"):
             raise ValueError("{0} format is not supported".format(y_type))
@@ -226,11 +248,14 @@ def get_scorer(scoring):
     """
     if isinstance(scoring, str):
         try:
-            scorer = SCORERS[scoring]
+            if scoring == 'brier_score_loss':
+                scorer = brier_score_loss_scorer
+            else:
+                scorer = SCORERS[scoring]
         except KeyError:
             raise ValueError('%r is not a valid scoring value. '
                              'Use sorted(sklearn.metrics.SCORERS.keys()) '
-                             'to get valid options.' % (scoring))
+                             'to get valid options.' % scoring)
     else:
         scorer = scoring
     return scorer
@@ -529,9 +554,16 @@ def make_scorer(score_func, greater_is_better=True, needs_proba=False,
 # Score function for probabilistic classification
 neg_log_loss_scorer = make_scorer(log_loss, greater_is_better=False,
                                   needs_proba=True)
+neg_brier_score_loss_scorer = make_scorer(brier_score_loss,
+                                          greater_is_better=False,
+                                          needs_proba=True)
 brier_score_loss_scorer = make_scorer(brier_score_loss,
                                       greater_is_better=False,
                                       needs_proba=True)
+deprecation_msg = ('Scoring method brier_score_loss was renamed to '
+                   'neg_brier_score_loss in version 0.22 and will '
+                   'be removed in 0.24.')
+brier_score_loss_scorer._deprecation_msg = deprecation_msg
 
 
 # Clustering scores
@@ -563,7 +595,7 @@ def make_scorer(score_func, greater_is_better=True, needs_proba=False,
                balanced_accuracy=balanced_accuracy_scorer,
                average_precision=average_precision_scorer,
                neg_log_loss=neg_log_loss_scorer,
-               brier_score_loss=brier_score_loss_scorer,
+               neg_brier_score_loss=neg_brier_score_loss_scorer,
                # Cluster metrics that use supervised evaluation
                adjusted_rand_score=adjusted_rand_scorer,
                homogeneity_score=homogeneity_scorer,
diff --git a/sklearn/metrics/tests/test_score_objects.py b/sklearn/metrics/tests/test_score_objects.py
index 120bae5275281..cde83f8d8ae84 100644
--- a/sklearn/metrics/tests/test_score_objects.py
+++ b/sklearn/metrics/tests/test_score_objects.py
@@ -51,7 +51,7 @@
                'roc_auc', 'average_precision', 'precision',
                'precision_weighted', 'precision_macro', 'precision_micro',
                'recall', 'recall_weighted', 'recall_macro', 'recall_micro',
-               'neg_log_loss', 'log_loss', 'brier_score_loss',
+               'neg_log_loss', 'log_loss', 'neg_brier_score_loss',
                'jaccard', 'jaccard_weighted', 'jaccard_macro',
                'jaccard_micro', 'roc_auc_ovr', 'roc_auc_ovo',
                'roc_auc_ovr_weighted', 'roc_auc_ovo_weighted']
@@ -546,3 +546,14 @@ def test_scoring_is_not_metric():
         check_scoring(Ridge(), r2_score)
     with pytest.raises(ValueError, match='make_scorer'):
         check_scoring(KMeans(), cluster_module.adjusted_rand_score)
+
+
+def test_deprecated_scorer():
+    X, y = make_blobs(random_state=0, centers=2)
+    X_train, X_test, y_train, y_test = train_test_split(X, y, random_state=0)
+    clf = DecisionTreeClassifier()
+    clf.fit(X_train, y_train)
+
+    deprecated_scorer = get_scorer('brier_score_loss')
+    with pytest.warns(DeprecationWarning):
+        deprecated_scorer(clf, X_test, y_test)


Code review comments:
diff_hunk: @@ -61,7 +61,7 @@ Scoring                           Function
 'accuracy'                        :func:`metrics.accuracy_score`
 'balanced_accuracy'               :func:`metrics.balanced_accuracy_score`
 'average_precision'               :func:`metrics.average_precision_score`
-'brier_score_loss'                :func:`metrics.brier_score_loss`
+'neg_brier_score_loss'            :func:`metrics.brier_score_loss`
path: doc/modules/model_evaluation.rst
text: @user1:
I think 'neg_brier_score` might be a more friendly name. neg already implies that the score would otherwise decrease with improvement, and "Brier score loss" is not something known in the literature: it appears 70 times on Google not associated with scikit-learn. 

@user2:
> I think 'neg_brier_score` might be a more friendly name. neg already implies that the score would otherwise decrease with improvement,

Maybe neg_brier_score_loss is a better name? because the name of the function is ``brier_score_loss`` and we also have other similar scorers like "neg_log_loss"? @user1

> and "Brier score loss" is not something known in the literature

If so, we should change the name of the function.

@user3:
Can you explain the definition of this neg_brier_score? look at my post please: https://stackoverflow.com/questions/62853440/neg-brier-score-returns-negative-score