-
Notifications
You must be signed in to change notification settings - Fork 608
Qualcomm AI Engine Direct - Quantizer refine for qat #6747
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
Qualcomm AI Engine Direct - Quantizer refine for qat #6747
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6747
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: ✅ No FailuresAs of commit 844acda with merge base ecdc007 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
get_16a8w_qnn_ptq_config, | ||
get_default_16bit_qnn_ptq_config, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing get_default_16bit_qnn_ptq_config
causing internal failure...can we do it in a the following way
- This PR includes both new config and the old config
- I submit a PR internally to remove the old config call site
- You submit a new PR to remove the old config
Then we can land it safely...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Chen,
Thanks for pointing out what to fix.
It seems to fail again. Shuold I also add get_default_8bit_qnn_ptq_config
back ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry what is fail again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh hmm I feel like the Meta Internal-Only
check isn't very accurate...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem. Let me know what should be fix if any. :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me import and check CI again...
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Oh looks like some internal failure is resolved! There is one more left but I think it's already there. I'll submit a PR to fix that and then merge this change. |
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Hey can you rebase this change? I landed the internal PR... |
|
||
|
||
# TODO move to torch/ao/quantization/observer.py. | ||
class PerChannelParamObserver(UniformQuantizationObserverBase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some comments here as to why chose the min/max ranges in the way being done here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kimishpatel, just add the comment. If it's not clear enought please feel free to let me know. Thanks
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Hey wonder if you can rebase this PR, I've been failing to import this PR and the error is following
|
- Reorginize qualcomm/quantizer - Split quantizer/utils.py to -- qconfig -- annotators -- observers directory - Change coresponding callees - Rename get_default_Nbit_qnn_ptq_config to get_NaNw_qnn_ptq_config - Add 16a4w conv test* (It is not compared with original model)
- Move and rename param_observer.py to per_channel_param_observer.py - Add todo to merge qconfig
- Add todo for per_channel_param_observer.py
dd19c66
to
b353d4c
Compare
Just rebased. Thanks for pointing out. |
Thanks! There is still a small |
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
- Fix get_ptq_per_channel_quant_config not founded error
@cccclai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Follow the instruction to resubmit the PR after PR6513 is reverted.