-
Notifications
You must be signed in to change notification settings - Fork 413
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
Refactor CadenceQuantizer #7540
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7540
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit a8dc686 with merge base 1bac885 (): This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This pull request was exported from Phabricator. Differential Revision: D67645196 |
Summary: The current class structure is hard to cleanly extend. This diff: - Makes `CadenceQuantizer` a base class - Creates a `CadenceDefaultQuantizer` that is exactly the same as the previous `CadenceQuantizer` class - Removes the qconfig from the `CadenceQuantizer`, since it really belongs to the `CadenceAtenQuantizer` (it is defined per op) - Makes both the default qconfig and the default quantizer list module level variables Using this structure will make it much cleaner to add new quantizers in the future. Differential Revision: D67645196
1a19d50
to
2092fec
Compare
This pull request was exported from Phabricator. Differential Revision: D67645196 |
2092fec
to
3947edf
Compare
Summary: The current class structure is hard to cleanly extend. This diff: - Makes `CadenceQuantizer` a base class - Creates a `CadenceDefaultQuantizer` that is exactly the same as the previous `CadenceQuantizer` class - Removes the qconfig from the `CadenceQuantizer`, since it really belongs to the `CadenceAtenQuantizer` (it is defined per op) - Makes both the default qconfig and the default quantizer list module level variables Using this structure will make it much cleaner to add new quantizers in the future. Differential Revision: D67645196
This pull request was exported from Phabricator. Differential Revision: D67645196 |
act_qspec, | ||
act_qspec, | ||
wgt_qspec, | ||
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.
why do we need None as a default?
Summary: The current class structure is hard to cleanly extend. This diff: - Makes `CadenceQuantizer` a base class - Creates a `CadenceDefaultQuantizer` that is exactly the same as the previous `CadenceQuantizer` class - Removes the qconfig from the `CadenceQuantizer`, since it really belongs to the `CadenceAtenQuantizer` (it is defined per op) - Makes both the default qconfig and the default quantizer list module level variables Using this structure will make it much cleaner to add new quantizers in the future. Reviewed By: zonglinpeng Differential Revision: D67645196
3947edf
to
a8dc686
Compare
This pull request was exported from Phabricator. Differential Revision: D67645196 |
Summary:
The current class structure is hard to cleanly extend. This diff:
CadenceQuantizer
a base classCadenceDefaultQuantizer
that is exactly the same as the previousCadenceQuantizer
classCadenceQuantizer
, since it really belongs to theCadenceAtenQuantizer
(it is defined per op)Using this structure will make it much cleaner to add new quantizers in the future.
Differential Revision: D67645196