-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Adding multiweight support to Quantized InceptionV3 #4850
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
Adding multiweight support to Quantized InceptionV3 #4850
Conversation
💊 CI failures summary and remediationsAs of commit bc51d53 (more details on the Dr. CI page):
1 failure not recognized by patterns:
1 job timed out:
This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
beefef4
to
aa1adfb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few comments to assist review:
progress: bool = True, | ||
quantize: bool = False, | ||
**kwargs: Any, | ||
) -> "QuantizableInception3": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving the builder on the bottom of the page to use proper typing.
progress: bool = True, | ||
quantize: bool = False, | ||
**kwargs: Any, | ||
) -> QuantizableInception3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No review, just copy-paste.
@@ -28,11 +28,11 @@ class Inception3Weights(Weights): | |||
) | |||
|
|||
|
|||
def inception_v3(weights: Optional[Inception3Weights] = None, progress: bool = True, **kwargs: Any) -> Inception3: | |||
def inception_v3(weights: Optional[InceptionV3Weights] = None, progress: bool = True, **kwargs: Any) -> Inception3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming is inconsistent. The model class is called Inception3
, while the mode builder inception_v3. It's unclear how we should name the weights. Following the convention from ResNets this should have been Inception_V3
which is ugly. I propose not to spend more time on this now and tackle it at #4652. I've already added a reference for this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree! Lets tackle it in the cleanup
quantize_model(model, backend) | ||
|
||
if weights is not None: | ||
if quantize and not original_aux_logits: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Special handling needed here. See original implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -28,11 +28,11 @@ class Inception3Weights(Weights): | |||
) | |||
|
|||
|
|||
def inception_v3(weights: Optional[Inception3Weights] = None, progress: bool = True, **kwargs: Any) -> Inception3: | |||
def inception_v3(weights: Optional[InceptionV3Weights] = None, progress: bool = True, **kwargs: Any) -> Inception3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree! Lets tackle it in the cleanup
Summary: * Moving builder to the bottom to use proper typing. * Renaming weights. * Adding quantizated inception builder. * Correct meta info. * Fix linter. * Removing init_weights to avoid exposing it on the class. Reviewed By: kazhang Differential Revision: D32216665 fbshipit-source-id: 11dcacf95db067ab08a93408600f2b01e1b7367b
* Moving builder to the bottom to use proper typing. * Renaming weights. * Adding quantizated inception builder. * Correct meta info. * Fix linter. * Removing init_weights to avoid exposing it on the class.
Fixes partially #4674
Verified with:
cc @bjuncek