[quant][graphmode][fx] Scope support for call_method in QuantizationTracer #50173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…racer Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
💊 CI failures summary and remediationsAs of commit 3e9560f (more details on the Dr. CI page):
ci.pytorch.org: 1 failedThis 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. This comment has been revised 32 times. |
jerryzh168
requested review from
vkuzo,
z-a-f,
jamesr66a,
raghuramank100 and
supriyar
and removed request for
vkuzo
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
jerryzh168
added a commit
that referenced
this pull request
Jan 7, 2021
…racer Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 811f46cb9cbbcab1d79b670f36f4987be7f3982b Pull Request resolved: #50173
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
jerryzh168
added a commit
that referenced
this pull request
Jan 7, 2021
…racer Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: f30902dc69be9c263d31a851b07f757efe5ac2e1 Pull Request resolved: #50173
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path and type of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to (module_path, module_type), which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
jerryzh168
added a commit
that referenced
this pull request
Jan 7, 2021
…racer Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to module path, which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 69417ef56dab0f487d68d262eccadaa857c8d069 Pull Request resolved: #50173
vkuzo
approved these changes
Jan 8, 2021
torch/quantization/fx/quantize.py
Outdated
| qconfig_dict: Any) -> None: | ||
| global_qconfig = qconfig_dict.get('', None) | ||
| qconfig_dict: Any, | ||
| node_name_to_scope: Dict[str, Tuple[str, Any]]) -> None: |
There was a problem hiding this comment.
any chance we can clarify Any further to the real type? Motivation: if someone other than the code author is debugging this, as written there is no way to deduce the type other than looking up this PR and tracing what's happening. If the type was specified, the code reader has a lower burden.
if it's challening to define the type for some reason, can we add a comment with an example value
jerryzh168
commented
Jan 8, 2021
| not isinstance(m, torch.nn.Sequential)) or \ | ||
| module_qualified_name in self.skipped_module_names or \ | ||
| type(m) in self.skipped_module_classes or \ | ||
| isinstance(m, _FusedModule) | ||
|
|
||
| def call_module(self, m: torch.nn.Module, forward: Callable[..., Any], args : Tuple[Any, ...], kwargs : Dict[str, Any]) -> Any: | ||
| module_qualified_name = self.path_of_module(m) |
There was a problem hiding this comment.
@jamesr66a this is copied from default call_module code, would it be a problem if the default code changes? do you think it's better to provide an API in call_module to allow this kind of extension?
There was a problem hiding this comment.
I think the future-proofed way to do this is to instantiate the guard and call super().call_module
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path and type of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to (module_path, module_type), which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
…antizationTracer" Summary: Previously we did not set the qconfig for call_method node correctly since it requires us to know the scope (module path and type of the module whose forward graph contains the node) of the node. This PR modifies the QuantizationTracer to record the scope information and build a map from call_method Node to (module_path, module_type), which will be used when we construct qconfig_map Test Plan: python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25818132](https://our.internmc.facebook.com/intern/diff/D25818132) [ghstack-poisoned]
Codecov Report
@@ Coverage Diff @@
## gh/jerryzh168/528/base #50173 +/- ##
==========================================================
- Coverage 80.68% 80.59% -0.09%
==========================================================
Files 1904 1904
Lines 206561 206587 +26
==========================================================
- Hits 166667 166508 -159
- Misses 39894 40079 +185 |
|
This pull request has been merged in f10e7aa. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Summary:
Previously we did not set the qconfig for call_method node correctly since it requires us to know
the scope (module path and type of the module whose forward graph contains the node) of the node. This
PR modifies the QuantizationTracer to record the scope information and build a map from call_method
Node to (module_path, module_type), which will be used when we construct qconfig_map
Test Plan:
python test/test_quantization.py TestQuantizeFx.test_qconfig_for_call_method
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: D25818132