Skip to content

support by channel group on k of pa second token - #20

Open
tiger100256-hu wants to merge 4 commits into
ceciliapeng2011:dev/quant_by_channelfrom
tiger100256-hu:dev/quant_by_channel
Open

support by channel group on k of pa second token#20
tiger100256-hu wants to merge 4 commits into
ceciliapeng2011:dev/quant_by_channelfrom
tiger100256-hu:dev/quant_by_channel

Conversation

@tiger100256-hu

Copy link
Copy Markdown

No description provided.

Comment thread opencl/tests/pageatten/pa_2nd_token.py Outdated
Comment on lines +968 to +969
-DKV_CACHE_COMPRESSION_BY_CHANNEL_SUBGROUP={kvcache_quantization_by_channel_group}
-DKV_CACHE_COMPRESSION_BY_CHANNEL={kvcache_quantization_by_channel}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why no merge the two cases?

Comment on lines +49 to +50
#define K_SCALE_ZP_SIZE 0 // no scale/zp
#define V_SCALE_ZP_SIZE 0 // no scale/zp

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds no need to split KV_SCALE_ZP_SIZE

Comment on lines +35 to +42
#if KV_CACHE_COMPRESSION_BY_CHANNEL_SUBGROUP
#define SUB_GROUP_SIZE 16
#define GROUPS_PER_BLOCK (KV_BLOCK_SIZE / SUB_GROUP_SIZE) // 256/16 = 16
#define K_SCALE_ZP_SIZE (4 * GROUPS_PER_BLOCK) // scale/zp bytes
#define V_SCALE_ZP_SIZE 4 // scale/zp bytes
#elif KV_CACHE_COMPRESSION_BY_CHANNEL
#define K_SCALE_ZP_SIZE 4 // scale/zp bytes
#define V_SCALE_ZP_SIZE 4 // scale/zp bytes

@ceciliapeng2011 ceciliapeng2011 Jan 20, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please merge the two cases... we only need to extend KV_CACHE_COMPRESSION_BY_CHANNEL with a SUB_GROUP_SIZE.. if it equals to KV_BLOCK_SIZE, then it is only a group.

Comment thread opencl/tests/pageatten/pa_2nd_token.py Outdated
Comment on lines 58 to 62
kvcache_quantization_by_token = int(kv_cache_quantization_mode == "by_token")
print(f"{kv_cache_quantization_mode=}, {kvcache_quantization_by_token=}")
kvcache_quantization_by_channel = int(kv_cache_quantization_mode == "by_channel")
kvcache_quantization_by_channel_group = int(kv_cache_quantization_mode == "by_channel_group")
print(f"{kv_cache_quantization_mode=}, {kvcache_quantization_by_token=}, {kvcache_quantization_by_channel_group=}, {kvcache_quantization_by_channel=}")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably by refer to XuChen's PR to use an enum type is better: compressed_kvcache = 0, 1, 2 for fp16, by_token, by_channel respectively.

…nnnel" mode

use by channel and sub_group_size to replace the "by channel_group" mode
add enum for different mode
print gpu compile kernel params

Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants