[quant][pt2] Fix QAT conv-bn bias derived qspec #112159

andrewor14 · 2023-10-26T15:43:29Z

Stack from ghstack (oldest at bottom):

Summary: Today, we have special handling for special qspecs like
SharedQuantizationSpec or DerivedQuantizationSpec, since these
qspecs refer to other nodes in the graph and these node references
need to be updated after replacement (since they referred to nodes
in the original graph that no longer exist in the new graph).

However, we only do the above for special nodes like conv, bn,
getitem, and relu. This doesn't cover the common use case of
having conv bias derive its qparams from those of conv input
activations and conv weight. This commit adds support for this
use case by also replacing the node references for these nodes.

Test Plan:
python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec

Reviewers: jerryzh168, kimishpatel

Subscribers: jerryzh168, kimishpatel, supriyar

Differential Revision: D50697078

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar [ghstack-poisoned]

pytorch-bot · 2023-10-26T15:43:33Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/112159

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 72ec7e4 with merge base c120e56 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: f33e2564530d5d499f4e162db875304462d31431 Pull Request resolved: #112159

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar [ghstack-poisoned]

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: e376519dfba57f373eb1bd08c9329de73ba470f0 Pull Request resolved: #112159

andrewor14 · 2023-10-26T15:52:17Z

@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

torch/ao/quantization/pt2e/qat_utils.py

jerryzh168 · 2023-10-27T02:46:09Z

torch/ao/quantization/pt2e/qat_utils.py

    mapping = {
        "conv": (o_conv, r_conv),
+        "conv_input": (o_conv_input, r_conv_input),
+        "conv_weight": (o_conv_weight, r_conv_weight),
        "bn": (o_bn, r_bn),
        "getitem": (o_getitem, r_getitem),


just trying to make sure we have everything, this is output?

yeah, or the relu node below

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) [ghstack-poisoned]

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: 269e07ac2e9f02cdefa9122f498d63451e10aa48 Pull Request resolved: #112159

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) [ghstack-poisoned]

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: 235dcdc6f24598167661a722ca958390a9789664 Pull Request resolved: #112159

andrewor14 · 2023-10-27T15:32:26Z

@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) [ghstack-poisoned]

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: 76de9afb8051c7c016354ee2c55b42a7f9c7c403 Pull Request resolved: #112159

andrewor14 · 2023-10-27T19:03:55Z

@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) [ghstack-poisoned]

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: 59fba976c39b59e26f92aac9f0eebd94220d3701 Pull Request resolved: #112159

andrewor14 · 2023-10-27T19:47:51Z

@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

andrewor14 · 2023-10-27T20:03:24Z

@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: 59fba976c39b59e26f92aac9f0eebd94220d3701 Pull Request resolved: #112159

jerryzh168 · 2023-10-31T00:29:32Z

test/quantization/pt2e/test_quantize_pt2e_qat.py

+        m(*example_inputs)
+
+
+class ConvBnDerivedBiasQuantizer(Quantizer):


should we add a test for referring to bias input edge as well? e.g. qspec for weight input edge being shared with bias input edge

conv_annotation = {input_qspec_map={input: ..., weight: SharedQuantizationSpec(bias, conv_node), bias: ...}}

Ok. That is a bit separate so I think I'll add it in a future PR instead

jerryzh168

LG, maybe add a test for SharedQuantizationSpec of bias input edge as well

andrewor14 · 2023-10-31T14:20:37Z

@pytorchbot merge

pytorchmergebot · 2023-10-31T14:22:38Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

Summary: This commit refactors q-dq patterns used in QAT fusion, reducing code duplication. This is important for future efforts to support quantizing bias. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Pull Request resolved: #112279 Approved by: https://github.com/jerryzh168 ghstack dependencies: #112159

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar ghstack-source-id: f833648d7cf3a34b9c03b84f0132e18527c5d3c5 Pull Request resolved: pytorch/pytorch#112159

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) Pull Request resolved: pytorch#112159 Approved by: https://github.com/jerryzh168

Summary: This commit refactors q-dq patterns used in QAT fusion, reducing code duplication. This is important for future efforts to support quantizing bias. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Pull Request resolved: pytorch#112279 Approved by: https://github.com/jerryzh168 ghstack dependencies: pytorch#112159

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) Pull Request resolved: pytorch#112159 Approved by: https://github.com/jerryzh168

Summary: This commit refactors q-dq patterns used in QAT fusion, reducing code duplication. This is important for future efforts to support quantizing bias. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Pull Request resolved: pytorch#112279 Approved by: https://github.com/jerryzh168 ghstack dependencies: pytorch#112159

Summary: Today, we have special handling for special qspecs like `SharedQuantizationSpec` or `DerivedQuantizationSpec`, since these qspecs refer to other nodes in the graph and these node references need to be updated after replacement (since they referred to nodes in the original graph that no longer exist in the new graph). However, we only do the above for special nodes like conv, bn, getitem, and relu. This doesn't cover the common use case of having conv bias derive its qparams from those of conv input activations and conv weight. This commit adds support for this use case by also replacing the node references for these nodes. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT.test_qat_conv_bn_bias_derived_qspec Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Differential Revision: [D50697078](https://our.internmc.facebook.com/intern/diff/D50697078) Pull Request resolved: pytorch#112159 Approved by: https://github.com/jerryzh168

Summary: This commit refactors q-dq patterns used in QAT fusion, reducing code duplication. This is important for future efforts to support quantizing bias. Test Plan: python test/test_quantization.py TestQuantizePT2EQAT Reviewers: jerryzh168, kimishpatel Subscribers: jerryzh168, kimishpatel, supriyar Pull Request resolved: pytorch#112279 Approved by: https://github.com/jerryzh168 ghstack dependencies: pytorch#112159

andrewor14 requested a review from jerryzh168 as a code owner October 26, 2023 15:43

pytorch-bot bot added the release notes: quantization release notes category label Oct 26, 2023

jerryzh168 reviewed Oct 27, 2023

View reviewed changes

torch/ao/quantization/pt2e/qat_utils.py Show resolved Hide resolved

jerryzh168 reviewed Oct 27, 2023

View reviewed changes

andrewor14 requested a review from jerryzh168 October 27, 2023 19:47

andrewor14 mentioned this pull request Oct 27, 2023

[quant][pt2][be] Refactor QAT q-dq patterns #112279

Closed

jerryzh168 reviewed Oct 31, 2023

View reviewed changes

jerryzh168 approved these changes Oct 31, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 31, 2023

pytorchmergebot added the merging label Oct 31, 2023

pytorchmergebot added the Merged label Oct 31, 2023

pytorchmergebot closed this in 231129e Oct 31, 2023

pytorchmergebot removed the merging label Oct 31, 2023

andrewor14 mentioned this pull request Oct 31, 2023

[quant][pt2] Support quantized conv bias in QAT fusion #112528

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[quant][pt2] Fix QAT conv-bn bias derived qspec #112159

[quant][pt2] Fix QAT conv-bn bias derived qspec #112159

andrewor14 commented Oct 26, 2023 •

edited

pytorch-bot bot commented Oct 26, 2023 •

edited

andrewor14 commented Oct 26, 2023

jerryzh168 Oct 27, 2023 •

edited

andrewor14 Oct 27, 2023

andrewor14 commented Oct 27, 2023

andrewor14 commented Oct 27, 2023

andrewor14 commented Oct 27, 2023

andrewor14 commented Oct 27, 2023

jerryzh168 Oct 31, 2023 •

edited

andrewor14 Oct 31, 2023

jerryzh168 left a comment

andrewor14 commented Oct 31, 2023

pytorchmergebot commented Oct 31, 2023

		m(*example_inputs)


		class ConvBnDerivedBiasQuantizer(Quantizer):

[quant][pt2] Fix QAT conv-bn bias derived qspec #112159

[quant][pt2] Fix QAT conv-bn bias derived qspec #112159

Conversation

andrewor14 commented Oct 26, 2023 • edited

pytorch-bot bot commented Oct 26, 2023 • edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/112159

✅ No Failures

andrewor14 commented Oct 26, 2023

jerryzh168 Oct 27, 2023 • edited

Choose a reason for hiding this comment

andrewor14 Oct 27, 2023

Choose a reason for hiding this comment

andrewor14 commented Oct 27, 2023

andrewor14 commented Oct 27, 2023

andrewor14 commented Oct 27, 2023

andrewor14 commented Oct 27, 2023

jerryzh168 Oct 31, 2023 • edited

Choose a reason for hiding this comment

andrewor14 Oct 31, 2023

Choose a reason for hiding this comment

jerryzh168 left a comment

Choose a reason for hiding this comment

andrewor14 commented Oct 31, 2023

pytorchmergebot commented Oct 31, 2023

Merge started

andrewor14 commented Oct 26, 2023 •

edited

pytorch-bot bot commented Oct 26, 2023 •

edited

jerryzh168 Oct 27, 2023 •

edited

jerryzh168 Oct 31, 2023 •

edited