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
Fix/multiclass recall macro avg ignore index #2710
base: master
Are you sure you want to change the base?
Fix/multiclass recall macro avg ignore index #2710
Changes from 46 commits
176711d
df36d0f
0773bab
78177ac
f7701ea
d6f041b
fb6c23d
3ae861b
a0401f6
858e0d1
bac6267
2976947
dbe1a5a
bb36be4
ead62fe
e0ed7e7
263548d
8cc5bf1
0483219
982cfea
d16c815
c078bd2
d61727e
9aa5928
581d3ec
61a4b56
a75c5e3
005cc94
4c8bea2
6767a4c
daa1006
2642f1c
52fda88
8fa7dbf
0b4818b
c396f5b
94e3b37
844ad9c
b63a661
a1cbaad
4b65013
69c3a31
48bea14
3753de1
043ca6b
bd84496
214e78b
34c88f7
a2b108a
b3feb3f
580ee6e
8209416
daf6d72
561f791
401e923
4568288
20e116a
7d804bd
9c25e0b
08bc4dd
62436fd
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.
Seems we are already testing various ignore_index with reference metric so if we had it wrong this did not pass already... it is possible that we also have a bug in the reference metric?
cc: @SkafteNicki
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.
looking to the code and the ignore index is already applied in
_multilabel_stat_scores_format
which reduces the preds/target size the same way as the reference metric so calling it with null weights in fact ignores additional indexThere 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.
The problem is we are using sklearn's
recall_score
as a reference for our unittests. So even if in_reference_sklearn_precision_recall_multiclass()
function we are usingremove_ignore_index
function for removing those predictions whose real values areignore_index
class before passing it torecall_score
function, it does not matter. Because wheneveraverage='macro'
sklearn'srecall_score
will always return mean cosidering the total no. of classes (as we are passing all the classes inrecall_score()
function'slabels
argument). That is the reason why unittests failed in the first place. I think we need to fix the unittests to take care of ignore_index using sklearn'srecall_score()
function'slabels
argument. I've prepared a notebook for explanation. cc:@Borda.