Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: correct output scale compute #1077

Merged
merged 4 commits into from
Nov 15, 2024
Merged

Conversation

Giuseppe5
Copy link
Collaborator

@Giuseppe5 Giuseppe5 commented Oct 28, 2024

Reason for this PR

There is a bug when computing output scale factor, when the input scale is quantized per-row and the weights are quantized per channel

Changes Made in this PR

Resolves the issue by skipping scale factor computation in this case. This means that we cannot have a properly formed quant tensor with this quantization strategy.
In theory, if I am not hallucinating, the math would results in a per-element scale factor, but we need to decide whether it's worth keeping track of this since any operation with such QuantTensor would results in dequantize.

Testing Summary

None

Risk Highlight

NA

Checklist

  • Code comments added to any hard-to-understand areas, if applicable.
  • Changes generate no new warnings.
  • Updated any relevant tests, if applicable.
  • No conflicts with destination dev branch.
  • I reviewed my own code changes.
  • Initial CI/CD passing.
  • 1+ reviews given, and any review issues addressed and approved.
  • Post-review full CI/CD passing.

@Giuseppe5 Giuseppe5 requested review from nickfraser and removed request for nickfraser October 28, 2024 20:47
@Giuseppe5 Giuseppe5 requested review from nickfraser and removed request for nickfraser October 30, 2024 13:46
@Giuseppe5 Giuseppe5 added the next release PRs which should be merged for the next release label Nov 7, 2024
@Giuseppe5 Giuseppe5 merged commit c3208cf into Xilinx:dev Nov 15, 2024
373 of 374 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release PRs which should be merged for the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant