[PT2E][Docs] Calibration is a must for both static and dynamic quantization#4307
[PT2E][Docs] Calibration is a must for both static and dynamic quantization#4307Xia-Weiwen merged 5 commits intopytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/4307
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: ❌ 1 New FailureAs of commit b93240d with merge base 67a78e5 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
Xia-Weiwen
added
module: not user facing
| # if the observer is uninitialized (empty min_val) and its input is a constant | ||
| # weight tensor, and run the observer eagerly in that case. | ||
| if ( | ||
| hasattr(activation_post_process, "min_val") |
There was a problem hiding this comment.
this condition is too ad hoc I think, won't work if activation_post_process is more general
There was a problem hiding this comment.
Oh. It checks if the observer has not been run and its argument is a constant (from getattr). It looks complicated but I did not figure out a better way. Do you have any suggestions 😂 Thanks.
There was a problem hiding this comment.
Hi @jerryzh168 Do you have any good ideas to simplify this or to fix the issue in another way? Thanks.
There was a problem hiding this comment.
what is the use case for dynamic quantization? why is just going through an example input for dynamic quant flow not good? I think we need example input for torch.export as well anyways
There was a problem hiding this comment.
I feel we could also just add some asserts saying observer should always run, or just make sure we have proper docs saying calibration step can't be skipped
There was a problem hiding this comment.
Hi @jerryzh168 Thanks for the suggestions. We already have a warning here:
ao/torchao/quantization/pt2e/utils.py
Line 148 in 67a78e5
So, I just add a comment in the docs to say calibration is a must for dynamic quantization. Please take a look.
Xia-Weiwen
added
the
topic: documentation
Xia-Weiwen
changed the title
|
Hi @jerryzh168 Could you please review again? Thanks. |
| m = prepare_pt2e(m, quantizer) | ||
|
|
||
| # run calibration | ||
| # calibration is a must for both static and dynamic quantization |
There was a problem hiding this comment.
nit: must run calibration for both static and dynamic quantization
jerryzh168
left a comment
jerryzh168
left a comment
There was a problem hiding this comment.
thanks, can change the comment to be a bit more natural
Updated. Thanks. |
2 participants
This pull request adds a note in the PT2E docs stating that the calibration is mandatory.