FIX Fix recall in multilabel classification when true labels are all negative #19085
Conversation
|
Thanks @varunagrawal for following-up #14621. The failing check is unrelated with your PR: it will be solved merging #18930. |
|
Is there a plan to merge this commit? |
|
Hi @varunagrawal, thanks for your patience! Please add an entry to the change log at |
|
@cmarmo done and done. |
sklearn/metrics/_ranking.py
Outdated
| @@ -856,7 +856,7 @@ def precision_recall_curve(y_true, probas_pred, *, pos_label=None, sample_weight | |||
|
|
|||
| precision = tps / (tps + fps) | |||
| precision[np.isnan(precision)] = 0 | |||
| recall = tps / tps[-1] | |||
| recall = np.ones(tps.size) if tps[-1] == 0 else tps / tps[-1] | |||
There was a problem hiding this comment.
In precision_recall_fscore_support, there is a zero_division parameter that controls how we handle the edge case. This parameter is also used in recall_score and precision_score. If we want to be consistent with precision_recall_fscore_support, we would need to add a zero_division and use the same semantics.
What do you think?
There was a problem hiding this comment.
@thomasjpfan this issue has been sitting unfixed for years. I think this fix should be merged as-is before it bit rots, and open a separate issue for semantic consistency.
There was a problem hiding this comment.
I agree with @crypdick on this. It's been over 3 years for what is a very simple bugfix, and we can track semantic consistency via another issue. That issue can be tackled by someone else, potentially from the sklearn team, which would get the ball rolling sooner.
There was a problem hiding this comment.
Okay, let's keep the recall set to one. May we update precision_recall_curve's docstring to reflect this behavior?
| y_true = np.array([[1, 0], [0, 1]]) | ||
| y_score = np.array([[0, 1], [1, 0]]) |
There was a problem hiding this comment.
I think we can remove this, since the input is the exactly the same as the one above it.
doc/whats_new/v1.0.rst
Outdated
| - |Fix| Fix recall in multilabel classification when true labels are all negative. | ||
| :pr:`19085` by :user:`Varun Agrawal <varunagrawal>`. |
There was a problem hiding this comment.
We can adjust the whats new to state which function is being fixed.
- |Fix| Fixes `average_precision_score` for multilabel classification when true labels are
all negative. :pr:`19085` by :user:`Varun Agrawal <varunagrawal>`.There also needs to be an entry for precision_recall_curve to describe the new behavior.
These entries need to be moved to doc/whats_new/v1.1.rst.
sklearn/metrics/_ranking.py
Outdated
| @@ -856,7 +856,7 @@ def precision_recall_curve(y_true, probas_pred, *, pos_label=None, sample_weight | |||
|
|
|||
| precision = tps / (tps + fps) | |||
| precision[np.isnan(precision)] = 0 | |||
| recall = tps / tps[-1] | |||
| recall = np.ones(tps.size) if tps[-1] == 0 else tps / tps[-1] | |||
There was a problem hiding this comment.
Okay, let's keep the recall set to one. May we update precision_recall_curve's docstring to reflect this behavior?
|
I merged main and addressed the comments from reviews and the comments I was going to do. |
There was a problem hiding this comment.
LGTM. @thomasjpfan and @glemaitre do you want to take a look ?
|
Note that setting the recall to one in that case lead to the following behavior, which seems like an acceptable expected behavior. import numpy as np
from<
A8C6
/span> sklearn.metrics import precision_recall_curve
import matplotlib.pyplot as plt
from sklearn.metrics import PrecisionRecallDisplay
y_true = np.array([0, 0, 0, 0])
y_scores = np.array([0.1, 0.4, 0.35, 0.8])
display = PrecisionRecallDisplay.from_predictions(y_true, y_scores)
plt.show()
|
|
Thanks @varunagrawal ! |
…negative (scikit-learn#19085) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Co-authored-by: jeremiedbb <jeremiedbb@yahoo.fr>
Note that scikit-learn>=1.1 is required in order to have valid values for average precision score. That is due to version 1.1+ of scikit-learn including this fix: scikit-learn/scikit-learn#19085 NB: version 1.1+ of scikit-learn requires python 3.8+.
Reference Issues/PRs
Fixes #8245
What does this implement/fix? Explain your changes.
When all the y_true labels are negative, precision_recall_curve returns
nanbecause of recall being set tonaninstead of 1. This is because of the direction division of thetpsvector bytps[-1]which is be 0.This fix checks if
tps[-1]is 0 and if yes, sets the recall to 1 directly since there are no True Positives or False Negatives, else we calculate recall as normal.I updated and added tests to check for this case.
Any other comments?
Please refer to the issue thread for more discussion points.