Repository navigation
Update the qspec sharing logic for concat op - #153
Conversation
anotheranshu
left a comment
There was a problem hiding this comment.
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. |
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)