-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ns for fx: support comparing fp32 vs fp32_prepared/int8, except shadowed #61129
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
Summary: Adds support the comparing fp32 model (without quantization) to a fp32 model prepared with quantization. The main missing feature was handling conv-bn fusion, since this fusion for PTQ happens outside of quantization patterns. Adds testing for this case for comparing weights and comparing activations Adds a TODO for also handling this for shadow activations, we need to first stop removing observers in graph passes before we can add this support, will be in a future PR. Test Plan: ``` python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2 python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2_qat python test/test_quantization.py TestFXNumericSuiteCoreAPIsModels.test_compare_activations_conv ``` Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
💊 CI failures summary and remediationsAs of commit b68e341 (more details on the Dr. CI page and at hud.pytorch.org/pr/61129): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 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. |
Summary: Adds support the comparing fp32 model (without quantization) to a fp32 model prepared with quantization. The main missing feature was handling conv-bn fusion, since this fusion for PTQ happens outside of quantization patterns. Adds testing for this case for comparing weights and comparing activations Adds a TODO for also handling this for shadow activations, we need to first stop removing observers in graph passes before we can add this support, will be in a future PR. Test Plan: ``` python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2 python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2_qat python test/test_quantization.py TestFXNumericSuiteCoreAPIsModels.test_compare_activations_conv ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 792bbd5 Pull Request resolved: #61129
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
… shadowed" Summary: Adds support the comparing fp32 model (without quantization) to a fp32 model prepared with quantization. The main missing feature was handling conv-bn fusion, since this fusion for PTQ happens outside of quantization patterns. Adds testing for this case for comparing weights and comparing activations Adds a TODO for also handling this for shadow activations, we need to first stop removing observers in graph passes before we can add this support, will be in a future PR. Test Plan: ``` python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2 python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2_qat python test/test_quantization.py TestFXNumericSuiteCoreAPIsModels.test_compare_activations_conv ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D29520009](https://our.internmc.facebook.com/intern/diff/D29520009) [ghstack-poisoned]
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
self.assert_ns_compare_dict_valid(results) | ||
# test both m vs mp and mp vs mq | ||
for m1, m2 in ((m, mp), (mp, mq)): | ||
results = extract_weights_fun('a', mp, 'b', mq) |
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 this be m1 and m2?
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.
yes, great catch, will fix that
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.
yes, great catch, will fix that
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 wonder what would be the use case for comparing fp32 and fp32_prepared? Is it for sanity check to make sure fp32 is numerically the same after prepare and helping debug the implementation of prepare function?
there was a use case discussed during design review to compare fp32 to quantized. This PR implements this functionality (once you can compare A to B and B to C, you can compare A to C). Let me update the summary to say this. |
…xcept shadowed" Summary: Adds support the comparing fp32 model (without quantization) to a fp32 model prepared with quantization or a quantized model. The main missing feature was handling conv-bn fusion, since this fusion for PTQ happens outside of quantization patterns. Adds testing for this case for comparing weights and comparing activations Adds a TODO for also handling this for shadow activations, we need to first stop removing observers in graph passes before we can add this support, will be in a future PR. Test Plan: ``` python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2 python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2_qat python test/test_quantization.py TestFXNumericSuiteCoreAPIsModels.test_compare_activations_conv ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D29520009](https://our.internmc.facebook.com/intern/diff/D29520009) [ghstack-poisoned]
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
…xcept shadowed" Summary: Adds support the comparing fp32 model (without quantization) to a fp32 model prepared with quantization or a quantized model. The main missing feature was handling conv-bn fusion, since this fusion for PTQ happens outside of quantization patterns. Adds testing for this case for comparing weights and comparing activations Adds a TODO for also handling this for shadow activations, we need to first stop removing observers in graph passes before we can add this support, will be in a future PR. Test Plan: ``` python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2 python test/test_quantization.py TestFXGraphMatcherModels.test_mobilenet_v2_qat python test/test_quantization.py TestFXNumericSuiteCoreAPIsModels.test_compare_activations_conv ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D29520009](https://our.internmc.facebook.com/intern/diff/D29520009) [ghstack-poisoned]
@vkuzo has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
This pull request has been merged in a70505c. |
Stack from ghstack:
Summary:
Adds support the comparing fp32 model (without quantization) to a
fp32 model prepared with quantization or a quantized model. The main missing feature was
handling conv-bn fusion, since this fusion for PTQ happens outside
of quantization patterns.
Adds testing for this case for comparing weights and comparing
activations
Adds a TODO for also handling this for shadow activations, we need to
first stop removing observers in graph passes before we can add
this support, will be in a future PR.
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: D29520009