-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Adding multiweight support for googlenet prototype model #4813
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 for googlenet prototype model #4813
Conversation
💊 CI failures summary and remediationsAs of commit b42bdf6 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
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.
Just a few notes/clarifications:
@@ -25,43 +25,6 @@ | |||
_GoogLeNetOutputs = GoogLeNetOutputs | |||
|
|||
|
|||
def googlenet(pretrained: bool = False, progress: bool = True, **kwargs: Any) -> "GoogLeNet": |
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.
Literally just moving it to the bottom of the file so that we can use proper typing.
from .shufflenetv2 import * | ||
from .vgg import * |
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.
Just reordering stuff and adding googlenet in the list.
weights = GoogLeNetWeights.ImageNet1K_TheCodezV1 if kwargs.pop("pretrained") else None | ||
weights = GoogLeNetWeights.verify(weights) | ||
|
||
remove_aux_logits = False |
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.
Lots of workarounds required by this model. I would appreciate a close review to ensure the new logic is aligned with the previous one.
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! thanks for making the code more readable as well.
* Move model builder at the bottom of the file, so we can use proper typing. * Adding GoogLeNet with multi-weight support. * Simplify expression.
Fixes partially #4672
cc @datumbox @bjuncek