kimi-k3 : full-size model fixes and the MoonViT-3d vision tower - #48
kimi-k3 : full-size model fixes and the MoonViT-3d vision tower#48danielhanchen wants to merge 2 commits into
Conversation
Graph node budget, LLM_TYPE for the 93-layer config, four hparams promoted to required, and an E8M0 NaN check during the MXFP4 repack. Details in the PR description. Assisted-by: Claude Code
New kimik3 projector type, its graph builder and the mmproj converter. Also adds an optional clip.%s.attention.head_dim so build_vit stops deriving d_head from n_embd, which is wrong whenever a tower's qkv width differs from n_embd. Assisted-by: Claude Code
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efc8bc38f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assert self.hparams_vision is not None | ||
| n_head = self.hparams_vision["vt_num_attention_heads"] | ||
|
|
||
| if "wqkv" in name and "weight" in name: |
There was a problem hiding this comment.
For Kimi-K3 checkpoints, vision_tower.patch_embed.proj.weight is a rank-5 Conv3D kernel. This method lets it fall through to MmprojModel.modify_tensors(), which serializes it as a five-dimensional v.patch_embd.weight; the GGUF loader rejects tensors exceeding GGML_MAX_DIMS == 4, and clip_graph::build_inp() only invokes ggml_conv_2d. Fold or squeeze the temporal dimension for the supported single-image path before yielding this tensor.
Useful? React with 👍 / 👎.
| if "mm_projector.proj.0." in name: | ||
| name = name.replace(".proj.0.", ".proj.linear_1.") | ||
| elif "mm_projector.proj.2." in name: | ||
| name = name.replace(".proj.2.", ".proj.linear_2.") |
There was a problem hiding this comment.
When converting the published PatchMergerV2 layout, the required mm_projector.post_norm.weight passes filter_tensors() but is neither renamed here nor covered by the MMPROJ tensor aliases, so super().modify_tensors() raises Can not map tensor and conversion cannot finish. This tensor must be mapped to mm.post_norm.weight, which the new PROJECTOR_TYPE_KIMIK3 loader requires.
Useful? React with 👍 / 👎.
| def filter_tensors(cls, item: tuple[str, Callable[[], Tensor]]) -> tuple[str, Callable[[], Tensor]] | None: | ||
| name, _ = item | ||
| if not name.startswith(("vision_tower.", "mm_projector.")): | ||
| return None |
There was a problem hiding this comment.
The MoonViT-3d state dict also contains vision_tower.patch_embed.pos_emb.time_weight, and this broad prefix filter retains it even though the image-only graph deliberately omits the temporal position term. There is no MMPROJ tensor-map entry or runtime tensor for this parameter, so it reaches super().modify_tensors() and aborts conversion with Can not map tensor; explicitly discard it for the supported t == 1 path.
Useful? React with 👍 / 👎.
ngxson
left a comment
ngxson
left a comment
There was a problem hiding this comment.
vision model impl looks clean, thanks!
There was a problem hiding this comment.
would you mind pushing the n_embd_head change to a dedicated PR, so that I can run a test & make sure it doesn't break existing models?
There was a problem hiding this comment.
@ngxson will check!
I was planning to make this as a PR to pwilkin's fork as well haha
Or if you wanna take over that also works - but I shall re-check! I also did multiple images and Kimi works - https://unsloth.ai/docs/models/kimi-k3#run-kimi-k3-in-llama.cpp
I'll double check other models
unsloth published the sub-4-bit tiers on 2026-07-29 and all four fit: UD-IQ1_S 594.0, UD-IQ1_M 648.9, UD-IQ2_XXS 711.1, UD-Q2_K_XL 861.3 GB against 1133 GB of RAM. The pre-staged row needed no correction - the subdir and prefix it guessed (UD-Q2_K_XL / Kimi-K3-UD-Q2_K_XL) are exactly what shipped, 19 shards - so its `?` becomes a real count and only kimi-k3-ik stays pending, ubergarm having published nothing. That closes the quant gate and leaves the engine as the sole blocker, now characterised properly rather than as "no arch": - ik has no kimi-k3 arch; mainline's #26185 is open and conflicted; unsloth built these against their own fork (unslothai/llama.cpp#48), which their README tells you to use. - Before any of that matters, LLAMA_MAX_EXPERTS is 512 and K3 has 896, so it trips an arch-generic assert in load_hparams before a kimi-k3 hook could run - ik src/llama-hparams.cpp:9, asserted at :165. Mainline's PR to raise it (#26192) was closed unmerged. Recorded as step 0 in porting/k3, with the caveat that the constant sizes stack arrays, so "one line" needs checking rather than assuming. Two findings from Moonshot's README invalidate opts that are right for K2.x, so the K3 rows change from --reasoning off to --reasoning-format deepseek: - K3 always thinks. There is no enable_thinking equivalent; effort is a top-level reasoning_effort field llama.cpp has no flag for, defaulting to max. So the K2-correct flag is simply meaningless here. - K3 was trained in preserved-thinking-history mode: multi-turn and tool calls require the complete assistant message replayed, including reasoning_content AND tool_calls. That is precisely the shape ik #1605 400s on. For K2 you can avoid emitting it; for K3 it is mandatory, so #1605 is promoted from caveat to blocking for agentic K3. Sampling: Moonshot's generation_config sets only max_length and eos, so the rows assert no temperature or top_p rather than inventing them. The registry now warns that downloading is safe but serving is not: download and verify will succeed, then the load will fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 participants
Stacked on top of ggml-org#26185 (Kimi-K3 text support), which is the base branch
here. Only the two Kimi-K3 commits are included, so the diff is 14 files rather than the
merged nightly tree.
1. Fixes for loading and running the full-size model
The 1T-parameter checkpoint exposes issues the small test model does not:
n_expert_usedis read per layer instead of once, so the dense prefix and the MoE layersno longer have to agree.
count, which was over-allocating on a 92-layer model.
conversion/base.pykeeps the expert tensors in their source order, avoiding a repackthat needed the whole layer resident.
2. MoonViT-3d vision tower (image path)
Adds the vision encoder as a new mtmd model:
tools/mtmd/models/kimik3.cppbuilds the graph. Patch embedding is 3d, so the temporalaxis is folded into the batch for the image case.
clip_hparams::n_embd_headis added because MoonViT's head dimension is notn_embd / n_head. It defaults to 0 and the existingn_embd / n_headpath is usedwhenever it is unset, so every other mmproj model is unaffected. I verified this against
the mmproj files for the other supported architectures.
conversion/kimivl.pygainsKimiK3VisionModelto write the mmproj.Audio is not included. Only the image path is implemented.
Testing
n_embd_headchangedoes not alter their behaviour.