Skip to content

Conversation

@fairydreaming
Copy link
Collaborator

When reshaping kqv at the end of llm_build_kqv() n_embd_head_k is incorrectly used instead of n_embd_head_v to calculate kqv dimensions.

@mofosyne mofosyne added bugfix fixes an issue or bug Review Complexity : Medium Generally require more time to grok but manageable by beginner to medium expertise level labels May 16, 2024
…d n_embd_head_k when making a view of cached value vectors.
@fairydreaming
Copy link
Collaborator Author

I found another place when variables for key vectors were used for processing value vectors, so I added another commit to this PR.

Copy link
Member

@ggerganov ggerganov left a comment

Choose a reason for hiding this comment

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

Which models are affected by this?

@fairydreaming
Copy link
Collaborator Author

DeepSeek-V2 needs this since it has n_embd_head_k != n_embd_head_v, I'm not sure about other models:

llm_load_print_meta: n_embd_head_k    = 192
llm_load_print_meta: n_embd_head_v    = 128

@ggerganov ggerganov merged commit 27b0406 into ggml-org:master May 17, 2024
@fairydreaming fairydreaming deleted the llm_build_kqv_fix branch March 22, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix fixes an issue or bug Review Complexity : Medium Generally require more time to grok but manageable by beginner to medium expertise level

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants