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

Add groups support to ResNet #822

Merged
merged 3 commits into from
Apr 2, 2019
Merged

Conversation

fmassa
Copy link
Member

@fmassa fmassa commented Mar 28, 2019

This adds the necessary building blocks to add support for ResNeXt models in torchvision.

One thing worth discussion: should we let the user specify arbitrary number of channels in each layer (as is currently supported in BaseResNet), or just stick with the options that are necessary to cover ResNet / ResNeXt?

@fmassa fmassa requested a review from soumith March 28, 2019 18:00
@codecov-io
Copy link

Codecov Report

Merging #822 into master will decrease coverage by 0.04%.
The diff coverage is 92%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #822      +/-   ##
=========================================
- Coverage   52.45%   52.4%   -0.05%     
=========================================
  Files          35      35              
  Lines        3401    3408       +7     
  Branches      545     547       +2     
=========================================
+ Hits         1784    1786       +2     
- Misses       1485    1488       +3     
- Partials      132     134       +2
Impacted Files Coverage Δ
torchvision/models/resnet.py 83.1% <92%> (-0.58%) ⬇️
torchvision/transforms/transforms.py 82.84% <0%> (-0.68%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a61803f...0e1e363. Read the comment docs.

@ekagra-ranjan
Copy link
Contributor

ResNets and its variants find their place as a building block in many new models. So providing a flexibility in choosing the number of channels (besides choosing the layer configuration) would give more power and ease to users while constructing their own custom models. And since there is a relation between the number of channels and groups, the implementation is quite neat and will remind the user (experimenting the source code) about the relation proposed by the authors.

@fmassa
Copy link
Member Author

fmassa commented Apr 1, 2019

@ekagra-ranjan just to make sure I understand, were you advocating for passing the layers argument (as it was before, in the BaseResNet) or just the width_per_group ?

@ekagra-ranjan
Copy link
Contributor

I am supporting the BaseResNet implementation that you implemented earlier in the PR which has planes and layers arguments.

@fmassa
Copy link
Member Author

fmassa commented Apr 2, 2019

I'll be merging this as is for now to unblock a few other things, and I can add back BaseResNet in a follow-up implementation.

@fmassa fmassa merged commit ad0daff into pytorch:master Apr 2, 2019
@fmassa fmassa deleted the add-groups-resnet branch April 2, 2019 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants