Some fixes for AWQ #269
Some fixes for AWQ #269
Conversation
| @@ -199,6 +199,18 @@ def is_preset_scheme(name: str) -> bool: | |||
| ), | |||
| ) | |||
|
|
|||
| # AWQ quantization | |||
| AWQ = dict( | |||
There was a problem hiding this comment.
Not needed; just nice to have a preset, easier to define a scheme in QuantizationModifier than specifying QuantizationArgs over and over
There was a problem hiding this comment.
I talked to rahul about this, we can either add a "W4A16_ASYMMETRIC" preset or remove entirely. I am not a big fan of presets here, this seems like something we want the user to be explicit about, but we have a bunch of them. what do people think?
There was a problem hiding this comment.
In general I'm in factor of creating generic and composable schemes rather than creating paper-specific presets. I'd rather remove this
There was a problem hiding this comment.
Couldn't we just do this, and create an alias?
I think W4A16_asymmetric makes sense with an AWQ alias
There was a problem hiding this comment.
Moved to W4A16_ASYM preset, leaving off "AWQ" alias for now since it's orthogonal to AWQ, just what we've been using for lining up with the paper's table 4.
| @@ -199,6 +199,18 @@ def is_preset_scheme(name: str) -> bool: | |||
| ), | |||
| ) | |||
|
|
|||
| # AWQ quantization | |||
| AWQ = dict( | |||
There was a problem hiding this comment.
I talked to rahul about this, we can either add a "W4A16_ASYMMETRIC" preset or remove entirely. I am not a big fan of presets here, this seems like something we want the user to be explicit about, but we have a bunch of them. what do people think?
| scales = torch.clamp(scales, min=torch.finfo(torch.float32).eps) | ||
| scales = torch.clamp(scales, min=1e-5) |
There was a problem hiding this comment.
we pulled this to match up with AutoAWQ's pseudo_quantize_tensor logic exactly, but we can probably revert this line. I think it's preferable to a hard-coded 1e-5. What do people think?
There was a problem hiding this comment.
I'm in favor of removing this line and using our own implementation. LC cannot simultaneously match all paper's implementations, so code-readability takes priority here
There was a problem hiding this comment.
I agree.
For any change to asym, we should generate W8A8 int8 asym models and validate evals. These models are actively supported in vLLM atm.
| @@ -199,6 +199,18 @@ def is_preset_scheme(name: str) -> bool: | |||
| ), | |||
| ) | |||
|
|
|||
| # AWQ quantization | |||
| AWQ = dict( | |||
There was a problem hiding this comment.
Couldn't we just do this, and create an alias?
I think W4A16_asymmetric makes sense with an AWQ alias
| scales = torch.clamp(scales, min=torch.finfo(torch.float32).eps) | ||
| scales = torch.clamp(scales, min=1e-5) |
There was a problem hiding this comment.
I agree.
For any change to asym, we should generate W8A8 int8 asym models and validate evals. These models are actively supported in vLLM atm.
Signed-off-by: Brian Dellabetta <[email protected]>
Signed-off-by: Brian Dellabetta <[email protected]>
This aligns the logic for asymmetric quantization with the implementation found in AutoAWQ's
pseudo_quantize_tensorfunction. This is some core logic here so we should all make sure we're in agreement with the changes before merging in.To be reviewed/merged in conjunction with vllm-project/llm-compressor#1177