[MRG+1] Partial AUC #3840

Alexander-N · 2014-11-07T23:15:42Z

This needs better tests, any ideas how to make them?

Issue #3273
partial AUC: integrate the ROC curve until chosen maximal FPR and standardize it to lie between 0.5 and 1.

lesshaste · 2017-09-01T08:56:04Z

This seems like a useful addition. What happened to it?

albertcthomas · 2017-09-21T09:41:14Z

This could be useful for anomaly/outlier/novelty detection as well as we are especially interested in the performance of the scoring function (e.g. decision_function) in the regions corresponding to small FPR values.

Alexander-N · 2017-09-21T17:29:34Z

I stopped working on this since I wasn't sure how to test this properly. If anybody could give me some hints I would pick it up again.

jnothman · 2017-09-24T21:51:39Z

sklearn/metrics/tests/test_ranking.py

@@ -138,6 +138,12 @@ def test_roc_curve():
    assert_equal(fpr.shape, tpr.shape)
    assert_equal(fpr.shape, thresholds.shape)

+    # test partial ROC


This does not belong in test_roc_curve.

Make a new function

jnothman · 2017-09-24T21:51:39Z

sklearn/metrics/tests/test_ranking.py

+    y_true = np.array([0, 0, 1, 1])
+    y_scores = np.array([0.1,  0,  0.1, 0.01])
+    assert_equal(roc_auc_score(y_true, y_scores, max_fpr=0.3), 0.5)
+    assert_equal(roc_auc_score(y_true, y_true, max_fpr=0.7), 1)


This varies in both data and max_fpr from the previous line. A better test would consider minimal variations to the parameter and the data and show that the changes affect the metric in the expected way. I'd demonstrate for instance that max_fpr=1 equates to leaving it out, then reduce it.

serv-inc · 2018-01-31T09:09:13Z

Works fine with 0.18.2. File is ranking.py.txt

How can this be brought to a finish?

@Alexander-N: would you prefer if you edited some testing code into this, or are you no longer interested? @jnothman probably meant that you should add a new function to test_ranking.py with a few different test cases.

result for max_fpr=1 is the same as not setting the parameter
results for max_fpr=0.9 as compared to max_fpr=1, etc

Alexander-N · 2018-01-31T09:51:54Z

Sorry for being unresponsive. I will look into it this weekend.

Alexander-N · 2018-02-22T17:39:34Z

I addressed the comments, this is ready for further review. Just wanted to make that clear to attract reviewers :-).

jnothman

Can you add this as a metric for common testing in metrics/tests/test_common.py? This way you can check its basic properties and invariances.

jnothman · 2018-02-22T21:07:39Z

sklearn/metrics/ranking.py

@@ -257,6 +258,9 @@ def roc_auc_score(y_true, y_score, average="macro", sample_weight=None):
    sample_weight : array-like of shape = [n_samples], optional
        Sample weights.

+    max_fpr : float, optional If not ``None``, the standardized partial AUC
+        over the range [0, max_fpr] is returned.


Include a citation of the reference

please insert a newline

max_fpr : float, optional If not ``None``, the standardized partial AUC over the range [0, max_fpr] is returned.

Also, maybe add here that it should be in [0, 1] and raise a ValueError in the code if it's not?

jnothman · 2018-02-22T21:07:39Z

sklearn/metrics/ranking.py

+        if max_fpr is None or max_fpr == 1:
+            return auc(fpr, tpr)
+
+        idx = np.where(fpr <= max_fpr)[0]


fpr is increasing, so it would be clearer if we just did something like:

stop = np.searchsorted(fpr, max_fpr, 'right')

and below used tpr[:stop] instead of tpr[idx] and avoided having both names idx_in and idx_out

jnothman · 2018-02-22T21:07:39Z

sklearn/metrics/ranking.py

+            return auc(fpr, tpr)
+
+        idx = np.where(fpr <= max_fpr)[0]
+        # Linearly interpolate the ROC curve until max_fpr


perhaps this should be clearer to say we're just adding a single point at max_fpr by interpolation

albertcthomas

A partial review. For the tests, I was told very recently by @jnothman that we should use assert instead of assert_equal to check equality.

albertcthomas · 2018-02-23T10:55:56Z

sklearn/metrics/ranking.py

@@ -257,6 +258,9 @@ def roc_auc_score(y_true, y_score, average="macro", sample_weight=None):
    sample_weight : array-like of shape = [n_samples], optional
        Sample weights.

+    max_fpr : float, optional If not ``None``, the standardized partial AUC
+        over the range [0, max_fpr] is returned.


please insert a newline

max_fpr : float, optional If not ``None``, the standardized partial AUC over the range [0, max_fpr] is returned.

Also, maybe add here that it should be in [0, 1] and raise a ValueError in the code if it's not?

albertcthomas · 2018-02-23T10:55:56Z

sklearn/metrics/ranking.py

-        return auc(fpr, tpr)
+        if max_fpr is None or max_fpr == 1:
+            return auc(fpr, tpr)
+


WDYT about adding

if max_fpr == 0: return 0

as the AUC is equal to 0 when max_fpr = 0

Since setting max_fpr to zero makes no sense I think it might be best to raise a ValueError.

albertcthomas · 2018-02-23T10:55:56Z

sklearn/metrics/tests/test_ranking.py

+    for max_fpr in np.linspace(0, 1, 5):
+        assert_almost_equal(
+            roc_auc_score(y_true, y_pred, max_fpr=max_fpr),
+            _partial_roc_auc_score(y_true, y_pred, max_fpr))


Also test the behavior when max_fpr is not in [0, 1]. If we raise a ValueError, then test that the ValueError is raised with an informative message.

albertcthomas · 2018-02-23T10:55:56Z

sklearn/metrics/tests/test_ranking.py

+    y_true = np.array([0, 0, 1, 1])
+    assert_equal(roc_auc_score(y_true, y_true, max_fpr=1), 1)
+    assert_equal(roc_auc_score(y_true, y_true, max_fpr=0.001), 1)
+    assert_equal(np.isnan(roc_auc_score(y_true, y_true, max_fpr=0)), True)


Just use

assert np.isnan(roc_auc_score(y_true, y_true, max_fpr=0))

But I would be in favor of returning 0 when max_fpr=0 (see my comment above). WDYT?

albertcthomas · 2018-02-23T12:33:43Z

sklearn/metrics/ranking.py

+        fpr = np.append(fpr[idx], max_fpr)
+        partial_auc = auc(fpr, tpr)
+
+        # McClish correction: standardize result to lie between 0.5 and 1


It seems that the computation of min_area assumes that the estimator is always better than random guess. In practice, for a very bad estimator (doing worse than random guess) partial_auc could be less than min_area and the result will not necessarily be in [0.5, 1]. It will always be lower than 1 but can be less than 0.5 which would mean that we are doing worse than a random guess estimator. I think we should keep this standardization but we should explain somewhere that the result is not necessarily in [0.5, 1].

Alexander-N · 2018-02-25T01:07:14Z

Great comments, thanks! Except returning 0 when max_fpr is 0, everything done as suggested.

albertcthomas

I would have returned 0 when max_fpr is 0, even if it does not make sense, that's what the output should be in theory but LGTM. Thanks @Alexander-N!

jnothman

Nice work. LGTM

jnothman · 2018-02-25T22:14:37Z

Please add an entry to the change log at doc/whats_new/v0.20.rst. Like the other entries there, please reference this pull request with :issue: and credit yourself (and other contributors if applicable) with :user:

jnothman · 2018-02-25T22:16:28Z

max_fpr=0 might have a theoretically value, but it is a nonsense setting for an evaluation metric. Error is good.

albertcthomas · 2018-02-25T22:48:08Z

Fair enough.

Alexander-N · 2018-02-26T09:23:55Z

Ok, done.

jnothman · 2018-02-26T12:22:59Z

Please add cmmots rather than amending to make it easier to review the changes

Alexander-N · 2018-02-26T12:51:03Z

Ah ok sorry. I amended to correct the original commit message. Only change is the addition of the entry in doc/whats_new/v0.20.rst.

jnothman

I was going to merge this, but I think it deserves a mention in doc/modules/model_evaluation.rather when discussing the auc score. Try to help users understand why this may be a better/worse metric for their task.

partial AUC: Integrate the ROC curve until chosen maximal FPR and standardize the result to be 0.5 if non-discriminant and 1 if maximal. Closes scikit-learn#3273

Alexander-N · 2018-02-28T21:05:05Z

I could add something along the lines of

In applications where a high false positive rate is not tolerable the parameter ``max_fpr`` can be used to summarize the ROC curve up to the given limit.

I feel that this might not be very helpful. Do you have something specific in mind?

jnothman · 2018-02-28T21:36:50Z

yes, that sounds good. or perhaps precede witg "one critique of auc roc is that it gives undue weight to operating points with impractically large for" or something with better wording!

…

On 1 Mar 2018 8:05 am, "Alexander-N" ***@***.***> wrote: I could add something along the lines of In applications where a high false positive rate is not tolerable the parameter ``max_fpr`` can be used to summarize the ROC curve up to the given limit. I feel that this might not be very helpful. Do you have something specific in mind? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3840 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEz69uXfpUh15YcLkfJ8Jp21zxdbFSSks5tZb-DgaJpZM4C4keZ> .

Alexander-N · 2018-03-01T17:40:46Z

Ok, done.

jnothman · 2018-03-01T21:37:33Z

I had been suggesting you put "one critique" first as a way to improve cohesion. Now it reads a bit strangely. (And I didn't really like my own wording here)

Perhaps you should just drop my words and leave your own.

Alexander-N · 2018-03-01T22:10:46Z

Ok, Done :-)

jnothman · 2018-03-01T23:45:33Z

thanks for completing this after so long

jnothman

Thanks

jnothman changed the title ~~[WIP] adress Issue #3273~~ [WIP] Partial AUC Nov 8, 2014

raghavrv mentioned this pull request Feb 3, 2015

Implement Average SGD #543

Closed

albertcthomas mentioned this pull request Sep 21, 2017

partial AUC #3273

Closed

jnothman reviewed Sep 24, 2017

View changes

Alexander-N force-pushed the partial-AUC branch from b613233 to c083006 Compare Feb 4, 2018

Alexander-N changed the title ~~[WIP] Partial AUC~~ [MRG] Partial AUC Feb 4, 2018

jnothman reviewed Feb 22, 2018

View changes

albertcthomas reviewed Feb 23, 2018

View changes

Alexander-N force-pushed the partial-AUC branch from 99ceece to 421af56 Compare Feb 25, 2018

albertcthomas approved these changes Feb 25, 2018

View changes

jnothman approved these changes Feb 25, 2018

View changes

jnothman changed the title ~~[MRG] Partial AUC~~ [MRG+1] Partial AUC Feb 25, 2018

Alexander-N force-pushed the partial-AUC branch 2 times, most recently from 1d16f37 to 089f58d Compare Feb 26, 2018

jnothman reviewed Feb 26, 2018

View changes

Add partial AUC to roc_auc_score

49a7f23

partial AUC: Integrate the ROC curve until chosen maximal FPR and standardize the result to be 0.5 if non-discriminant and 1 if maximal. Closes scikit-learn#3273

Alexander-N force-pushed the partial-AUC branch from 089f58d to 49a7f23 Compare Feb 28, 2018

Mention 'max_fpr' in model_evaluation.rst

c6a38d9

model_evaluation.rst: Reword the sentence

9eddd69

jnothman approved these changes Mar 1, 2018

View changes

jnothman merged commit 02ddc70 into scikit-learn:master Mar 1, 2018
4 of 6 checks passed

Fengjun-Wang mentioned this pull request Mar 22, 2021

[MRG] Add min_tpr parameter to roc_auc_score, allow both min_tpr and max_fpr #19751

Open

scikit-learn / scikit-learn Public

[MRG+1] Partial AUC #3840

[MRG+1] Partial AUC #3840

Conversation

Alexander-N commented Nov 7, 2014

lesshaste commented Sep 1, 2017

albertcthomas commented Sep 21, 2017

Alexander-N commented Sep 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serv-inc commented Jan 31, 2018 • edited

Alexander-N commented Jan 31, 2018

Alexander-N commented Feb 22, 2018

jnothman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albertcthomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albertcthomas Feb 23, 2018 • edited

Choose a reason for hiding this comment

albertcthomas Feb 23, 2018 • edited

Choose a reason for hiding this comment

Alexander-N commented Feb 25, 2018

albertcthomas left a comment

jnothman left a comment

jnothman commented Feb 25, 2018

jnothman commented Feb 25, 2018

albertcthomas commented Feb 25, 2018

Alexander-N commented Feb 26, 2018

jnothman commented Feb 26, 2018

Alexander-N commented Feb 26, 2018

jnothman left a comment

Alexander-N commented Feb 28, 2018

jnothman commented Feb 28, 2018

Alexander-N commented Mar 1, 2018

jnothman commented on c6a38d9 Mar 1, 2018

Choose a reason for hiding this comment

Alexander-N commented on c6a38d9 Mar 1, 2018

Choose a reason for hiding this comment

jnothman commented on c6a38d9 Mar 1, 2018

Choose a reason for hiding this comment

jnothman left a comment

serv-inc commented Jan 31, 2018 •

edited

albertcthomas Feb 23, 2018 •

edited

albertcthomas Feb 23, 2018 •

edited

jnothman commented on `c6a38d9` Mar 1, 2018

Alexander-N commented on `c6a38d9` Mar 1, 2018

jnothman commented on `c6a38d9` Mar 1, 2018