-
Notifications
You must be signed in to change notification settings - Fork 993
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
rfcs: graph: support int4/int8 compression for K/V in fused SDPA #2041
base: rfcs
Are you sure you want to change the base?
Conversation
943a6b3
to
83217a2
Compare
inputs should be `1d` tensors. | ||
2. For `per_group` quantization, all dimensions should match the input, | ||
excepts for the dimension where grouped quantization applies, which should | ||
be `src_dim / group_size`. |
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.
Not sure how that matches the case when Batch dimension should be broadcasted for scales/zero-points:
W = [B, K, N]
pre-scale W: [1, gK, N] x [B, K, N] = W'
matmul: [B, M, K] x W' = [B, M, N]
Use case I've seen are like that, batch dimension doesn't receive its own dimension of scales.
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.
Thanks for providing the case. According to potential request from IPEX and per-token quantization, we decide to update the scale/zp input shape requirement for per-group quantization for scalability and flexibility. One potential option is to allow 1
on dimensions other than the last two, K
and N
.
1d5e1d2
to
d633cf7
Compare
04a94db
to
5b84ad0
Compare
/// 4-bit signed integer. | ||
dnnl_s4 = 11, | ||
/// 4-bit unsigned integer. | ||
dnnl_u4 = 12, |
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.
For API completeness, I feel yes we need to add both s4 and u4. Just for my curiosity, do you know if both s4 and u4 are used for this int4 K/V compression request? If both are used, any difference in quantization recipe between s4 and u4?
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.
According to user request, they are likely to use s4
for KV storage. But as the KV compression is still WIP, it might change in the future. Regarding the difference between u4
and s4
recipes, since int4 data types always use asymmetric quantization, the parameters will be quite similar for u4
and s4
. The difference should be mainly related to the de-quantization logic.
5b84ad0
to
6f68c51
Compare
6f68c51
to
f1c5003
Compare
c6ed1d1
to
f1c0722
Compare
f1c0722
to
a8a3a20
Compare
Description
This is to propose to support int4/int8 compression for K/V in fused SDPA.
Link to the rendered document.