Arm backend: Rework reporting of qspecs and qconfigs#19016
Arm backend: Rework reporting of qspecs and qconfigs#19016martinlsm merged 6 commits intopytorch:mainfrom
Conversation
When importing backends.cortex_m.quantizer.quantizer_reporter from backends.arm.quantizer.arm_quantizer_utils, a cyclic dependency chain is is formed. The problem is that quantizer_reporter triggers backends/cortex_m/quantizer/__init__.py when imported, which in turn has imports leading back to the Arm backend. To fix this problem, move quantizer_reporter to backends/cortex_m so it can be imported without forming any cycle. Signed-off-by: Martin Lindström <Martin.Lindstroem@arm.com> Change-Id: I757aa090d35f47bdce4d523b064217c3069adf41
_QuantizerReporterUserMixin was a duplicated class with the name QuantizerReporterUser. Remove the former to instead use the latter. Signed-off-by: Martin Lindström <Martin.Lindstroem@arm.com> Change-Id: Iafe28304ab1813a8f88f58e75fc9a277158e7773
The quantizer reporter logs the quantization config in a human-readable format. Prior to this patch, this was done with the help of a dict called `SUPPORTED_QCONFIGS`, which was defined in quantizer_reporter.py and populated by the user. This patch reworks this concept by instead adding a label attribute to `QuantizationConfig` that the reporter can use to print the config in a human-readable format. Signed-off-by: Martin Lindström <Martin.Lindstroem@arm.com> Change-Id: I38e80c9c3d57fb9d858119fe4281b713bf472475
The quantization reporter prints quantization specs in human-readable format. Prior to this patch, this was implemented such that quantizer_reporter.py defined a dict `SUPPORTED_QSPECS` which was populated by the user. This dict would map qspec objects to string representations. This patch removes this dict and instead modifies the helper function `_qspec_repr` to return a compact string representation based on the attributes of the qspec. Signed-off-by: Martin Lindström <Martin.Lindstroem@arm.com> Change-Id: I9ccd9127b8c332e7c30662be6986ccad4a38881f
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19016
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 3 New Failures, 3 Unrelated FailuresAs of commit 1076083 with merge base 6be4fb5 ( NEW FAILURES - The following jobs have failed:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot label ciflow/trunk |
|
@pytorchbot label "partner: arm" |
|
@pytorchbot label "release notes: none" |
There was a problem hiding this comment.
Pull request overview
This PR refactors quantization reporting for Arm/Cortex-M backends to avoid import cycles and to make qconfig/qspec reporting self-describing (via QuantizationConfig.label and attribute-based qspec formatting) rather than relying on external registration dicts.
Changes:
- Move
quantizer_reportertobackends/cortex_m/and update imports/build targets to eliminate a Cortex-M ↔ Arm cyclic dependency. - Replace qconfig/qspec “registry dict” reporting with
QuantizationConfig.labeland a newqspec_repr()helper for compact, attribute-based qspec formatting. - Update Arm/Cortex-M quantizers and tests to use the new reporter API (
QuantizerInfo.qconfig_label) and new string representations.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backends/cortex_m/test/misc/test_quantizer_reporter.py | Updates tests to use the relocated reporter module and asserts new qspec string formatting via qspec_repr(). |
| backends/cortex_m/quantizer_reporter.py | Relocates and refactors reporter: removes qspec/qconfig registries, adds qspec_repr(), and changes QuantizerInfo to use qconfig_label. |
| backends/cortex_m/quantizer/quantizer.py | Updates reporter import path to the new module location. |
| backends/cortex_m/quantizer/quantization_configs.py | Stops registering qspec/qconfig dicts; instead sets human-readable labels directly on config instances. |
| backends/cortex_m/quantizer/TARGETS | Removes per-subdir reporter target; depends on the new top-level //executorch/backends/cortex_m:quantizer_reporter. |
| backends/cortex_m/TARGETS | Adds a new Buck target exporting quantizer_reporter.py from the Cortex-M root. |
| backends/arm/quantizer/quantization_config.py | Adds optional label field to QuantizationConfig for debug/reporting. |
| backends/arm/quantizer/arm_quantizer_utils.py | Removes duplicated reporter mixin and switches to QuantizerReporterUser; updates qconfig labeling logic. |
| backends/arm/quantizer/arm_quantizer.py | Constructs and propagates meaningful label strings when creating quantization configs (instead of external registration). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR restructures Cortex-M/Arm quantization reporting to eliminate import cycles and to make qconfig/qspec reporting more self-describing (via labels and attribute-based qspec formatting).
Changes:
- Move
quantizer_reportertobackends/cortex_m/quantizer_reporter.pyand adjust Buck targets/imports accordingly. - Remove the Arm-side
_QuantizerReporterUserMixinin favor ofQuantizerReporterUser, and updateQuantizerInfoto use aqconfig_label. - Add
labeltoQuantizationConfigand rework qspec reporting to useqspec_repr()instead of user-populated registries.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| backends/cortex_m/test/misc/test_quantizer_reporter.py | Updates tests for new reporter module path and new qspec_repr() output (adds unit tests for representations). |
| backends/cortex_m/quantizer_reporter.py | Introduces qspec_repr(), renames QuantizerInfo field, and changes QuantizerReport to store QuantizerInfo directly. |
| backends/cortex_m/quantizer/quantizer.py | Updates import to the relocated QuantizerReporter. |
| backends/cortex_m/quantizer/quantization_configs.py | Removes reporter registry updates and sets label on predefined Cortex-M qconfigs. |
| backends/cortex_m/quantizer/TARGETS | Removes local quantizer_reporter library and depends on the new top-level cortex_m reporter target. |
| backends/cortex_m/TARGETS | Adds new Buck python_library target for quantizer_reporter.py. |
| backends/arm/quantizer/quantization_config.py | Adds optional label field to QuantizationConfig for debugging/visualization/reporting. |
| backends/arm/quantizer/arm_quantizer_utils.py | Removes duplicated reporter-user mixin; uses QuantizerReporterUser and reports qconfig_label from QuantizationConfig.label. |
| backends/arm/quantizer/arm_quantizer.py | Builds human-readable labels for produced quantization configs; removes old reporter registration logic. |
Comments suppressed due to low confidence (1)
backends/cortex_m/quantizer_reporter.py:53
- qspec_repr()'s QuantizationSpec formatting currently only includes dtype and (optional) quant_min/quant_max. This collapses distinct qspecs (e.g., per-tensor vs per-channel, different qscheme/ch_axis/is_dynamic) to the same string, which can make the reporter output misleading and less useful for debugging. Consider including qscheme (and ch_axis when per-channel) and possibly is_dynamic in the representation so different specs remain distinguishable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -357,59 +361,32 @@ def get_symmetric_a16w8_quantization_config( | |||
| is_qat=is_qat, | |||
| is_dynamic=is_dynamic, | |||
This PR contains four commits: 1. **Move quantizer_reporter out from cortex_m.quantizer** When importing `backends.cortex_m.quantizer.quantizer_reporter` from `backends.arm.quantizer.arm_quantizer_utils`, a cyclic dependency chain is is formed. The problem is that quantizer_reporter triggers `backends/cortex_m/quantizer/__init__.py` when imported, which in turn has imports leading back to the Arm backend. To fix this problem, move quantizer_reporter to backends/cortex_m so it can be imported without forming any cycle. 3. **Arm backend: Remove _QuantizerReporterUserMixin** _QuantizerReporterUserMixin was a duplicated class with the name QuantizerReporterUser. Remove the former to instead use the latter. 4. **Arm backend: Add label attribute to QuantizationConfig** The quantizer reporter logs the quantization config in a human-readable format. Prior to this patch, this was done with the help of a dict called `SUPPORTED_QCONFIGS`, which was defined in quantizer_reporter.py and populated by the user. This patch reworks this concept by instead adding a label attribute to `QuantizationConfig` that the reporter can use to print the config in a human-readable format. 5. **Arm backend: Rework reporting of qspecs** The quantization reporter prints quantization specs in human-readable format. Prior to this patch, this was implemented such that quantizer_reporter.py defined a dict `SUPPORTED_QSPECS` which was populated by the user. This dict would map qspec objects to string representations. This patch removes this dict and instead modifies the helper function `_qspec_repr` to return a compact string representation based on the attributes of the qspec. Signed-off-by: Martin Lindström <Martin.Lindstroem@arm.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This PR contains four commits:
Move quantizer_reporter out from cortex_m.quantizer
When importing
backends.cortex_m.quantizer.quantizer_reporterfrombackends.arm.quantizer.arm_quantizer_utils, a cyclic dependency chain isis formed. The problem is that quantizer_reporter triggers
backends/cortex_m/quantizer/__init__.pywhen imported, which in turn hasimports leading back to the Arm backend. To fix this problem, move
quantizer_reporter to backends/cortex_m so it can be imported without
forming any cycle.
Arm backend: Remove _QuantizerReporterUserMixin
_QuantizerReporterUserMixin was a duplicated class with the name
QuantizerReporterUser. Remove the former to instead use the latter.
Arm backend: Add label attribute to QuantizationConfig
The quantizer reporter logs the quantization config in a human-readable
format. Prior to this patch, this was done with the help of a dict
called
SUPPORTED_QCONFIGS, which was defined in quantizer_reporter.pyand populated by the user. This patch reworks this concept by instead
adding a label attribute to
QuantizationConfigthat the reporter canuse to print the config in a human-readable format.
Arm backend: Rework reporting of qspecs
The quantization reporter prints quantization specs in human-readable
format. Prior to this patch, this was implemented such that
quantizer_reporter.py defined a dict
SUPPORTED_QSPECSwhich waspopulated by the user. This dict would map qspec objects to string
representations. This patch removes this dict and instead modifies
the helper function
_qspec_reprto return a compact stringrepresentation based on the attributes of the qspec.
cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell