Skip to content
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

Sglang integrarion: Fix dtype mismatch #456

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yichiche
Copy link

@yichiche yichiche commented Feb 5, 2025

  • Cast all int64 values to int32 to fix compatibility issue of sglang interface.
  • Add support for tkl.bf16 to expand data type handling.

@@ -162,12 +163,12 @@ def extend_attention(
N_KV, H_KV, D_KV, ADDRESS_SPACE, wave_input_dtype, v_cache_layout
],
block_table: tkl.Memory[
S, N_KV, GLOBAL_ADDRESS_SPACE, wave_size_dtype, block_table_layout
S, N_KV, GLOBAL_ADDRESS_SPACE, tkl.i32, block_table_layout
],
request_indices: tkl.Memory[S, GLOBAL_ADDRESS_SPACE, wave_size_dtype],
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is a very specific signature, I think you can remove wave_size_dtype and set it to tkl.i64. This will also require some changes to the tests. Would you like to fix this or do you want me to take it over?

@@ -202,11 +204,15 @@ def first_loop(
elements_per_thread=LOAD_ELEMS_PER_THREAD_QK,
mapping=q_mapping,
)
if wave_input_dtype == tkl.bf16:
q_reg = tkw.cast(tkw.cast(q_reg, tkl.f32), tkl.f16)
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize now that this is not necessary, so we can remove this here and elsewhere.

@@ -215,12 +221,14 @@ def first_loop(
e_delta = tkw.exp2(x_j - m_j)
e_init = partial_sum * e_delta_max
d_j = tkw.sum(e_delta, e_init, dim=N_KV)
imm_f16 = tkw.cast(e_delta, wave_input_dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also keep this as imm_f16 = tkw.cast(e_delta, wave_input_dtype) just because we may have bf16 inputs

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.

3 participants