Skip to content

model : avoid ggml_cont_3d for fused QKV weights#15662

Merged
ggerganov merged 7 commits intomasterfrom
gg/model-avoid-cont3d
Sep 8, 2025
Merged

model : avoid ggml_cont_3d for fused QKV weights#15662
ggerganov merged 7 commits intomasterfrom
gg/model-avoid-cont3d

Conversation

@ggerganov
Copy link
Member

@ggerganov ggerganov commented Aug 29, 2025 •
edited
Loading

ref #15602 (comment)

  • Avoid Vcur = ggml_cont_3d(..) when the QKV weights are merged in a single tensor
  • Make llama_kv_cache:: cpy_k and cpy_v more readable

@ggerganov ggerganov mentioned this pull request Aug 29, 2025
Open
4 tasks
@CISC CISC mentioned this pull request Aug 29, 2025
Merged
4 tasks
@ggerganov ggerganov marked this pull request as ready for review September 7, 2025 17:24
@ggerganov ggerganov force-pushed the gg/model-avoid-cont3d branch from f15d515 to 3dec397 Compare September 8, 2025 06:47
CISC
CISC approved these changes Sep 8, 2025
View reviewed changes
Copy link
Collaborator

@CISC CISC left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Tested with CodeQwen1.5, Phi2, jina-embeddings-v3 and PLaMo2.

@CISC
Copy link
Collaborator

CISC commented Sep 8, 2025

Hmmm, https://github.com/ggml-org/ci/blob/results/llama.cpp/60/d6e7c6fd8bacac0892b8722f5d5c585139cb43/ggml-4-x86-cuda-v100/stdall#L1957

@ggerganov
Copy link
Member Author

ggerganov commented Sep 8, 2025

Hmmm, https://github.com/ggml-org/ci/blob/results/llama.cpp/60/d6e7c6fd8bacac0892b8722f5d5c585139cb43/ggml-4-x86-cuda-v100/stdall#L1957

This is due to #15687

@CISC
Copy link
Collaborator

CISC commented Sep 8, 2025

Hmmm, https://github.com/ggml-org/ci/blob/results/llama.cpp/60/d6e7c6fd8bacac0892b8722f5d5c585139cb43/ggml-4-x86-cuda-v100/stdall#L1957

This is due to #15687

Ah, I get a segfault locally though at the first REPEAT test after ARGMAX.

@ggerganov
Copy link
Member Author

ggerganov commented Sep 8, 2025

On my end, all tests except GET_ROWS and the new IM2COL_3D are passing.

@CISC
Copy link
Collaborator

CISC commented Sep 8, 2025 •
edited
Loading

On my end, all tests except GET_ROWS and the new IM2COL_3D are passing.

Nvm, must have been some other issue pre-rebase, I pulled latest changes and applied #15868 and everything is fine now.

Edit: Eh, almost, got GGML_ASSERT(ggml_is_contiguous(src0)) on PAD, but that's surely not related. It's pad_ext test with v == true. Fixed in #15869

@ggerganov ggerganov merged commit cf0e3ba into master Sep 8, 2025
52 of 55 checks passed
@ggerganov ggerganov deleted the gg/model-avoid-cont3d branch September 8, 2025 07:25
@CISC CISC mentioned this pull request Sep 9, 2025
Closed
@ggerganov ggerganov mentioned this pull request Oct 3, 2025
Closed
@CISC CISC mentioned this pull request Oct 3, 2025
Merged
Nexesenex added a commit to Nexesenex/croco.cpp that referenced this pull request Oct 26, 2025
@Nexesenex
Revert "model : avoid ggml_cont_3d for fused QKV weights (ggml-org#15662
679abcd 
)"

This reverts commit cf0e3ba.
blime4 referenced this pull request in blime4/llama.cpp Feb 5, 2026
@ggerganov
model : avoid ggml_cont_3d for fused QKV weights (#15662)
60def81 
* model : avoid ggml_cont_3d for fused QKV weights

ggml-ci

* kv-cache : make cpy_k and cpy_v implementation more readable

ggml-ci

* cont : add comments

ggml-ci

* cont : minor fix [no ci]

* cont : one more fix

* cont : clarity

ggml-ci

* kv-cache : require contiguous heads of k_cur and v_cur

ggml-ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CISC CISC CISC approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ggerganov @CISC