Skip to content
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

Added a flag to allow skipping the first projection in small ResNets #11176

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

laxmareddyp
Copy link
Collaborator

@laxmareddyp laxmareddyp commented Feb 29, 2024

Description

This should fix #10583
In this issue, I described how the small ResNets implemented here have an extra convolution (for projection), that is neither present in the original paper or in PyTorch.
This PR allows to get rid of this extra convolution with a keyword argument that defaults to the previous behaviour so that this remains a non-breaking change.

This PR created from #10584 as it is having problem to merge in codebase with manual copybara sync.

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note:I didn't wait for a discussion because this seemed like a relatively small and simple change,
so it's not bothering for me to have written it even if rejected in the end.

  • New feature (non-breaking change which adds functionality)

Tests

I added tests to check the parameter count.
I also offline checked the number of non-trainable parameters to checked that it matched against PyTorch's one.

Checklist

@laxmareddyp laxmareddyp added the models:official models that come under official repository label Feb 29, 2024
@laxmareddyp laxmareddyp added the ready to pull ready to pull (create internal pr review and merge automatically) label Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
models:official models that come under official repository ready to pull ready to pull (create internal pr review and merge automatically)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small ResNets have an extra convolution
2 participants