Skip to content

remove contiguous copy for flash-attn opbuilder - #372

Merged
tjruwase merged 3 commits into
deepspeedai:mainfrom
YizhouZ:yizhou/fupdate_flash
Apr 2, 2024
Merged

tjruwase merged 3 commits into
deepspeedai:mainfrom
YizhouZ:yizhou/fupdate_flash

Conversation

@YizhouZ

@YizhouZ YizhouZ commented Mar 27, 2024

Copy link
Copy Markdown

following megatron-ds original attention shape, now flash-attn op builder supports only [B, S, H, D] shape. Two contiguous copy time could be improved by this PR.

@YizhouZ

YizhouZ commented Mar 27, 2024

Copy link
Copy Markdown
Author

@conglongli Hi, could you help to review this PR? Thanks

@YizhouZ

YizhouZ commented Apr 1, 2024

Copy link
Copy Markdown
Author

Hi @tjruwase, it seems that conglong is not available, could you help to review this? Thanks.

@tjruwase

tjruwase commented Apr 1, 2024

Copy link
Copy Markdown

@YizhouZ, will this change work with older flash-attn op builder?

@YizhouZ

YizhouZ commented Apr 1, 2024

Copy link
Copy Markdown
Author

@YizhouZ, will this change work with older flash-attn op builder?

I think not. These are two different shape required for input tensor.

I could make it specific for xpu device if you want less impact.

@tjruwase

tjruwase commented Apr 1, 2024

Copy link
Copy Markdown

I think not. These are two different shape required for input tensor.

In that case, can we condition based on flash attn version? It is important to maintain backwards-compatibility. Thanks!

@YizhouZ

YizhouZ commented Apr 2, 2024

Copy link
Copy Markdown
Author

I think not. These are two different shape required for input tensor.

In that case, can we condition based on flash attn version? It is important to maintain backwards-compatibility. Thanks!

I made some changes:

  • isolate flash_attn_builder from other flash attn usages
  • compatibility is ensured by checking function name, new version would be flash_attn_builder.flash_attn_func_v2

could you help to review again? Thanks!

@tjruwase

tjruwase commented Apr 2, 2024

Copy link
Copy Markdown

@YizhouZ, thanks for the PR.

@tjruwase
tjruwase merged commit 3c5f475 into deepspeedai:main Apr 2, 2024
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