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

[Tentative] Adding 192 head dim (step_size = 12) #454

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Narsil
Copy link

@Narsil Narsil commented Aug 19, 2024

Not sure if this PR actually works correctly (I'm going to check it).

Deepseek models use head_dim=192, and this cannot be compiled because of this static assert.
This modification works by jumping like step_size=4 + jumping by 8 every iteration.

Let me know if this is interesting in here.

@zhyncs
Copy link
Member

zhyncs commented Aug 19, 2024

@Narsil May you write unit test for it? And you can ref https://github.com/zhyncs/dl/blob/master/flashinfer_build.sh to compile from source.

@zhyncs zhyncs requested a review from yzh119 August 19, 2024 08:38
@Narsil
Copy link
Author

Narsil commented Aug 19, 2024

Which tests would you like me to add, batch_prefill_kernels ? others ?

Wdym ref to build from source ? I am building already.

@zhyncs
Copy link
Member

zhyncs commented Aug 19, 2024

@Narsil
Copy link
Author

Narsil commented Aug 19, 2024

Are the tests ran anywhere ?

@zhyncs
Copy link
Member

zhyncs commented Aug 19, 2024

Are the tests ran anywhere ?

There is currently no CI configured, you can use pytest in the local development environment to run.

@yzh119
Copy link
Collaborator

yzh119 commented Aug 19, 2024

Hi @Narsil , thanks for your contribution!
You can add 192 to

head_dims = os.environ.get("FLASHINFER_HEAD_DIMS", "64,128,256").split(",")

if compilation successes, you can run unittests such as https://github.com/flashinfer-ai/flashinfer/blob/0d618712faff20a84bbd513d02ac01e16be19306/python/tests/test_batch_prefill_kernels.py and see how does it work.

@zhyncs
Copy link
Member

zhyncs commented Sep 19, 2024

Hi @Narsil Any update?

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