[WIP] Move XNNPACKQuantizer from PyTorch to ExecuTorch #144940
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/144940
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 5a7bfc9 with merge base b2c89bc ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
This pull request was exported from Phabricator. Differential Revision: D68191752 |
There was a problem hiding this comment.
cc @kimishpatel feels like reference representation is not really used, should we delete?
There was a problem hiding this comment.
WHy is this test being deleted entirely though? I am guessing because xnnpack quantizer is moving hence? I would have to think a bit on removing reference representation, and we should discuss. Potentially we remove those in favor providing an API that will use customer defined representation
There was a problem hiding this comment.
yeah this test is moved to executorch I think. sounds good, custom defined representation makes sense as well
There was a problem hiding this comment.
I am thinking of adding a dummy_quantizer in the test dir to run these tests especially qat ones.
| @@ -31,7 +31,7 @@ def generate_numeric_debug_handle(ep: ExportedProgram) -> None: | |||
| generate_numeric_debug_handle(ep) | |||
|
|
|||
| m = ep.module() | |||
| quantizer = XNNPACKQuantizer() | |||
| quantizer = XNNPACKQuantizer() # In the ExecuTorch repo. | |||
There was a problem hiding this comment.
please add an import for this
There was a problem hiding this comment.
what do you mean? This is an example, do you mean to put import line in addition to the comment in the doc string?
There was a problem hiding this comment.
yeah.. just to make it clearer since it's in executorch now
| @@ -64,6 +58,73 @@ | |||
| "get_x86_inductor_linear_dynamic_fp16_config", | |||
| ] | |||
|
|
|||
| # In the absence of better name, just winging it with QuantizationConfig | |||
| @dataclass(eq=True, frozen=True) | |||
| class QuantizationConfig: | |||
There was a problem hiding this comment.
we can probably put these in quantizer/utils.py since multiple quantizers are using it, but should be clear that there is no requirement of quantizers using them
I remember we discussed a bit before @kimishpatel and you were pushing for having these as utils, I feel it's fine to have them in utils as long as we are clear that it's more of a convenience function and not required to use now
There was a problem hiding this comment.
I can move them, but IIRC no one else is using it so kept it here.
docs/source/conf.py
Outdated
| @@ -359,7 +359,6 @@ | |||
| "prepare_qat_pt2e", | |||
| # torch.ao.quantization.quantizer.embedding_quantizer | |||
| "get_embedding_operators_config", | |||
| # torch.ao.quantization.quantizer.xnnpack_quantizer_utils | |||
There was a problem hiding this comment.
Everything below this comment until the next comed from this namespace. So you can delete all these lines as well
|
|
||
| __all__ = [ | ||
| "get_embedding_operators_config", | ||
| "EmbeddingQuantizer", | ||
| ] | ||
|
|
||
| # In the absence of better name, just winging it with QuantizationConfig | ||
| @dataclass(eq=True, frozen=True) |
There was a problem hiding this comment.
These need to be added to the __all__ block above and be properly documented on the doc website if you want to move them here as public API.
|
This pull request was exported from Phabricator. Differential Revision: D68191752 |
76818cb to
a175859
Compare
Summary: Pull Request resolved: pytorch#144940 X-link: pytorch/ao#1572 This migrates XNNPACKQuantizer from PyTorch to ExecuTorch. Rationale: Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent. Other impact and considerations: - PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, after talking with jerryzh168 - this diff moves most of the tests to ExecuTorch/backends/xnnpack/test/quantizer. - Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch. Differential Revision: D68191752
|
This pull request was exported from Phabricator. Differential Revision: D68191752 |
a175859 to
6955b5f
Compare
efd0614 to
13be4ac
Compare
Summary: Pull Request resolved: pytorch#7804 X-link: pytorch/pytorch#144940 This migrates XNNPACKQuantizer from PyTorch to ExecuTorch. Rationale: Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent. Other impact and considerations: - PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, we will keep the torch.ao.quantization.xnnpack_quantizer as is but mark is as not BC, and deprecated to discourace future new dependencies on it. - Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch. Reviewed By: mcr229 Differential Revision: D68191752
13be4ac to
a401071
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68191752 |
Summary: Pull Request resolved: pytorch#7804 X-link: pytorch/pytorch#144940 This migrates XNNPACKQuantizer from PyTorch to ExecuTorch. Rationale: Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent. Other impact and considerations: - PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, we will keep the torch.ao.quantization.xnnpack_quantizer as is but mark is as not BC, and deprecated to discourace future new dependencies on it. - Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch. Reviewed By: mcr229 Differential Revision: D68191752
|
This pull request was exported from Phabricator. Differential Revision: D68191752 |
Summary: X-link: pytorch/executorch#7804 Pull Request resolved: pytorch#144940 This migrates XNNPACKQuantizer from PyTorch to ExecuTorch. Rationale: Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent. Other impact and considerations: - PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, we will keep the torch.ao.quantization.xnnpack_quantizer as is but mark is as not BC, and deprecated to discourace future new dependencies on it. - Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch. Test Plan: CI Reviewed By: mcr229 Differential Revision: D68191752
a401071 to
4698bda
Compare
Summary: X-link: pytorch/executorch#7804 Pull Request resolved: pytorch#144940 This migrates XNNPACKQuantizer from PyTorch to ExecuTorch. Rationale: Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent. Other impact and considerations: - PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, we will keep the torch.ao.quantization.xnnpack_quantizer as is but mark is as not BC, and deprecated to discourace future new dependencies on it. - Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch. Test Plan: CI Reviewed By: mcr229 Differential Revision: D68191752
Summary: Pull Request resolved: pytorch#7804 X-link: pytorch/pytorch#144940 This migrates XNNPACKQuantizer from PyTorch to ExecuTorch. Rationale: Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent. Other impact and considerations: - PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, we will keep the torch.ao.quantization.xnnpack_quantizer as is but mark is as not BC, and deprecated to discourace future new dependencies on it. - Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch. Reviewed By: mcr229 Differential Revision: D68191752
|
This pull request was exported from Phabricator. Differential Revision: D68191752 |
4698bda to
5a7bfc9
Compare
|
@pytorchbot merge (Initiating merge automatically since Phabricator Diff has merged) |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Summary:
This replicates XNNPACKQuantizer from PyTorch to ExecuTorch.
Rationale:
Main motivation is to avoid pytorch pin update in OSS after updating XNNPACKQuantizer, which can be rather frequent.
Other impact and considerations:
PT2e flow (which lives in PyTorch) relies havily on XNNPACKQuantizer for a "example" implementation for quantizer and more importantly tests. Fow now, we will keep the torch.ao.quantization.xnnpack_quantizer as is but mark is as not BC, and deprecated to discourace future new dependencies on it.
Other OSS repository using XNNPACKQuantizer from PyTorch now have to take an additional dependency on ExecuTorch.
Differential Revision: D68191752
cc @albanD @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames