-
Notifications
You must be signed in to change notification settings - Fork 237
Add regression test for Hub model labels #1280
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
base: main
Are you sure you want to change the base?
Conversation
|
want to share 1 Note on Everglades Nest Model: |
|
Thanks for the contribution. Prediction isn't necessary for a sanity check here. The problem is not that the incorrect weights are loaded, it's that the wrong labels were being assigned. I think it's sufficient to check that the Thanks also for the catch on the Nest model. We may want some other sanity checks performance hasn't regressed on in-domain images, but we already do that for at least the tree model. We also already have a unit test file for HF models: https://github.com/weecology/DeepForest/blob/main/tests/test_hf_models.py so these tests could be moved there. Stylistic note: we should check both (1) model.name passed as a |
|
Thank you @jveitchmichaelis and @henrykironde, I have Updated the PR based on the feedback. I moved the test to |
|
Hi @musaqlain thanks again for adding this, could you move this logic to the following file |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1280 +/- ##
==========================================
- Coverage 87.89% 87.16% -0.73%
==========================================
Files 20 20
Lines 2776 2782 +6
==========================================
- Hits 2440 2425 -15
- Misses 336 357 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
henrykironde
left a comment
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.
We want to make sure that other models are also added
|
thanks, I've updated the PR as requested. kindly check now 👍 cc @henrykironde |
henrykironde
left a comment
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.
Looks good so far. made some suggestions that may help us make the tests better
|
PTAL @henrykironde |
Added
tests/test_hub_models.pyto verify thatload_modelcorrectly populateslabel_dictfor non-default models. This ensures regression coverage for the changes in #1263.Models Tested:
weecology/deepforest-bird(Verified label: "Bird")weecology/deepforest-livestock(Verified label: "Livestock")weecology/deepforest-tree(Verified label: "Tree")Related Issue
Fixes #1276
AI-Assisted Development