-
Notifications
You must be signed in to change notification settings - Fork 287
[AWQ] modernized typehints, allow for n_grid/duo_scaling parameters #2003
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
Conversation
Signed-off-by: Brian Dellabetta <bdellabe@redhat.com>
|
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
Signed-off-by: Brian Dellabetta <bdellabe@redhat.com>
fynnsu
left a comment
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.
Looks good! In the future these should probably be two separate prs but changes make sense to me.
Signed-off-by: Brian Dellabetta <bdellabe@redhat.com>
Signed-off-by: Brian Dellabetta <bdellabe@redhat.com>
Signed-off-by: Brian Dellabetta <bdellabe@redhat.com>
fynnsu
left a comment
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.
Looks good!
SUMMARY:
This updates the AWQModifier type hints to python 3.10+, exposes n_grid as a parameter rather than the hard-coded 20, and allows the value of duo_scaling to be None, in which case half of the grid search is performed with duo_scaling off and the other half with duo_scaling on. This stems from @fynnsu 's findings that the previous default value of True was actually leading to worse behavior in some circumstances
TEST PLAN:
Run benchmarks with different values of n_grid and duo_scaling to see how much this affects accuracy/runtime