Skip to content

Update the qspec sharing logic for concat op - #153

Merged
aseemw merged 2 commits into
apple:mainfrom
aseemw:u/awadhwa/quantizer_fixes_2
Oct 23, 2026
Merged

aseemw merged 2 commits into
apple:mainfrom
aseemw:u/awadhwa/quantizer_fixes_2

Conversation

@aseemw

@aseemw aseemw commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

If a concat node gets inputs with per-channel quantization spec (along concat dim) , then disable qspec sharing. In other words, only allow for qspec sharing when the inputs/outputs use per-tensor or when the quantization axis is a different than concat_dim axis.

We would normally not hit this condition with activation quantization, where per-tensor is the default. However in certain models, the weights may also get concatenated, in which case the update logic would let different weights use different quantizers (instead of erroring out as it currently does on the main branch)

@anotheranshu anotheranshu 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.

I'd suggest some test case updates, but otherwise LGTM. One thing though. Your PR description says this:

If a concat node gets inputs with per-channel quantization spec (along concat dim) , then disable qspec sharing. In other words, only allow for qspec sharing when the inputs/outputs use per-tensor.

I think the second sentence (the "In other words" part) isn't quite the same as the first part. We still allow sharing across the concat when the input and output do per-channel in a different axis to the concat dimension, right? Want to make sure my understanding of the PR is the same as your intent.

Comment thread tests/quantization/test_annotation_pattern_registry.py Outdated
Comment thread tests/quantization/test_annotation_pattern_registry.py
@aseemw

aseemw commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

I'd suggest some test case updates, but otherwise LGTM. One thing though. Your PR description says this:

If a concat node gets inputs with per-channel quantization spec (along concat dim) , then disable qspec sharing. In other words, only allow for qspec sharing when the inputs/outputs use per-tensor.

I think the second sentence (the "In other words" part) isn't quite the same as the first part. We still allow sharing across the concat when the input and output do per-channel in a different axis to the concat dimension, right? Want to make sure my understanding of the PR is the same as your intent.

Sorry, it was a bit confusing, I have updated the PR description.

@aseemw
aseemw merged commit c61756d into apple:main Oct 23, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants