Enable dequant fp8 weights quantized per-channel with compressed-tensor method #621
Conversation
mandy-li
requested review from
adobrzyn,
afierka-intel,
iboiko-habana,
kzawora-intel,
mgawarkiewicz-intel,
michalkuligowski,
vivekgoe and
xuechendi
as code owners
✅ CI PassedAll checks passed successfully against the following vllm commit: |
|
|
||
| def get_dequant_weights_func(self, ) -> Optional[Callable[[torch.nn.Module], torch.Tensor]]: | ||
| return self.dequant_fp8_weight | ||
|
|
There was a problem hiding this comment.
It would be better to assign get_dequant_weights_func to the layer to stay consistent with the existing implementation, and no changes are required on the INC side.
vllm-gaudi/vllm_gaudi/extension/ops.py
Lines 787 to 789 in e18a075
✅ CI PassedAll checks passed successfully against the following vllm commit: |
|
@yiliu30 , address your comment by binding dequant function to linear layer after loading weight. Please review |
|
@xuechendi Please be aware this change, thanks! |
mandy-li
force-pushed
the
main
branch
2 times, most recently
from
73d8ed6 to
b91f94c
Compare
✅ CI PassedAll checks passed successfully against the following vllm commit: |
|
@skavulya @lkk12014402 , please help to cross review since you're working on compressed-tensor |
…or method Signed-off-by: mandy-li <[email protected]>
|
|
||
| # bind dequant function to layer for per-channel quantization | ||
| if layer.scheme.strategy == QuantizationStrategy.CHANNEL: | ||
| hpu_ops.bind_dequant_func(layer) |
There was a problem hiding this comment.
If the PR is only for INC dynamic, should not bind the dequant for any per-channel here, right?
What is the scope for this PR?
There was a problem hiding this comment.
For inc. I can check if QUANT_CONFIG env var is set or not if you think necessary
There was a problem hiding this comment.
please add check, we can't hijack non inc path
There was a problem hiding this comment.
seems the change will also impact #552
Please also did a check for dynamic scheme
There was a problem hiding this comment.
No, this should apply to static quant as well.
If we change bind_dequant_func() to something like fp8_perchannel_linear_postprocess_weights to be consistent with fp8_block_linear_postprocess_weights which is not INC specific, do I still need to check if inc path?
| def dequant_fp8_weight(self, layer: torch.nn.Module) -> torch.Tensor: | ||
| if layer.scheme.strategy == QuantizationStrategy.CHANNEL: # weights were quantized per-channel | ||
| dequant_weight = layer.weight.to(layer.weight_scale.dtype) * layer.weight_scale.squeeze() | ||
| return dequant_weight.to(torch.bfloat16).t() |
There was a problem hiding this comment.
Is this works for Gaudi2? Will it gets nan since scale might out of range
There was a problem hiding this comment.
Oh, I checked CI, seems Gaudi2 is not getting nan, this is quite unexpected.
@yiliu30, is there any recent changes fix the Gaudi2 scale issue? Or it is because "scale_method": "ACT_MAXABS_PCS_POW2_WEIGHT_MAXABS_PTS_POW2_HW", will keep range under 244?
There was a problem hiding this comment.
Never mind, I realized this is handled at create_weights
|
@yiliu30 , please help to review, this PR is to enable INC dynamic for compressed_tensor, would like to know if meet your initial design |
✅ CI PassedAll checks passed successfully against the following vllm commit: |
|
LGTM |
| return wrapper | ||
|
|
||
|
|
||
| def bind_dequant_func(layer): |
There was a problem hiding this comment.
Suggest to follow the same name pattern to the rest: fp8_perchannel_linear_postprocess_weights
Yes, it’s aligned with what we did for block-wise scaling. |
5 participants
This PR enables dequant fp8 weights quantized with compressed-tensor method channel-wise