-
Notifications
You must be signed in to change notification settings - Fork 316
Make AWQ more general #2400
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
Make AWQ more general #2400
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/2400
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 8b229a7 with merge base ffaf572 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
torchao/prototype/awq/api.py
Outdated
eps=eps, | ||
) | ||
else: | ||
observer = AWQObserver2( |
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 not add kwargs to the AWQObserver and just check 'base_config' in kwargs
?
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.
yes, this is temporary, I think we can deprecate the old one in the end
|
||
|
||
@dataclass | ||
class AWQConfig(AOBaseConfig): |
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.
Ok this is consolidating with quantize_ api's config based design?
dummy_mod = DummyModule(observed_linear.weight * equalization_scale) | ||
quant_mod = base_config_handler(dummy_mod, config.base_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.
I am not sure whats happening here?. Isnt module
already nn.Module?
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.
this is just trying to quantize the weight with the quantization type specified by config.base_config
torchao/prototype/awq/api.py
Outdated
if config.set_inductor_config: | ||
torchao.quantization.utils.recommended_inductor_config_setter() | ||
|
||
observed_linear = module |
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.
If this is for linear only should you not assert that this nn.Linear? Plus how to you make sure this function is called only on nn.Linear?
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.
yeah that's true, will add an assert, we rely on user to use quantize_ correctly (it's through specifying the filter_fn arg in quantize_ API)
ao/torchao/quantization/quant_api.py
Line 578 in 4e3d019
filter_fn: Optional[Callable[[torch.nn.Module, str], bool]] = None, |
4d7eeb7
to
60f8852
Compare
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.
Changes look excellent!
7c91a04
to
9074308
Compare
Summary: * Added AWQConfig that takes a base config and made corresponding changes in other parts of the flow Test Plan: Tested on Phi4-mini and Qwen3-8B Qwen3-8B |Task | calibration_limit | no-awq | awq | |-----+------------------+ ------+ ------+ |leaderboard_math_hard (v3) | 2 | 0.3543 | 0.4371 | |gpqa_main_zeroshot | 50 | 0.32 | 0.36 | |mmlu | 5 | 0.7372 | 0.7463 | |bbh | 1 | 0.7385 | 0.7556| Phi4-mini | Task | calibration_limit | no-awq | awq | |------+------------------+--------+------| | mmlu_pro | 2 | 0.4057 | 0.4757 | | gsm8k | 5 | 0.72 | 0.76 | Reviewers: Subscribers: Tasks: Tags:
9074308
to
8b229a7
Compare
cc @Xia-Weiwen and @xiaowangintel we updated AWQ, please check if it still works for CPU and XPU |
Thanks. Will check it out later. |
TransformerEvalWrapper( | ||
model=model.to(device), | ||
tokenizer=tokenizer, | ||
max_seq_length=max_seq_length, | ||
device=device, | ||
).run_eval( | ||
tasks=tasks, | ||
limit=calibration_limit, | ||
) |
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 @jerryzh168 This part does not work with PPL
on my side. It seems that we should use wiki2_eval
for PPL
. How was it when you ran?
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 we can probably can remove PPL, I'm using lm-eval tasks to do evals
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.
here is the list of tasks you can use: https://github.com/EleutherAI/lm-evaluation-harness/blob/main/lm_eval/tasks/README.md
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.
OK. Perhaps we need to update the default value of --tasks
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.
Do you have a preference for the default value, i.e. the default task?
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.
I see, you can use hellaswag
I think
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.
Sure. Thanks.
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 @jerryzh168 I have created a PR for CPU: #2688 Please take a look. Thanks.
Summary:
in other parts of the flow
Test Plan:
Tested on Phi4-mini and Qwen3-8B
Qwen3-8B
Phi4-mini
Reviewers:
Subscribers:
Tasks:
Tags: