Skip to content

Adapt sym quantizer to ET (#24875) - #24875

Open
mgiordy wants to merge 1 commit into
pytorch:mainfrom
mgiordy:export-D91777784
Open

mgiordy wants to merge 1 commit into
pytorch:mainfrom
mgiordy:export-D91777784

Conversation

@mgiordy

@mgiordy mgiordy commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary:

Summary

This diff includes a symmetric 8 bit quantizer in ET.

Differential Revision: D91777784

Copilot AI balanced review requested due to automatic review settings November 11, 2026 00:16
@pytorch-bot

pytorch-bot Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24875

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 618a0a9 with merge base 6411f1e (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 11, 2026
@linux-foundation-easycla

linux-foundation-easycla Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: mgiordy / name: Marco Giordano (618a0a9)

@meta-codesync

meta-codesync Bot commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

@mgiordy has exported this pull request. If you are a Meta employee, you can view the originating Diff in D91777784.

@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The review reports no active issues with the symmetric W8A32 quantization changes.

No open issues 🎉

What changed in this PR

This PR updates Cadence’s mixed W8A32 quantizer to use a symmetric signed 8-bit range of [-127, 127] for weights and biases while keeping activations in floating point.

Changes:

  • Adds the symmetric quantization specification and applies it to linear, convolution, and GRU patterns.
  • Adds annotation and range tests, removing the mixed quantizer’s annotation-test exclusion.
File Description
backends/​cadence/​aot/​tests/​test_quantizer_ops.py Adds W8A32 annotation checks and a symmetric weight-range assertion.
backends/​cadence/​aot/​quantizer/​quantizer.py Defines and applies the [-127, 127] configuration for mixed W8A32 quantization.

Review effort: Balanced

@meta-codesync meta-codesync Bot changed the title Adapt sym quantizer to ET Adapt sym quantizer to ET (#24875) Nov 12, 2026
Copilot AI balanced review requested due to automatic review settings November 12, 2026 00:24
mgiordy added a commit to mgiordy/executorch that referenced this pull request Nov 12, 2026
Summary:

# Summary
This diff includes a symmetric 8 bit quantizer in ET.

Differential Revision: D91777784

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The GRU test checks only the quantization spec type, not that b_hh shares with (b_ih, gru_node), so an incorrect bias-scale relationship could pass.

1 open issue

Review effort: Balanced

self.assertEqual(annotation.input_qspec_map.keys(), expected.keys())
for arg, qspec in expected.items():
if isinstance(qspec, type):
self.assertIsInstance(annotation.input_qspec_map[arg], qspec)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The tests cover only configuration and annotation, leaving converted and fused quantization values, scales, outputs, and GRU bias observer sharing unverified.

2 open issues

Review effort: Balanced

Comment on lines +1005 to +1008
def test_w8a32_weight_range_is_symmetric(self) -> None:
weight_qspec = qconfig_A32W8sym_127.weight
assert weight_qspec is not None
self.assertEqual((weight_qspec.quant_min, weight_qspec.quant_max), (-127, 127))
mgiordy added a commit to mgiordy/executorch that referenced this pull request Nov 12, 2026
Summary:

# Summary
This diff includes a symmetric 8 bit quantizer in ET.

Differential Revision: D91777784
Copilot AI balanced review requested due to automatic review settings November 12, 2026 19:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The GRU test does not verify that b_hh shares quantization parameters specifically with b_ih, allowing an incorrect sharing target to pass and silently alter fused GRU results.

2 open issues

Review effort: Balanced

Summary:

# Summary
This diff includes a symmetric 8 bit quantizer in ET.

Differential Revision: D91777784

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the mixed W8A32 symmetric-int8 range update.

2 open issues

Review effort: Balanced

This branch was previously deployed

1 inactive deployment
cadence — 618a0a96 Deployed Nov 13, 2026 by mgiordy via hifi-op-test / hifi4 #38132
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants