Skip to content

clip : fix confused naming ffn_up and ffn_down#13290

Merged
ngxson merged 6 commits intoggml-org:masterfrom
ngxson:xsn/clip_ffn_up_down_fix
May 5, 2025
Merged

clip : fix confused naming ffn_up and ffn_down#13290
ngxson merged 6 commits intoggml-org:masterfrom
ngxson:xsn/clip_ffn_up_down_fix

Conversation

@ngxson
Copy link
Collaborator

@ngxson ngxson commented May 3, 2025 •
edited
Loading

The old clip.cpp code reverse the ffn_up and ffn_down naming by mistake, which make it extremely messy when migrating the conversion script to convert_hf_to_gguf.py

To save myself from some headaches, so I decided to fix it once and for all 😤😤

The new rule is:

  • New model always have fc1 == ffn_up and fc2 == ffn_down
  • For old models, if the reversed ffn naming is detected, we reverse it back

This PR also contain name changing to make it more align with llama.cpp style:

  • n_intermediate --> n_embd
  • hidden_size --> n_ff

Small note: GGUF converted from the old qwen surgery script has n_ff = 0, hopefully this will not be messy in the future

Tested by converting fresh new GGUF and run it with llama-mtmd-cli locally:

  • Gemma 3 4B
  • SmolVLM2 2.2B
  • Qwen 2.5 3B

Tested with existing GGUF on the internet:

OK:   llama-mtmd-cli ggml-org/SmolVLM-500M-Instruct-GGUF:Q8_0
OK:   llama-mtmd-cli ggml-org/SmolVLM2-2.2B-Instruct-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/SmolVLM2-500M-Video-Instruct-GGUF:Q8_0
OK:   llama-mtmd-cli ggml-org/gemma-3-4b-it-GGUF:Q4_K_M
OK:   llama-mtmd-cli guinmoon/MobileVLM-3B-GGUF:Q4_K_M
OK:   llama-mtmd-cli THUDM/glm-edge-v-5b-gguf:Q4_K_M
OK:   llama-mtmd-cli second-state/Llava-v1.5-7B-GGUF:Q2_K
OK:   llama-mtmd-cli cjpais/llava-1.6-mistral-7b-gguf:Q3_K
OK:   llama-mtmd-cli ibm-research/granite-vision-3.2-2b-GGUF:Q4_K_M
OK:   llama-mtmd-cli second-state/MiniCPM-Llama3-V-2_5-GGUF:Q2_K
OK:   llama-mtmd-cli openbmb/MiniCPM-V-2_6-gguf:Q2_K
OK:   llama-mtmd-cli openbmb/MiniCPM-o-2_6-gguf:Q4_0
OK:   llama-mtmd-cli bartowski/Qwen2-VL-2B-Instruct-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/Qwen2.5-VL-3B-Instruct-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/pixtral-12b-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/Mistral-Small-3.1-24B-Instruct-2503-GGUF
OK:   llama-mtmd-cli ggml-org/Qwen2-VL-2B-Instruct-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/Qwen2-VL-7B-Instruct-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/Qwen2.5-VL-3B-Instruct-GGUF:Q4_K_M
OK:   llama-mtmd-cli ggml-org/Qwen2.5-VL-7B-Instruct-GGUF:Q4_K_M

@ngxson ngxson requested a review from ggerganov May 3, 2025 21:37
@github-actions github-actions bot added examples python python script changes labels May 3, 2025
@ngxson
Copy link
Collaborator Author

ngxson commented May 5, 2025 •
edited
Loading

Hi @slaren @ggerganov , sorry for pinging but I realized that some models current does not work correctly without this fix.

If you have time, could you please review this PR? Thanks!

@ngxson ngxson merged commit 5215b91 into ggml-org:master May 5, 2025
57 of 58 checks passed
gabe-l-hart added a commit to gabe-l-hart/llama.cpp that referenced this pull request May 6, 2025
@gabe-l-hart
Merge remote-tracking branch 'origin/master' into GraniteFour
c3b7922 
* origin/master: (27 commits)
llama : fix build_ffn without gate (ggml-org#13336)
CUDA: fix bad asserts for partial offload (ggml-org#13337)
convert : qwen2/3moe : set yarn metadata if present (ggml-org#13331)
CUDA: fix --split-mode row for MMQ (ggml-org#13323)
gguf-py : avoid requiring pyside6 for other scripts (ggml-org#13036)
CUDA: fix logic for clearing padding with -ngl 0 (ggml-org#13320)
sampling : Integrate Top-nσ into main sampling chain (and add it to the server) (ggml-org#13264)
server : Webui - change setText command from parent window to also send the message. (ggml-org#13309)
mtmd : rename llava directory to mtmd (ggml-org#13311)
clip : fix confused naming ffn_up and ffn_down (ggml-org#13290)
convert : bailingmoe : set yarn metadata if present (ggml-org#13312)
SYCL: Disable mul_mat kernels for noncontiguous tensor b (ggml-org#13308)
mtmd : add C public API (ggml-org#13184)
rpc : use backend registry, support dl backends (ggml-org#13304)
ggml : activate s390x simd for Q3_K (ggml-org#13301)
llava/mtmd : fixes to fully support dl backends (ggml-org#13303)
llama : build windows releases with dl backends (ggml-org#13220)
CUDA: fix race condition in MMQ stream-k fixup (ggml-org#13299)
CUDA: fix race condition in MMQ ids_dst (ggml-org#13294)
vulkan: Additional type support for unary, binary, and copy (ggml-org#13266)
...
timwu pushed a commit to timwu/llama.cpp that referenced this pull request Dec 20, 2025
@ngxson @timwu
clip : fix confused naming ffn_up and ffn_down (ggml-org#13290)
058f7cf 
* clip :  fix confused naming ffn_up and ffn_down

* rm ffn_i/o/g naming

* rename n_embd, n_ff

* small fix

* no check n_ff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slaren slaren slaren approved these changes

@ggerganov ggerganov Awaiting requested review from ggerganov

Assignees

No one assigned

Labels

examples python python script changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ngxson @slaren