[quant][graphmode] insert_quant_dequant work with qconfig_dict #25127

jerryzh168 · 2019-08-23T21:40:54Z

Stack from ghstack:

[refactor] Remove unnecessary checks in InsertQuantDeQuantImpl #25370 [refactor] Remove unnecessary checks in InsertQuantDeQuantImpl
[quant][graphmode] Skip inserting observers for Tensors inside fused op #25281 [quant][graphmode] Skip inserting observers for Tensors inside fused op
[quant][graphmode] Integration tests for qconfig_dict #25217 [quant][graphmode] Integration tests for qconfig_dict
[quant][graphmode] insert_quant_dequant work with qconfig_dict #25127 [quant][graphmode] insert_quant_dequant work with qconfig_dict
[graphmode] insert_observers use qconfig_dict #25069 [graphmode] insert_observers use qconfig_dict

Summary:
Extend insert_quant_dequant pass to go through forward function graphs

Test Plan:

python test/test_jit.py 'TestJit.test_insert_quant_dequant'
python test/test_quantizer.py

Reviewers:
pt1quant
Subscribers:

Tasks:

Tags:

Differential Revision: D17001137

Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags:

…dict" Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags:

Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: ghstack-source-id: c58f347 Pull Request resolved: #25127

…dict" Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags:

Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: ghstack-source-id: bff14e6 Pull Request resolved: #25127

…dict" Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: Differential Revision: [D17001137](https://our.internmc.facebook.com/intern/diff/D17001137)

Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: ghstack-source-id: 6cfcf34 Pull Request resolved: #25127

…dict" Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: Differential Revision: [D17001137](https://our.internmc.facebook.com/intern/diff/D17001137)

ZolotukhinM

I think we should not duplicate the logic from insertObservers for figuring out what values we need to quantize. Instead we should just look for the corresponding CallMethod nodes and replace them with Q-DQ nodes. This logic was introduced earlier, but earlier it was slightly better "justified" than now.

torch/csrc/jit/passes/quantization.cpp

ZolotukhinM · 2019-08-28T17:51:20Z

torch/csrc/jit/passes/quantization.cpp

+
+} // namespace
+
+TORCH_API void InsertObservers(


Please avoid unnecessary code movements to make the diffs smaller. I'd suggest undoing this part of this PR.

oh the reason is I'm separating out a InsertObserversImpl function and put that into a anonymous namespace

Ah, ok. Please try to split such changes in future as they are not related to what the commit message says.

ZolotukhinM · 2019-08-28T17:54:33Z

torch/csrc/jit/passes/quantization.cpp


  for (Value* v : values_to_quantize) {
    if (v->type()->isSubtypeOf(TensorType::get())) {
+      if (v->node()->kind() == prim::CallMethod && v->node()->s(attr::name) == "forward") {


Do we need to replicate the logic of insertObservers in the loop above? We had to do it previously, but now we can just rely on having prim::CallMethod nodes and replace them with Q-DQ?

or should I remove these in a separate PR? that change seems unrelated actually

…dict" Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: Differential Revision: [D17001137](https://our.internmc.facebook.com/intern/diff/D17001137)

ZolotukhinM

Yeah, it's fine if you do it in a separate PR.

ZolotukhinM · 2019-08-28T22:38:45Z

torch/csrc/jit/passes/quantization.cpp

+
+} // namespace
+
+TORCH_API void InsertObservers(


Ah, ok. Please try to split such changes in future as they are not related to what the commit message says.

…dict" Summary: Extend insert_quant_dequant pass to go through forward function graphs Test Plan: ``` python test/test_jit.py 'TestJit.test_insert_quant_dequant' python test/test_quantizer.py ``` Reviewers: pt1quant Subscribers: Tasks: Tags: Differential Revision: [D17001137](https://our.internmc.facebook.com/intern/diff/D17001137)

ZolotukhinM · 2019-08-29T04:46:05Z

torch/csrc/jit/passes/quantization.cpp

-script::Module InsertQuantDeQuant(
-    script::Module& input_module,
+c10::optional<script::Module> QuantizeHelper::findChildModuleToQuantize(Value* v) {
+  if (v->node()->kind() == prim::CallMethod && v->node()->s(attr::name) == "forward") {


Nit: you could use early exits to avoid staircase indentation.

facebook-github-bot · 2019-08-29T07:08:41Z

This pull request has been merged in 96db3ad.

pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Aug 23, 2019

jerryzh168 mentioned this pull request Aug 27, 2019

[quant][graphmode] Integration tests for qconfig_dict #25217

Closed

jerryzh168 added 2 commits August 26, 2019 18:51

jerryzh168 requested review from ZolotukhinM and nishantpdce August 27, 2019 16:46

jerryzh168 added 2 commits August 27, 2019 09:52

jerryzh168 mentioned this pull request Aug 28, 2019

[quant][graphmode] Skip inserting observers for Tensors inside fused op #25281

Closed

jerryzh168 requested a review from apaszke as a code owner August 28, 2019 00:28

ZolotukhinM suggested changes Aug 28, 2019

View reviewed changes

jerryzh168 added 3 commits August 28, 2019 11:43

ZolotukhinM approved these changes Aug 28, 2019

View reviewed changes

jerryzh168 added 2 commits August 28, 2019 16:31

jerryzh168 mentioned this pull request Aug 29, 2019

[refactor] Remove unnecessary checks in InsertQuantDeQuantImpl #25370

Closed

facebook-github-bot closed this in 96db3ad Aug 29, 2019

ZolotukhinM reviewed Aug 29, 2019

View reviewed changes

facebook-github-bot added the merged label Aug 29, 2019

facebook-github-bot deleted the gh/jerryzh168/38/head branch October 28, 2019 22:15

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[quant][graphmode] insert_quant_dequant work with qconfig_dict #25127

[quant][graphmode] insert_quant_dequant work with qconfig_dict #25127

Uh oh!

jerryzh168 commented Aug 23, 2019 •

edited

Loading

Uh oh!

ZolotukhinM left a comment

Uh oh!

Uh oh!

ZolotukhinM Aug 28, 2019

Uh oh!

jerryzh168 Aug 28, 2019

Uh oh!

ZolotukhinM Aug 28, 2019

Uh oh!

ZolotukhinM Aug 28, 2019

Uh oh!

jerryzh168 Aug 28, 2019

Uh oh!

ZolotukhinM left a comment

Uh oh!

ZolotukhinM Aug 28, 2019

Uh oh!

ZolotukhinM Aug 29, 2019

Uh oh!

facebook-github-bot commented Aug 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[quant][graphmode] insert_quant_dequant work with qconfig_dict #25127

[quant][graphmode] insert_quant_dequant work with qconfig_dict #25127

Uh oh!

Conversation

jerryzh168 commented Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZolotukhinM left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ZolotukhinM Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

jerryzh168 Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

ZolotukhinM Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

ZolotukhinM Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

jerryzh168 Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

ZolotukhinM left a comment

Choose a reason for hiding this comment

Uh oh!

ZolotukhinM Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

ZolotukhinM Aug 29, 2019

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jerryzh168 commented Aug 23, 2019 •

edited

Loading