Skip to content
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

A bug in iter_confusion_matrices #3

Closed
wants to merge 2 commits into from
Closed

Conversation

yasirs
Copy link
Contributor

@yasirs yasirs commented Sep 28, 2011

This is the bug.

import yard
a = [(10,1),(20,0),(30,0),(40,1)]
d = yard.BinaryClassifierData(a)
list(d.iter_confusion_matrices([20,30,40]))
[(20, BinaryConfusionMatrix(tp=-1, fp=0, fn=3, tn=2)),
 (30, BinaryConfusionMatrix(tp=0, fp=1, fn=2, tn=1)),
 (40, BinaryConfusionMatrix(tp=0, fp=0, fn=2, tn=2)),
 (inf, BinaryConfusionMatrix(tp=-1, fp=0, fn=3, tn=2))]

Clearly, tp=-1 is wrong.

import yard
a = [(10,1),(20,0),(30,0),(40,1)]
d = yard.BinaryClassifierData(a)
list(d.iter_confusion_matrices([20,30,40]))
[(20, BinaryConfusionMatrix(tp=-1, fp=0, fn=3, tn=2)),
 (30, BinaryConfusionMatrix(tp=0, fp=1, fn=2, tn=1)),
 (40, BinaryConfusionMatrix(tp=0, fp=0, fn=2, tn=2)),
 (inf, BinaryConfusionMatrix(tp=-1, fp=0, fn=3, tn=2))]

Clearly, tp=-1 is wrong.
@ntamas
Copy link
Owner

ntamas commented Sep 28, 2011

Thanks, you seem to be spot on here. I'll check it tomorrow after I've slept a bit and merge it if it's OK.

@ntamas ntamas closed this Sep 29, 2011
@ntamas
Copy link
Owner

ntamas commented Sep 29, 2011

The final fix turned out to be different in the end because I wanted to make the results of get_confusion_matrix and iter_confusion_matrices consistent and I realized that inf should not be added to the end of the iterator when the thresholds are given explicitly. You should merge my changes into your branch to get the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants