models icon indicating copy to clipboard operation
models copied to clipboard

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

Open laxmareddyp opened this issue 11 months ago • 0 comments

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 https://github.com/tensorflow/models/pull/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.

  • [x] 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

  • [ ] I have signed the Contributor License Agreement.
  • [ ] I have read guidelines for pull request.
  • [ ] My code follows the coding guidelines.
  • [ ] I have performed a self code review of my own code.
  • [ ] I have commented my code, particularly in hard-to-understand areas.
  • [ ] I have made corresponding changes to the documentation.
  • [ ] My changes generate no new warnings.
  • [ ] I have added tests that prove my fix is effective or that my feature works.

laxmareddyp avatar Feb 29 '24 22:02 laxmareddyp