-
Notifications
You must be signed in to change notification settings - Fork 7.1k
add swin_s and swin_b variants and improved swin_t #6048
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
Conversation
docs/source/models.rst
Outdated
@@ -226,7 +228,9 @@ convnext_tiny 82.520 96.146 | |||
convnext_small 83.616 96.650 | |||
convnext_base 84.062 96.870 | |||
convnext_large 84.414 96.976 | |||
swin_t 81.358 95.526 | |||
swin_t 81.474 95.776 |
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.
original repo Acc@1 81.2 Acc@5 95.5
docs/source/models.rst
Outdated
@@ -226,7 +228,9 @@ convnext_tiny 82.520 96.146 | |||
convnext_small 83.616 96.650 | |||
convnext_base 84.062 96.870 | |||
convnext_large 84.414 96.976 | |||
swin_t 81.358 95.526 | |||
swin_t 81.474 95.776 | |||
swin_s 83.196 96.360 |
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.
original repo acc Acc@1 83.2 Acc@5 96.2
docs/source/models.rst
Outdated
swin_t 81.358 95.526 | ||
swin_t 81.474 95.776 | ||
swin_s 83.196 96.360 | ||
swin_b 83.582 96.640 |
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.
original repo Acc@1 83.5 Acc@5 96.5
@jdsgomes Good. It seems that you added some other tricks: |
@jdsgomes I've pushed a tiny change on doc strings to indicate its a "similar" recipe not the same. As @xiaohu2015 highlighted, we use a mix of the paper recipe with the standard recipe of TorchVision, so this will be more factual. My intention was to merge this prior merging the PR that removes the old models.srt (to avoid conflicts) but it seems that your PR fails on a specific test (see this) which looks related. Could you please check it out? |
Thanks @datumbox for the change in the docstrings, and @xiaohu2015 you are right this is a slightly modified recipe, I should have highlighted that initially! Looking in the error now |
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.
LGTM, thanks @jdsgomes. Feel free to merge on Green-ish CI.
The SwinV2 offical code has released, maybe we can also add swin_v2 |
@xiaohu2015 If you are up for it, by all means. I think it's possible to update this class to produce both Swin V1 and V2 models, similar to what we did with EfficientNets. |
Summary: * add swin_s and swin_b variants * fix swin_b params * fix n parameters and acc numbers * adding missing acc numbers * apply ufmt * Updating `_docs` to reflect training recipe * Fix exted for swin_b Reviewed By: NicolasHug Differential Revision: D36760946 fbshipit-source-id: 52b5056ee8b3efde4e00aebf7a7f732819d90c4f Co-authored-by: Vasilis Vryniotis <[email protected]>
Adding improved weights for
swin_t
and new weights forswin_s
, andswin_b
.The new variants were trained with trivial augment, instead of rand augment because it is actually closer to the original implementation.
Trainning commands:
Test commands:
cc @xiaohu2015 @datumbox