[quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input #45473

jerryzh168 · 2020-09-29T01:07:25Z

Stack from ghstack:

[quant][graphmode][fx][eagermode] Support sigmoid/hardsigmoid/tanh in eager and fx graph mode #45539 [quant][graphmode][fx][eagermode] Support sigmoid/hardsigmoid/tanh in eager and fx graph mode
[quant] Add FixedQParamsFakeQuantize module #45538 [quant] Add FixedQParamsFakeQuantize module
[quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input #45473 [quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input
[quant][graphmode][fx] Merge all quantization mode #45292 [quant][graphmode][fx] Merge all quantization mode
[quant] Use PlaceholderObserver as default dynamic quant observer #45343 [quant] Use PlaceholderObserver as default dynamic quant observer

Summary:
Currently we don't insert observer for the output, but we actually need to
use input observer for the output to be consistent with the transformation
we are doing.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D23981312

…herits quantization parameters for input Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

dr-ci · 2020-09-29T01:13:50Z

💊 CI failures summary and remediations

As of commit 133bcad (more details on the Dr. CI page):

10/10 failures possibly* introduced in this PR
- 3/10 non-CircleCI failure(s)---

7 failures not recognized by patterns:

Job	Step	Action
^{pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test}	^{Run tests}	🔁 rerun
^{pytorch_macos_10_13_py3_test}	^{Update Homebrew}	🔁 rerun
^{pytorch_linux_bionic_py3_6_clang9_test}	^{Run tests}	🔁 rerun
^{pytorch_linux_bionic_py3_8_gcc9_coverage_test}	^{Run tests}	🔁 rerun
^{pytorch_linux_xenial_py3_clang5_asan_test2}	^{Run tests}	🔁 rerun
^{pytorch_linux_xenial_py3_6_gcc5_4_ge_config_simple_test}	^{Run tests}	🔁 rerun
^{pytorch_linux_xenial_py3_6_gcc5_4_test}	^{Run tests}	🔁 rerun

🚧 6 fixed upstream failures:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

Since your merge base is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

Check out the recency history of this "viable master" tracking branch.

pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test on Sep 30 from 1:18pm to 5:03pm PDT (8 commits; ac9a708 - 869b056)
- 🔁 rerun
pytorch_linux_xenial_py3_clang5_asan_test2 on Sep 30 from 12:46pm to 5:03pm PDT (9 commits; ffd50b8 - 869b056)
- 🔁 rerun
pytorch_linux_bionic_py3_8_gcc9_coverage_test on Sep 30 from 1:18pm to 5:03pm PDT (8 commits; ac9a708 - 869b056)
- 🔁 rerun
pytorch_linux_bionic_py3_6_clang9_test on Sep 30 from 1:18pm to 5:03pm PDT (8 commits; ac9a708 - 869b056)
- 🔁 rerun
pytorch_linux_xenial_py3_6_gcc5_4_ge_config_simple_test on Sep 30 from 1:18pm to 5:03pm PDT (8 commits; ac9a708 - 869b056)
- 🔁 rerun
pytorch_linux_xenial_py3_6_gcc5_4_test on Sep 30 from 1:18pm to 5:03pm PDT (8 commits; ac9a708 - 869b056)
- 🔁 rerun

Extra GitHub checks: 1 failed

Failed: GitHub Actions - clang-tidy

ci.pytorch.org: 2 failed

Failed: pr/pytorch-linux-bionic-rocm3.8-py3.6
Failed: pr/pytorch-linux-bionic-rocm3.7-py3.6

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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 37 times.

…ops that inherits quantization parameters for input" Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23981312](https://our.internmc.facebook.com/intern/diff/D23981312) [ghstack-poisoned]

…herits quantization parameters for input Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: d4033c47e1fd672ada0d7b4018c157e4677dc6e9 Pull Request resolved: #45473

…ops that inherits quantization parameters for input" Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23981312](https://our.internmc.facebook.com/intern/diff/D23981312) [ghstack-poisoned]

…herits quantization parameters for input Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: eab591ce82e2fea5e6f54bb078d5a1bac0dfa8ab Pull Request resolved: #45473

…ops that inherits quantization parameters for input" Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23981312](https://our.internmc.facebook.com/intern/diff/D23981312) [ghstack-poisoned]

supriyar · 2020-09-30T00:17:20Z

torch/quantization/fx/quantize.py

                    env[node.name] = observed_graph.create_node('call_module', observer_name, (load_arg(node),), {})
+                    activation_post_process_name_map[node.name] = observer_name


Shouldn't this be self.activation_post_process?

oh this is storing the names of activation_post_process modules, since we need this information when we insert observer calls, and it's not stored anywhere else.

this can be a local variable since it's just needed when we process the graph

supriyar · 2020-09-30T00:18:03Z

torch/quantization/fx/quantize.py

+                    # use the same observer as input
+                    observed_input = node.args[0]
+                    if observed_input.name in activation_post_process_name_map:
+                        print('inserting observer of input', observed_input.target)


Extra print statement?

supriyar

Left couple comments that need to be addressed.

…ops that inherits quantization parameters for input" Summary: Currently we don't insert observer for the output, but we actually need to use input observer for the output to be consistent with the transformation we are doing. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23981312](https://our.internmc.facebook.com/intern/diff/D23981312) [ghstack-poisoned]

jerryzh168 · 2020-10-05T17:22:27Z

this is not needed anymore, we plan to observe the output for all the ops that inherits quantization parameters (e.g. average pool) and ops that currently have fixed quantization parameters (e.g. sigmoid).

This was referenced Sep 29, 2020

[quant][graphmode][fx] Support quantization for standalone module #44074

Closed

[quant] Use PlaceholderObserver as default dynamic quant observer #45343

Closed

[quant][graphmode][fx] Merge all quantization mode #45292

Closed

facebook-github-bot added the fx label Sep 29, 2020

This was referenced Sep 29, 2020

[quant] Add FixedQParamsFakeQuantize module #45538

Closed

[quant][graphmode][fx][eagermode] Support sigmoid/hardsigmoid/tanh in eager and fx graph mode #45539

Closed

supriyar reviewed Sep 30, 2020

View reviewed changes

supriyar approved these changes Sep 30, 2020

View reviewed changes

jerryzh168 mentioned this pull request Sep 30, 2020

[quantization]: Add support for quantization aware training for Leaky Relu and Sigmoid #45593

Closed

jerryzh168 added 2 commits September 30, 2020 15:16

jerryzh168 closed this Oct 5, 2020

jerryzh168 reopened this Oct 15, 2020

jerryzh168 closed this Oct 20, 2020

facebook-github-bot deleted the gh/jerryzh168/443/head branch November 20, 2020 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input #45473

[quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input #45473

jerryzh168 commented Sep 29, 2020 •

edited

dr-ci bot commented Sep 29, 2020 •

edited by facebook-github-bot

supriyar Sep 30, 2020

jerryzh168 Sep 30, 2020

jerryzh168 Sep 30, 2020

supriyar Sep 30, 2020

supriyar left a comment

jerryzh168 commented Oct 5, 2020

		env[node.name] = observed_graph.create_node('call_module', observer_name, (load_arg(node),), {})
		activation_post_process_name_map[node.name] = observer_name

[quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input #45473

[quant][graphmode][fx][fix] Fix observer insert logic for ops that inherits quantization parameters for input #45473

Conversation

jerryzh168 commented Sep 29, 2020 • edited

dr-ci bot commented Sep 29, 2020 • edited by facebook-github-bot

💊 CI failures summary and remediations

7 failures not recognized by patterns:

🚧 6 fixed upstream failures:

Extra GitHub checks: 1 failed

ci.pytorch.org: 2 failed

supriyar Sep 30, 2020

Choose a reason for hiding this comment

jerryzh168 Sep 30, 2020

Choose a reason for hiding this comment

jerryzh168 Sep 30, 2020

Choose a reason for hiding this comment

supriyar Sep 30, 2020

Choose a reason for hiding this comment

supriyar left a comment

Choose a reason for hiding this comment

jerryzh168 commented Oct 5, 2020

jerryzh168 commented Sep 29, 2020 •

edited

dr-ci bot commented Sep 29, 2020 •

edited by facebook-github-bot