-
Notifications
You must be signed in to change notification settings - Fork 23k
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
BatchNorm2D #61012
BatchNorm2D #61012
Conversation
💊 CI failures summary and remediationsAs of commit ed4443b (more details on the Dr. CI page and at hud.pytorch.org/pr/61012):
2 failures not recognized by patterns:
Preview docs built from this PR This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions to the (internal) Dr. CI Users group. |
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@migeed-z has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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.
Looking good!
if n.op == 'call_module': | ||
op_type = getattr(self.traced, str(n.target)) | ||
if type(op_type) in _INFERENCE_RULES: | ||
return _INFERENCE_RULES[type(op_type)](n, op_type) |
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 should document somewhere what the schema for inference rules should be since now it's not just the Node
. For example, without reading this code, I don't know what the op_type
argument is supposed to be. It seems like it's actually the module instance rather than the type?
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.
Yeah it's the module in this case. I'll rename it and add documentation.
Differential Revision: [D29562337](https://our.internmc.facebook.com/intern/diff/D29562337) [ghstack-poisoned]
Differential Revision: [D29562337](https://our.internmc.facebook.com/intern/diff/D29562337) [ghstack-poisoned]
Differential Revision: [D29562337](https://our.internmc.facebook.com/intern/diff/D29562337) [ghstack-poisoned]
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
Differential Revision: [D29562337](https://our.internmc.facebook.com/intern/diff/D29562337) [ghstack-poisoned]
@migeed-z has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Stack from ghstack:
Differential Revision: D29562337