Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Metrics] Add multiclass auroc #4236
[Metrics] Add multiclass auroc #4236
Changes from 10 commits
983b795
62137f4
b3947e0
70d4d0d
c36cd9b
3bf01c7
6bfca06
1ee1457
e3076d5
19c4b73
60c3c98
30ccb62
7f82b3b
f681b24
ee4078d
8b1b7f1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ddrevicky could you move this to the unreleased section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be okay now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we should check pred.size(0) == target.size(0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add that of course but this is not a check that is done in any other metric implementation. So if it's done here it should probably be done everywhere. If that's desired, I could add a helper to
classification.py
Would that work? Then this helper could be used in each metric instead of copy pasting the
if
clause and the exception.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are slowly unifying the functional and class based interface, we are doing more checks for shape, so this will come in a future PR :]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it be
torch.unique(target).size(0) <= pred.size(1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we could have a get_num_classes utils too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well the metric is undefined when
torch.unique(target).size(0) < pred.size(1)
, that's why there is a strict equals. The way this implementation works (based on sklearn's version) is, that it uses a one-vs-rest strategy of computing the AUROC. Forn
classes it computesn
binary AUROCs, for each class in turn, it is considered a positive class and all the other classes negative. Then it averages those.E.g., for
n=3
andtarget=[0, 1, 1, 2]
, for class0
we binarize the target to make0
the positive class:[1, 0, 0, 0]
and compute theAUC
ofROC
of that.If a target label is not present in the
target
, e.g.n=3
andtarget=[0, 1, 1, 1]
then for the absent class2
the binarized target would look like[0, 0, 0, 0]
(all negative) andROC
cannot be computed (would raise an error). Consequently, the whole multiclass AUROC is undefined in that case.As for
get_num_classes
there already is such a util, but it does something different than we need here. It doesn't look at dimensions of the predictions, just at the max value in both and deduced num classes from that (which when I think about it now, could fail silently for example whenn_cls=5
buttarget=[0,1,2,3]
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've implemented this using the
multiclass_auc_decorator
similarly as is done forauroc
but have to say that as not such an experienced Pythonist I was scratching my head for a good while trying to figure out what themulticlass_auc_decorator
was doing. It's possible that for other people reading the code it might also take unnecessary amount of time. Would the following be more readable? No decorator is needed either outside or within themulticlass_auroc
function. Just my humble opinion, which do you guys prefer? :)