allow user to define residual settings #965
Conversation
Codecov Report
@@ Coverage Diff @@
## master #965 +/- ##
==========================================
- Coverage 60.03% 60.01% -0.02%
==========================================
Files 64 64
Lines 5054 5057 +3
Branches 754 756 +2
==========================================
+ Hits 3034 3035 +1
- Misses 1817 1818 +1
- Partials 203 204 +1
Continue to review full report at Codecov.
|
torchvision/models/mobilenet.py
Outdated
| @@ -117,7 +111,21 @@ def mobilenet_v2(pretrained=False, progress=True, **kwargs): | |||
| pretrained (bool): If True, returns a model pre-trained on ImageNet | |||
| progress (bool): If True, displays a progress bar of the download to stderr | |||
| """ | |||
| model = MobileNetV2(**kwargs) | |||
| default_setting = [ | |||
There was a problem hiding this comment.
Would it be simpler to do
if inverted_residual_setting is None:
inverted_residual_setting = ...?
There was a problem hiding this comment.
Thank you for the suggestion i have changed it that way
torchvision/models/mobilenet.py
Outdated
| @@ -50,21 +50,15 @@ def forward(self, x): | |||
|
|
|||
|
|
|||
| class MobileNetV2(nn.Module): | |||
| def __init__(self, num_classes=1000, width_mult=1.0): | |||
| def __init__(self, inverted_residual_setting, num_classes=1000, width_mult=1.0): | |||
There was a problem hiding this comment.
This is a backwards incompatible change, because users might have been passing MobileNetV2(100), and this will now fail.
But moving it to the end will make it need to have a default value, which implies we will need to add a default value here as well with
if inverted_residual_setting is None:
inverted_residual_setting = ...I think we should try to keep backwards-compatibility, to avoid breakages on the user land.
There was a problem hiding this comment.
you are right, i have changed it to be default param
torchvision/models/mobilenet.py
Outdated
| @@ -108,7 +102,7 @@ def forward(self, x): | |||
| return x | |||
|
|
|||
|
|
|||
| def mobilenet_v2(pretrained=False, progress=True, **kwargs): | |||
| def mobilenet_v2(pretrained=False, progress=True, inverted_residual_setting=None, **kwargs): | |||
There was a problem hiding this comment.
Can you add another test passing a different configuration, with fewer lines in the inverted_residual_setting?
There was a problem hiding this comment.
added a test in test_models
This PR adds support to allow user define inverted_residual_setting.
It will use default setting if user did not specified setting, use specified otherwise.