Migrate the quantizer to use aten ops directly #4195
Closed
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
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/4195
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 6d1694d with merge base fbe0af1 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
mcremon-meta
added a commit
that referenced
this pull request
Jul 10, 2024
Summary: Pull Request resolved: #4195 This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools. The motivation is that some of those do not preserve or propagate `source_fn_stack` information, resulting in quantization misses. SDPA is an example, where the underlying `bmm` ops cannot be quantized with `source_fn_stack` information alone, or MHA, which can hide its SDPA component and sometimes even `linear` ops depending on the model (see ViT for an example). Summary of the changes: - change the quantizer to match aten ops directly, through `node.target` - propagate required changes to the `QuantFusion` pass - update/remove existing patterns Differential Revision: D59552606
mcremon-meta
force-pushed
the
export-D59552606
branch
from
0519368
to
22d7280
Compare
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
mcremon-meta
added a commit
that referenced
this pull request
Jul 10, 2024
Summary: Pull Request resolved: #4195 This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools. The motivation is that some of those do not preserve or propagate `source_fn_stack` information, resulting in quantization misses. SDPA is an example, where the underlying `bmm` ops cannot be quantized with `source_fn_stack` information alone, or MHA, which can hide its SDPA component and sometimes even `linear` ops depending on the model (see ViT for an example). Summary of the changes: - change the quantizer to match aten ops directly, through `node.target` - propagate required changes to the `QuantFusion` pass - update/remove existing patterns Differential Revision: D59552606
mcremon-meta
force-pushed
the
export-D59552606
branch
from
22d7280
to
02f71f6
Compare
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
mcremon-meta
added a commit
that referenced
this pull request
Jul 10, 2024
Summary: Pull Request resolved: #4195 This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools. The motivation is that some of those do not preserve or propagate `source_fn_stack` information, resulting in quantization misses. SDPA is an example, where the underlying `bmm` ops cannot be quantized with `source_fn_stack` information alone, or MHA, which can hide its SDPA component and sometimes even `linear` ops depending on the model (see ViT for an example). Summary of the changes: - change the quantizer to match aten ops directly, through `node.target` - propagate required changes to the `QuantFusion` pass - update/remove existing patterns Differential Revision: D59552606
mcremon-meta
force-pushed
the
export-D59552606
branch
from
02f71f6
to
4542695
Compare
dulinriley
approved these changes
Jul 11, 2024
| # pyre-fixme[16]: Pyre doesn't get that CadenceQuantizer has a patterns attribute | ||
| patterns = [q.pattern for q in quantizer.quantizers] | ||
| patterns = [ | ||
| assert_is_instance(q, CadenceAtenQuantizer).pattern |
There was a problem hiding this comment.
Is this a duplicate and not properly stacked with
#4047?
You can use "gh-stack" to help with this in the future
| @@ -44,18 +47,20 @@ class PartitionAnchors: | |||
|
|
|||
| class QuantizationPattern(ABC): | |||
| @abstractmethod | |||
| def partition_types(self): | |||
| def partition_types(self) -> list[OpOverload]: | |||
There was a problem hiding this comment.
I think we need to support Python 3.8 here, which doesn't support list[x], and you need to use typing.List[x]
facebook-github-bot
pushed a commit
that referenced
this pull request
Jul 12, 2024
Summary: This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools. The motivation is that some of those do not preserve or propagate `source_fn_stack` information, resulting in quantization misses. SDPA is an example, where the underlying `bmm` ops cannot be quantized with `source_fn_stack` information alone, or MHA, which can hide its SDPA component and sometimes even `linear` ops depending on the model (see ViT for an example). Also note than in most cases, we match single nodes anyway, with a 1-1 mapping between the op (either nn.Module or nn.functional) and the aten op, so using the aten op directly is simply easier. Summary of the changes: - change the quantizer to match aten ops directly, through `node.target` - propagate required changes to the `QuantFusion` pass - update/remove existing patterns Reviewed By: dulinriley Differential Revision: D59552606
facebook-github-bot
force-pushed
the
export-D59552606
branch
from
4542695
to
f680897
Compare
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
facebook-github-bot
pushed a commit
that referenced
this pull request
Jul 15, 2024
Summary: This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools. The motivation is that some of those do not preserve or propagate `source_fn_stack` information, resulting in quantization misses. SDPA is an example, where the underlying `bmm` ops cannot be quantized with `source_fn_stack` information alone, or MHA, which can hide its SDPA component and sometimes even `linear` ops depending on the model (see ViT for an example). Also note than in most cases, we match single nodes anyway, with a 1-1 mapping between the op (either nn.Module or nn.functional) and the aten op, so using the aten op directly is simply easier. Summary of the changes: - change the quantizer to match aten ops directly, through `node.target` - propagate required changes to the `QuantFusion` pass - update/remove existing patterns Reviewed By: dulinriley Differential Revision: D59552606
facebook-github-bot
force-pushed
the
export-D59552606
branch
from
f680897
to
1fd5271
Compare
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
Summary: Pull Request resolved: #4195 This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools. The motivation is that some of those do not preserve or propagate `source_fn_stack` information, resulting in quantization misses. SDPA is an example, where the underlying `bmm` ops cannot be quantized with `source_fn_stack` information alone, or MHA, which can hide its SDPA component and sometimes even `linear` ops depending on the model (see ViT for an example). Also note than in most cases, we match single nodes anyway, with a 1-1 mapping between the op (either nn.Module or nn.functional) and the aten op, so using the aten op directly is simply easier. Summary of the changes: - change the quantizer to match aten ops directly, through `node.target` - propagate required changes to the `QuantFusion` pass - update/remove existing patterns Reviewed By: dulinriley Differential Revision: D59552606
|
This pull request was exported from Phabricator. Differential Revision: D59552606 |
mcremon-meta
force-pushed
the
export-D59552606
branch
from
1fd5271
to
6d1694d
Compare
|
This pull request has been merged in a22e809. |
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.
Summary:
This major change allows a lot more flexibility in the quantizer, and reduces the dependency on the decompositions/graph tracing tools.
The motivation is that some of those do not preserve or propagate
source_fn_stackinformation, resulting in quantization misses. SDPA is an example, where the underlyingbmmops cannot be quantized withsource_fn_stackinformation alone, or MHA, which can hide its SDPA component and sometimes evenlinearops depending on the model (see ViT for an example).Summary of the changes:
node.targetQuantFusionpassDifferential Revision: D59552606