[MRG+2] ENH&BUG Add pos_label parameter and fix a bug in average_precision_score by qinhanmin2014 · Pull Request #9980 · scikit-learn/scikit-learn

qinhanmin2014 · 2017-10-23T08:52:46Z

Reference Issues/PRs

part of #9829

What does this implement/fix? Explain your changes.

(1)add pos_label parameter to average_precision_score (Although we finally decide not to introduce pos_label in roc_auc_score, I think we might need pos_label here. Because there are no relationship between the results if we reverse the true labels, also, precision/recall all support pos_label)
(2)fix a bug where average_precision_score will sometimes return nan when sample_weight contains 0

y_true = np.array([0, 0, 0, 1, 1, 1])
y_score = np.array([0.1, 0.4, 0.85, 0.35, 0.8, 0.9])
average_precision_score(y_true, y_score, sample_weight=[1, 1, 0, 1, 1, 0])
# output:nan

I do it here because of (3)
(3)move average_precision scores out of METRIC_UNDEFINED_BINARY (this should contain the regression test for (1) and (2))

Some comments:
(1)For the underlying method(precision_recall_curve), the default value of pos_label is None, but I choose to set the default value of pos_label to 1 because this is what P/R/F is doing. What's more, the meaning of pos_label=None is not clear even in scikit-learn itself (see #10010)
(2)I slightly modified the common test. Currently, the part I modified is only designed for brier_score_loss(I'm doing the same thing in #9562) . I think it is right because as a common test, it seems not good to force metrics to accept str y_true without pos_label.

Any other comments?

cc @jnothman Could you please take some time to review or at least judge whether this is the right way to go? Thanks a lot :)

qinhanmin2014 · 2017-10-26T07:26:04Z

ping @jnothman Could you please take some time to review it or at least judge whether this is the right way to go? Thanks a lot :)

jnothman · 2017-10-26T07:42:26Z

I've been taking a little break from review to work on other things, while still doing triage. this PR is not lost, merely waiting

qinhanmin2014 · 2017-12-06T06:29:31Z

ping @jnothman for a review. Thanks :)

jnothman

Otherwise LGTM

jnothman · 2017-12-06T08:43:41Z

sklearn/metrics/ranking.py

-
+    y_type = type_of_target(y_true)
+    if y_type == "binary":
+        _partial_binary_uninterpolated_average_precision = partial(


This is used once. It can have a short, uninformative name...

jnothman · 2017-12-09T21:01:04Z

sklearn/metrics/ranking.py

            Calculate metrics for each instance, and find their average.

+    pos_label : int or str (default=1)
+        The label of the positive class. For multilabel-indicator y_true,


Perhaps clarify that this is only applied to binary classification. We do not, for instance, binarize a multiclass problem using pos_label...

jnothman · 2017-12-09T21:01:06Z

sklearn/metrics/ranking.py

+        return _average_binary_score(
+            _partial_binary_uninterpolated_average_precision, y_true,
+            y_score, average, sample_weight=sample_weight)
+    else:


are we sure at this stage that the y_type is not multiclass?

jnothman · 2017-12-09T21:01:43Z

sklearn/metrics/ranking.py

+            raise ValueError("Parameter pos_label is fixed to 1 for "
+                             "multilabel-indicator y_true. Do not set "
+                             "pos_label or set pos_label to 1.")
+        return _average_binary_score(


Why don't you outdent this and use it in all cases?

…_pos_label

qinhanmin2014 · 2017-12-10T02:20:32Z

@jnothman Thanks a lot for the review :) Comments addressed. I also simplify the code.

are we sure at this stage that the y_type is not multiclass?

At this point not. But we do the check once we finish the preparation and enter _average_binary_score, so I think it seems unnecessary to add a duplicate check here.

jnothman

LGTM. Please add what's new

jnothman · 2018-01-16T09:52:57Z

sklearn/metrics/ranking.py

    ----------
    y_true : array, shape = [n_samples] or [n_samples, n_classes]
-        True binary labels (either {0, 1} or {-1, 1}).
+        True binary labels or binary label indicators.


label indicators -> multilabel indicators ??

…_pos_label

qinhanmin2014 · 2018-01-16T14:04:40Z

@jnothman Thanks for the review :) I update what's new (not sure whether we need two entries)

label indicators -> multilabel indicators ??

I'm not creating term but just reverting the previous change (#9557) since we now extend the function. We are using the term binary label indicators in several places and we have stated that it's for multilabel case in the document. Do you think I need to change it? Thanks.

jnothman · 2018-01-16T21:34:51Z

sounds alright. I'm just trying to help users new to the terminology. they are label indicators, used for multilabel representation. some of the terminology is confused for historical reasons: we used to have an alternative multilabel format.

GaelVaroquaux

This looks good aside from the cosmetic comment.

GaelVaroquaux · 2018-07-16T14:31:44Z

sklearn/metrics/ranking.py

 import numpy as np
 from scipy.sparse import csr_matrix
 from scipy.stats import rankdata
+from functools import partial


Cosmetic: you should move this import to the top of the imports, as it is an import from the standard library.

GaelVaroquaux · 2018-07-16T14:43:07Z

I am addressing the cosmetic comment myself and merging.

GaelVaroquaux

LGTM.

Merging when CI is green.

qinhanmin2014 · 2018-07-16T15:00:16Z

Thanks a lot @GaelVaroquaux :)
FYI, I remove an unused variable.

qinhanmin2014 added 4 commits October 23, 2017 16:42

average precision score pos_label

132ab73

minor improve

7123d4d

minor improve

846dada

minor improve

d61a4ae

jnothman reviewed Dec 9, 2017

View reviewed changes

qinhanmin2014 added 2 commits December 10, 2017 09:04

Merge remote-tracking branch 'upstream/master' into average_precision…

28ab371

…_pos_label

jnothman's comment

7b92714

jnothman approved these changes Jan 16, 2018

View reviewed changes

jnothman changed the title ~~[MRG] ENH&BUG Add pos_label parameter and fix a bug in average_precision_score~~ [MRG+1] ENH&BUG Add pos_label parameter and fix a bug in average_precision_score Jan 16, 2018

qinhanmin2014 added 2 commits January 16, 2018 21:40

Merge remote-tracking branch 'upstream/master' into average_precision…

5e17c11

…_pos_label

what's new

652d296

jnothman mentioned this pull request Jan 31, 2018

Averaging scores for classes for the average_precision_score #10564

Closed

Erotemic mentioned this pull request Apr 5, 2018

brier_score_loss returns incorrect value when all y_true values are True/1 #9300

Closed

glemaitre added this to the 0.20 milestone Jun 8, 2018

GaelVaroquaux reviewed Jul 16, 2018

View reviewed changes

Merge branch 'master' into average_precision_pos_label

b4bbb20

COSMIT: stdlib import come first

2e32565

GaelVaroquaux changed the title ~~[MRG+1] ENH&BUG Add pos_label parameter and fix a bug in average_precision_score~~ [MRG+2] ENH&BUG Add pos_label parameter and fix a bug in average_precision_score Jul 16, 2018

GaelVaroquaux approved these changes Jul 16, 2018

View reviewed changes

remove unused variable

9f45954

amueller merged commit dd69361 into scikit-learn:master Jul 16, 2018

qinhanmin2014 deleted the average_precision_pos_label branch July 17, 2018 01:47

amueller mentioned this pull request Oct 5, 2018

average_precision_score breaks on string labels #12312

Closed

qinhanmin2014 mentioned this pull request Sep 4, 2019

Precision Recall and F-score: behavior when all negative #14876

Closed

glemaitre mentioned this pull request Sep 7, 2022

FIX remove np.divide with where and without out argument in precision_recall_curve #24382

Merged

Uh oh!

Conversation

qinhanmin2014 commented Oct 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

qinhanmin2014 commented Oct 26, 2017

Uh oh!

jnothman commented Oct 26, 2017 via email

Uh oh!

qinhanmin2014 commented Dec 6, 2017

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman Dec 6, 2017

Choose a reason for hiding this comment

Uh oh!

jnothman Dec 9, 2017

Choose a reason for hiding this comment

Uh oh!

jnothman Dec 9, 2017

Choose a reason for hiding this comment

Uh oh!

jnothman Dec 9, 2017

Choose a reason for hiding this comment

Uh oh!

qinhanmin2014 commented Dec 10, 2017

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

jnothman Jan 16, 2018

Choose a reason for hiding this comment

Uh oh!

qinhanmin2014 commented Jan 16, 2018

Uh oh!

jnothman commented Jan 16, 2018 via email

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux Jul 16, 2018

Choose a reason for hiding this comment

Uh oh!

GaelVaroquaux commented Jul 16, 2018

Uh oh!

GaelVaroquaux left a comment

Choose a reason for hiding this comment

Uh oh!

qinhanmin2014 commented Jul 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

qinhanmin2014 commented Oct 23, 2017 •

edited

Loading