[Quant] Bug fix #107899
Closed
[Quant] Bug fix #107899
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
Summary: When two layers are quantized differently, observer map update updates map for key (observed_node, node), whereas it should really be (original_input, node) Test Plan: Test in the next diff adds a test where it otherwise fails Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/107899
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit b25f1b6 with merge base 0f1a225 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This was referenced Aug 19, 2023
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
jerryzh168
reviewed
Aug 24, 2023
| @@ -92,6 +92,16 @@ def _maybe_insert_input_observer_for_arg_or_kwarg( | |||
| new_obs_node = _insert_obs_or_fq( | |||
| arg, arg_as_input_act_obs_or_fq, model, named_modules, model.graph) # type: ignore[arg-type] | |||
| new_arg = new_obs_node | |||
| # When quantizing two layers with different configs we can have | |||
There was a problem hiding this comment.
can you add a test for this? surprised that this is not caught earlier..
There was a problem hiding this comment.
so didnt add separate test because the PR after this one fails for exactly this reason. Also we havent tested quantizing a different layers with different configs (which is what the next diff does). Hence did not catch it.
jerryzh168
approved these changes
Aug 25, 2023
Summary: When two layers are quantized differently, observer map update updates map for key (observed_node, node), whereas it should really be (original_input, node) Test Plan: Test in the next diff adds a test where it otherwise fails Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D48663145](https://our.internmc.facebook.com/intern/diff/D48663145) [ghstack-poisoned]
Summary: When two layers are quantized differently, observer map update updates map for key (observed_node, node), whereas it should really be (original_input, node) Test Plan: Test in the next diff adds a test where it otherwise fails Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D48663145](https://our.internmc.facebook.com/intern/diff/D48663145) [ghstack-poisoned]
Summary: When two layers are quantized differently, observer map update updates map for key (observed_node, node), whereas it should really be (original_input, node) Test Plan: Test in the next diff adds a test where it otherwise fails Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D48663145](https://our.internmc.facebook.com/intern/diff/D48663145) [ghstack-poisoned]
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
3 similar comments
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: When two layers are quantized differently, observer map update updates map for key (observed_node, node), whereas it should really be (original_input, node) Test Plan: Test in the next diff adds a test where it otherwise fails Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D48663145](https://our.internmc.facebook.com/intern/diff/D48663145) [ghstack-poisoned]
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@kimishpatel has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
pytorchmergebot
pushed a commit
that referenced
this pull request
Sep 2, 2023
Summary:
During convert step observers are first replaced by Q-DQ pair. In some
scenarios like following output DQ has a fan out.
---> OP2 -> Q -> DQ
/
OP -> Q -> DQ -
\
---> OP3 -> Q -> DQ
If either op OP2 or OP3 are configured to be quantized, then the input
is expected to quantized. In this case quantized equivalent of some
pattern, that quantizer asked to be quantized, should look like:
[DQ -> {pattern} -> Q]. However, in scenario like above where DQ node
is shared between multiple "quantized" patterns, boundary of "quantized"
pattern is not clear because DQ now belongs to multiple quantized
patterns.
This poses challenge for:
- Porting metadata: which "quantized" partition this DQ node belongs
- Quantized representation, equivalently, needs to identify
self-contained quantized pattern that is replaced by its equivalent pattern
that captures compute in the quantized precision.
Test Plan:
test_duplicate_dq_pass
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: [D48663147](https://our.internmc.facebook.com/intern/diff/D48663147)
Pull Request resolved: #107900
Approved by: https://github.com/jerryzh168, https://github.com/andrewor14, https://github.com/leslie-fang-intel
ghstack dependencies: #107105, #107106, #107899
pytorchmergebot
pushed a commit
that referenced
this pull request
Sep 2, 2023
…107107) Summary: This diff adds adding metadata to q-dq nodes by inferring the quatization intent from node annotations. Annotations on the node are way for user to specify how a node or subgraph is supposed to be quantized. We continue to use that information to copy metadata on Q/DQ node from appropriate nodes. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D48488416](https://our.internmc.facebook.com/intern/diff/D48488416) Pull Request resolved: #107107 Approved by: https://github.com/jerryzh168 ghstack dependencies: #107105, #107106, #107899, #107900
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 (oldest at bottom):
Summary:
When two layers are quantized differently, observer map update updates
map for key (observed_node, node), whereas it should really be
(original_input, node)
Test Plan:
Test in the next diff adds a test where it otherwise fails
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: D48663145