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

Moving common layers to ops #4504

Merged
merged 9 commits into from
Sep 30, 2021
Merged

Conversation

datumbox
Copy link
Contributor

@datumbox datumbox commented Sep 29, 2021

Partially resolves #4333

In this PR:

  • Move the commonly used _make_divisible() from MobileNetV2 to models._utils.
  • Deprecate the old ConvBNReLU and ConvBNActivation classes in favour of a new ConvNormActivation added in ops.misc.
  • Moved SqueezeExcitation to ops.misc and slightly modified the class to use only nn.Modules instead of functional.
  • Update RegNet implementation.
  • Validate that the accuracy of all affected models remains the same: https://www.internalfb.com/phabricator/paste/view/P460968703

@datumbox datumbox marked this pull request as draft September 29, 2021 16:14
Copy link
Contributor Author

@datumbox datumbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighting the interesting parts of the implementation

torchvision/models/efficientnet.py Show resolved Hide resolved
torchvision/models/mobilenetv2.py Show resolved Hide resolved
torchvision/models/mobilenetv2.py Show resolved Hide resolved
torchvision/ops/misc.py Show resolved Hide resolved
torchvision/ops/misc.py Show resolved Hide resolved
torchvision/ops/misc.py Show resolved Hide resolved
@datumbox datumbox marked this pull request as ready for review September 29, 2021 18:06
@oke-aditya
Copy link
Contributor

oke-aditya commented Sep 29, 2021

Can we move stochastic depth from ops.boxes to ops.misc.py ? I think we should avoid making it public until we decide about #4333 ?
Edit:
If we do decide to move, we would need to remove docs of it and not expose it in init.

@datumbox
Copy link
Contributor Author

@oke-aditya thanks for the comment.

Note that the StochasticDepth layer lives in its own module at ops.stochastic_depth. Including stuff on ops.misc doesn't make them less public AFAIK.

The historical convention for where to include things seems to be that layers with functional and class components end up on their own modules while those that are simple and use standard building convolutional blocks end up in ops.misc.

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@datumbox datumbox changed the title [WIP] Moving common layers on ops. Moving common layers on ops. Sep 30, 2021
@datumbox datumbox changed the title Moving common layers on ops. Moving common layers to ops Sep 30, 2021
@datumbox datumbox merged commit f749835 into pytorch:main Sep 30, 2021
@datumbox datumbox deleted the models/op_refactoring branch September 30, 2021 09:14
facebook-github-bot pushed a commit that referenced this pull request Oct 1, 2021
Summary:
* Moving _make_divisible to utils.

* Replace the old ConvBNReLU and ConvBNActivation layers

* Fix minor bug.

* Moving SE layer to ops.

* Adding deprecation warnings on old layers.

* Apply changes to regnets.

Reviewed By: prabhat00155, NicolasHug

Differential Revision: D31309549

fbshipit-source-id: 2780783ddfeb58974829607ac90f122b915f7366
mszhanyi pushed a commit to mszhanyi/vision that referenced this pull request Oct 19, 2021
* Moving _make_divisible to utils.

* Replace the old ConvBNReLU and ConvBNActivation layers

* Fix minor bug.

* Moving SE layer to ops.

* Adding deprecation warnings on old layers.

* Apply changes to regnets.
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
* Moving _make_divisible to utils.

* Replace the old ConvBNReLU and ConvBNActivation layers

* Fix minor bug.

* Moving SE layer to ops.

* Adding deprecation warnings on old layers.

* Apply changes to regnets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] API For Common Layers In Torchvision
4 participants