-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Adding multiweight support to SSD #4881
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
Conversation
💊 CI failures summary and remediationsAs of commit 19bb0f7 (more details on the Dr. CI page):
1 failure not recognized by patterns:
This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
datumbox
left a comment
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.
Adding few comments to assist review:
| weights = SSD300VGG16Weights.verify(weights) | ||
| if "pretrained_backbone" in kwargs: | ||
| warnings.warn("The argument pretrained_backbone is deprecated, please use weights_backbone instead.") | ||
| weights_backbone = VGG16Weights.ImageNet1K_Features if kwargs.pop("pretrained_backbone") else None |
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.
We are using special weights here. Not the standard VGG weights of TorchVision.
| "image_mean": [0.48235, 0.45882, 0.40784], | ||
| "image_std": [1.0 / 255.0, 1.0 / 255.0, 1.0 / 255.0], # undo the 0-1 scaling of toTensor | ||
| } | ||
| kwargs: Any = {**defaults, **kwargs} |
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.
Any type declaration needed to mypy.
| # We port the features of a VGG16 backbone trained by amdegroot because unlike the one on TorchVision, it uses the | ||
| # same input standardization method as the paper. Only the `features` weights have proper values, those on the | ||
| # `classifier` module are filled with nans. | ||
| ImageNet1K_Features = WeightEntry( |
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.
We are adding the weights as a separate entry on the VGG. This one only has the features part of the model.
| "interpolation": InterpolationMode.BILINEAR, | ||
| "recipe": "https://github.com/amdegroot/ssd.pytorch#training-ssd", | ||
| "acc@1": float("nan"), | ||
| "acc@5": float("nan"), |
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.
It doesn't contain a classifier, hence the nans here to denote it.
jdsgomes
left a comment
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, just left a minor comment for consideration
| "image_std": [1.0 / 255.0, 1.0 / 255.0, 1.0 / 255.0], # undo the 0-1 scaling of toTensor | ||
| } | ||
| kwargs: Any = {**defaults, **kwargs} | ||
| model = SSD(backbone, anchor_generator, (300, 300), num_classes, **kwargs) |
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.
should we get the size from the weights?
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.
It's the same discussion as in #4875 (comment).
Note that unlike the FasterRCNN models that adapt better on variable input size, SSD doesn't. It was investigated at #3819 and that's why it's hardcoded.
jdsgomes
left a comment
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.
Thanks
Reviewed By: datumbox Differential Revision: D32298973 fbshipit-source-id: e74e8cfc564f13681466a4006a3a879731b597ec
Fixes partially #4675
Verified that both commands return the same:
cc @datumbox @bjuncek