Qualcomm AI Engine Direct - Add rewrite function of observer
- Add function to rewrite observer after prepare_pt2e
- Add corresponding test case
:link: Helpful Links
:test_tube: See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/10093
- :page_facing_up: Preview Python docs built from this PR
Note: Links to docs will display an error until the docs builds have been completed.
:x: 1 New Failure
As of commit 3caeaab6615362dcc6af3898eaa3bc840930473f with merge base 95a1db5b797e4be6198cea2821069175fd9e61bd ():
NEW FAILURE - The following job has failed:
-
pull / unittest-arm-backend-with-no-fvp (test_pytest_models) / linux-job (gh)
RuntimeError: Command docker exec -t 820663bbe7e20c43cfa19c191f5a7c366181251ee3380a0333ff6db60064c3fa /exec failed with exit code 1
This comment was automatically generated by Dr. CI and updates every 15 minutes.
Hi @cccclai,
This PR is for the requested functionality to overwrite a quanization parameters after calibration. It can handle shared observer too. Thank you!
@chunit-quic thanks for putting up the pr. cc: @sxu @billmguo
Mind sharing a bit more details for the request? I probably miss it
Mind sharing a bit more details for the request? I probably miss it
No problem. :) We received some feature requests in a mail thread regarding quantization requirements. This particular request is for the following purpose:
The QNN Quantizer should allow users to override quantization parameters for specific tensors, regardless of the data ranges observed during calibration or QAT. This override must respect the transitive closures established by SharedQuantizationSpec.
After a brief discussion your team, we concluded that the rewriting stage should occur after calibration but before conversion. That's essentially the background.
Looks like this would work to me, however it would be great if someone from AO can confirm this kind of overwriting activation postprocessing submodule is enough and there's no other references to them.
Maybe @jerryzh168 can confirm?
@kirklandsign can you help checking the CI signal for this one too? I remember there are some errors.
Hey sorry for being late, can you rebase this PR?
Hey sorry for being late, can you rebase this PR?
Done! Thank you.
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Hey there some errors likely due to the pytorch pin update. Mind rebasing again? Sorry for the inconvinience
No problem. Just rebased. Pleasefel free to let me know if there is any issue. Thank you!
No problem. Just rebased. Pleasefel free to let me know if there is any issue. Thank you!
Did you rebase? Seems like there is no commit..
Ops, Sorry I rebased locally without push. Thank you for pointing out! Done.
Ops, Sorry I rebased locally without push. Thank you for pointing out! Done.
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
There seems to be a lint error, and that might be reason merging fails. Since https://github.com/pytorch/executorch/pull/11049 is landed, can you apply the according changes and see if the error is gone?
There seems to be a lint error, and that might be reason merging fails. Since #11049 is landed, can you apply the according changes and see if the error is gone?
Fixed. Thanks a lot for pointing out the error!
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
Mind sharing a bit more details for the request? I probably miss it
No problem. :) We received some feature requests in a mail thread regarding quantization requirements. This particular request is for the following purpose:
The QNN Quantizer should allow users to override quantization parameters for specific tensors, regardless of the data ranges observed during calibration or QAT. This override must respect the transitive closures established by SharedQuantizationSpec.
After a brief discussion your team, we concluded that the rewriting stage should occur after calibration but before conversion. That's essentially the background.
I dont quite follow why we cannot do this by allowing module specific customization? FOr example can I not say quanitzer.set_module_qconfig(....). And @jerryzh168 I really think we should make this part of the base class. Quantizers that dont implement this will fallback to baseclass which will just raise error.