-
-
Notifications
You must be signed in to change notification settings - Fork 19.6k
[PERF] Change GDN Attention State Layout from [N, HV, K, V] to [N, HV, V, K] #33291
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,7 +55,7 @@ def fused_recurrent_kda_fwd( | |
| if inplace_final_state: | ||
| final_state = initial_state | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the ssm_state / kv_cache also need to be created in the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This layout change is layout of ssm_state/kv_cache.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just checking whether anything needs to change when the kv_cache is initially created. If the current setup yields good accuracy, it should be fine. Let's double-check with spec decode since it’s currently supported.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran spec decoding and added results to description. No accuracy lost with spec decoding. |
||
| else: | ||
| final_state = q.new_empty(T, HV, K, V, dtype=initial_state.dtype) | ||
| final_state = q.new_empty(T, HV, V, K, dtype=initial_state.dtype) | ||
|
|
||
| stride_init_state_token = initial_state.stride(0) | ||
| stride_final_state_token = final_state.stride(0) | ||
|
|
||
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.
do we see as much speedup for larger batch sizes too with these additional transposes?
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.
What do you mean? In description I wrote result for batch=1024
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 the clarification, I misread it as
num-prompt 32. IMO we should still have performance across a range of batch sizes.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.
I added to description comparison with several additional batch sizes