[WIP] Added DET curve to classification metrics (Rebased)#8724
Closed
jucor wants to merge 10 commits intoscikit-learn:masterfrom
Closed
[WIP] Added DET curve to classification metrics (Rebased)#8724jucor wants to merge 10 commits intoscikit-learn:masterfrom
jucor wants to merge 10 commits intoscikit-learn:masterfrom
Conversation
dmohns
reviewed
Aug 7, 2017
Contributor
dmohns
left a comment
There was a problem hiding this comment.
I think the following two things should be added to this PR:
- The naming of inputs and outputs should be a little consistent with the rest of the ranking module e.g.
fprwhere rates are returned orfpswhen counts are required. Seeroc_curveor_binary_clf_curve - Some basic testing would be nice like for ROC and PR curves.
| .. [1] `Wikipedia entry for Detection error tradeoff | ||
| <https://en.wikipedia.org/wiki/Detection_error_tradeoff>`_ | ||
| .. [2] `The DET Curve in Assessment of Detection Task Performance | ||
| <http://www.itl.nist.gov/iad/mig/publications/storage_paper/det.pdf>`_ |
Contributor
There was a problem hiding this comment.
This link doesn't work. It appears to be a problem on https://www.nist.gov/ site though.
| .. [2] `The DET Curve in Assessment of Detection Task Performance | ||
| <http://www.itl.nist.gov/iad/mig/publications/storage_paper/det.pdf>`_ | ||
| .. [3] `2008 NIST Speaker Recognition Evaluation Results | ||
| <http://www.itl.nist.gov/iad/mig/tests/sre/2008/official_results/>`_ |
| .. [3] `2008 NIST Speaker Recognition Evaluation Results | ||
| <http://www.itl.nist.gov/iad/mig/tests/sre/2008/official_results/>`_ | ||
| .. [4] `DET-Curve Plotting software for use with MATLAB | ||
| <http://www.itl.nist.gov/iad/mig/tools/DETware_v2.1.targz.htm>`_ |
dmohns
reviewed
Aug 7, 2017
|
|
||
| Returns | ||
| ------- | ||
| fps : array, shape = [n_thresholds] |
Contributor
There was a problem hiding this comment.
If I am not mistaken the DET curve should return rates not counts. This looks like a copy/paste legacy which we should update accordingly.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Reference Issue
Resumes #4980 started by @jkarnows , which was left almost entirely finished.
What does this implement/fix? Explain your changes.
The great PR #4980 was left somewhat unfinished, and due to changes to the main trunk could not be merged any more.
The present PR rebases on master, and offers to finish this DET curve that I need for my own work.
Any other comments?
First time I contribute to scikit-learn, I do not mean to steal @jkarnows work, but it'd be a waste not to build on his great contribution.