Faster initialization of Inception family #2170
Conversation
torchvision/models/googlenet.py
Outdated
| raise ValueError('For fast initialization, init_weights (weights initialization) is set ' | ||
| 'to None. Pass init_weights = True or False.') |
There was a problem hiding this comment.
Instead of raising a ValueError (which would be a BC-breaking change), can you instead use a FutureWarning as warnings.warn(msg, FutureWarning) or something like that, and then set init_weights=True? This way users won't see their code failing right-away.
I would also change the message to something a bit more clear for the end-users. Maybe something like
The default weight initialization of GoogleNet will change in future releases of torchvision. If you wish to keep the old behavior (which leads to long initialization times due to scipy/scipy#11299), please set init_weight=True.
There was a problem hiding this comment.
Thanks for the review @fmassa. Actually I thought you were wishing some kind of exception.
forcing the users to be aware of it until they explicitly set the value to either True or False
We can definitely switch to using warning as at the current context it seems to be a better choice.
There was a problem hiding this comment.
@fmassa, One question If we set init_weights=True after giving the warning, will it make any difference, as the object construction will still take lots of time even where default weights initialization is not required.
As you said here, maybe we can set init_weights=False and ask them if
they need weights
please set init_weight=True
What do you suggest? Thanks.
There was a problem hiding this comment.
I think we should keep the init_weights default to True for now, because we need to give users some time to update their codes. If the behavior suddenly change without at least one release in the middle, it would not be a great experience.
There was a problem hiding this comment.
Yeah, sure!! I understand.
torchvision/models/inception.py
Outdated
| super(Inception3, self).__init__() | ||
| if inception_blocks is None: | ||
| inception_blocks = [ | ||
| BasicConv2d, InceptionA, InceptionB, InceptionC, | ||
| InceptionD, InceptionE, InceptionAux | ||
| ] | ||
| if init_weights is None: | ||
| raise ValueError('For fast initialization, init_weights (weights initialization) is set ' |
test/test_quantized_models.py
Outdated
| if name in ['inception_v3', 'googlenet']: | ||
| model = torchvision.models.quantization.__dict__[name](pretrained=False, quantize=True, init_weights=True) | ||
| else: | ||
| model = torchvision.models.quantization.__dict__[name](pretrained=False, quantize=True) |
There was a problem hiding this comment.
I believe those changes in the test are only needed because we currently raise an error, but if we instead send a warning this wouldn't be necessary, right?
There was a problem hiding this comment.
Yeah. For warning it's not required.
There was a problem hiding this comment.
If it's not required, maybe it might be worth removing this then, except if the tests are now much faster to run?
There was a problem hiding this comment.
That's not required. Removed.
test/test_models.py
Outdated
| if name in ['inception_v3', 'googlenet']: | ||
| model = models.__dict__[name](num_classes=50, init_weights=True) | ||
| else: | ||
| model = models.__dict__[name](num_classes=50) |
torchvision/models/googlenet.py
Outdated
| warnings.warn('The default weight initialization of GoogleNet will be changed in future releases of ' | ||
| 'torchvision. If you wish to keep the old behavior (which leads to long initialization times' | ||
| ' due to scipy/scipy#11299), please set init_weights=True.', FutureWarning) | ||
| init_weights = False |
There was a problem hiding this comment.
As I mentioned before, can you set it to True instead so that users don't have a change in behavior?
Codecov Report
@@ Coverage Diff @@
## master #2170 +/- ##
=========================================
- Coverage 0.48% 0.48% -0.01%
=========================================
Files 92 92
Lines 7442 7448 +6
Branches 1135 1138 +3
=========================================
Hits 36 36
- Misses 7393 7399 +6
Partials 13 13
Continue to review full report at Codecov.
|
This PR tries to fix #2166.
It tries to solve the slow object construction for GoogleNet & Inception_v3.
Now, the default argument for
init_weightsis set toNone. Users have to explicitly setinit_weights=TrueorFalsefor a new object, else it will raise an exception.