-
Notifications
You must be signed in to change notification settings - Fork 45.9k
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
Small ResNets have an extra convolution #10583
Open
1 task done
Labels
Comments
zaccharieramzi
added
models:official
models that come under official repository
type:feature
labels
Apr 7, 2022
9 tasks
Hi @zaccharieramzi , If the PR is already created, could you please attach the PR here. Thanks. |
It is attached above #10584 |
Thanks for the quick response. |
laxmareddyp
assigned arashwan, yeqingli and laxmareddyp and unassigned arashwan, xianzhidu and yeqingli
Aug 17, 2023
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Prerequisites
Please answer the following question for yourself before submitting an issue.
1. The entire URL of the file you are using
https://github.com/tensorflow/models/blob/master/official/vision/modeling/backbones/resnet.py
2. Describe the feature you request
The ResNet18 and ResNet34 have an extra convolution compared to what was described in the original paper and to PyTorch.
Indeed, in the first block of the small resnets, the projection should not be used, as there is no increase in dimension.
This can be seen clearly in Figure 3. of the paper where the first shortcut is not dashed.
It would be interesting for reproducibility's sake to have at least the option to remove this extra projection.
3. Additional context
I was actually implementing the ResNet18 and 34 in keras applications and had to remove this first projection too in order to get the same parameter count as PyTorch.
Note that this is different from #3825 which required a mean to have a different projection function.
4. Are you willing to contribute it? (Yes or No)
Yes, submitting a PR ASAP.
The text was updated successfully, but these errors were encountered: