-
Notifications
You must be signed in to change notification settings - Fork 112
feat: Add evaluate function for classifiers #195
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
Merged
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
453e5a9
Added multilabel option to training
Pringled 0226494
Added multilabel option to training
Pringled a22d61a
Added multilabel option to training
Pringled 68a4ae4
Added multilabel option to training
Pringled 614069a
Added multilabel option to training
Pringled b50bc4a
Added multilabel option to training
Pringled 6831bfe
Added threshold to predict
Pringled 7bf46ea
Updated docs
Pringled d277e79
Updated docs
Pringled d28b895
Removed fallback logic
Pringled 327ecb1
Updated docs
Pringled d38679f
Updated docs
Pringled 6d80e90
Resolved feedback
Pringled ad8ea8d
Update model2vec/train/README.md
Pringled b3363ff
Resolved feedback
Pringled 15f4873
Resolved feedback
Pringled 06dc246
Resolved feedback
Pringled 43de6da
Resolved feedback
Pringled 8e944ab
add multilabel targets, fix tests (#194)
stephantul ff4043f
Merge branch 'main' of https://github.com/MinishLab/model2vec into ad…
Pringled 5c9d397
Fixed bug with array conversion
Pringled 6a4f89b
Optimized inference performance
Pringled 3609e62
Changed classes to np array
Pringled b4df861
Added int as possible label type
Pringled ba29feb
Added int as possible label type
Pringled 3dcddf5
Use previous logic
Pringled eccec80
Updated type check
Pringled f9037d9
Updated type check
Pringled 2dc5b17
Updated type check logic
Pringled 5003768
Fixed merge conflict
Pringled b6c00b8
Added evaluate function
Pringled a51f0bb
Updated evaluate, updated tests to also include int type labels
Pringled 1c86d5e
Updated docs
Pringled f939695
Fixed inference tests
Pringled aa07183
Refactored evaluate. Made evaluate available for pipelines. Simplifie…
Pringled 69d990a
Removed unused imports
Pringled 065e04d
Updated classes logic
Pringled File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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.
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 feel like this function is hitting the wrong abstraction level. Here's some observations:
self.multilabel.So I would refactor this into a function that takes a bunch of labels, and then, based on the type and shape of the output, returns a report. This function is then called by this function.
So something like this:
The evaluate_single_or_multilabel then simplifies to:
That way you can also test these functions without needing to have models, and can also reuse them in other contexts. The consequence of all of this, however, is that
evaluatesimplifies to:So maybe having
evaluateis not even necessary any more.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 refactored the code as per your suggestions.
evaluate_single_or_multilabeland_is_multi_label_shapedevaluatefunction that callsevaluate_single_or_multilabelwith the model predictionsThis way
evaluateis available to both trained models and pipeline converted models. I also updated the tests to reflect this.As for your other comment: yes, this is a essentially a thin wrapper around MultiLabelBinarizer and classification_report. However, I think this is worth it. Consider the following example:
Vs:
This is much easier to run and understand in my opinion, and fits in with the rest of our training code, which creates a wrapper around torch/lightning. While the function does not add much for the singelabel case, it does provide a unified interface and function, and even in that case it does give a slightly nicer way to evaluate IMO:
Vs: