kimi-k3 : the MoonViT-3d vision tower and full-size loading fixes - #70
kimi-k3 : the MoonViT-3d vision tower and full-size loading fixes#70danielhanchen wants to merge 3 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://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Lines 1522 to 1525 in 6dd3474
Warm up with a smaller Kimi-K3 image
With the converter's default in_patch_limit of 16384 and a 14-pixel patch, max_pixels is 3,211,264, so this initializes the model by building a 1792x1792 graph with 16,384 pre-merge attention tokens. On CPU and other backends without flash attention, the quadratic attention scores alone require over 1 GiB, so loading can OOM even when the user will send only small images; preserve the advertised maximum but override warmup to a modest representative token grid as the other dynamic projectors do.
ℹ️ 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".
| @ModelBase.register("KimiK3ForConditionalGeneration") | ||
| class KimiK3VisionModel(MmprojModel): |
There was a problem hiding this comment.
When convert_hf_to_gguf.py --mmproj processes a Kimi-K3 config, get_model_class(..., mmproj=True) checks MMPROJ_MODEL_MAP before importing this decorated class, but that map has no KimiK3ForConditionalGeneration entry. The command therefore raises NotImplementedError: Architecture 'KimiK3ForConditionalGeneration' not supported before any vision tensors are converted; add the architecture-to-kimivl dispatch entry alongside Kimi-K2.5.
Useful? React with 👍 / 👎.
| elif "mm_projector.proj.2." in name: | ||
| name = name.replace(".proj.2.", ".proj.linear_2.") | ||
|
|
||
| yield from super().modify_tensors(data_torch, name, bid) |
There was a problem hiding this comment.
After the mmproj dispatch is fixed, the checkpoint's mm_projector.post_norm.weight reaches this call unchanged, but the MMPROJ tensor map's V_MM_POST_NORM aliases do not include that Kimi-K3 name. ModelBase.map_tensor_name() consequently raises ValueError during conversion, while clip_model_loader later requires mm.post_norm.weight; rename this tensor here or add the corresponding tensor-map alias.
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 | ||
| return super().filter_tensors(item) |
There was a problem hiding this comment.
Once dispatch reaches this converter, MoonViT's vision_tower.patch_embed.pos_emb.time_weight also passes this broad filter. The new graph explicitly supports only t == 1 and has no temporal-position input, while the MMPROJ tensor map recognizes only the spatial vision_tower.patch_embed.pos_emb; passing the temporal parameter to super().modify_tensors() therefore raises ValueError instead of producing an image-only mmproj. Filter this temporal-only parameter out.
Useful? React with 👍 / 👎.
build_kda consumes ssm_a through a broadcast ggml_mul, never through ggml_ssm_scan, but it was registered as LLM_TENSOR_SSM_A, which llama-arch.cpp maps to GGML_OP_SSM_SCAN. create_tensor probes the buffer type with that op through weight_buft_supported, no backend offers SSM_SCAN for a [n_head] tensor, the probe fails, and ssm_a is placed on the CPU along with everything downstream of it. LLM_TENSOR_SSM_A_NOSCAN exists for exactly this case and maps to GGML_OP_MUL. Both spellings resolve to the same "blk.%d.ssm_a" name, so there is no GGUF change and no converter change. qwen3next.cpp and qwen35moe.cpp already declare their A this way. Measured on Kimi-K3-Mini across two GPUs, llama-bench pp2048: before 10705.91 +- 445.93 tok/s after 17009.96 +- 360.89 tok/s 1.59x The loader reports it directly. Before the change it prints "tensor 'token_embd.weight' (bf16) (and 4 others) cannot be used with preferred buffer type CUDA_Host, using CPU instead"; after it prints "(and 0 others)".
2 participants
ggml-org#26185has taken most of whatunslothai#44carried. Its history now has its own copies of the Kimi K3 chat format,message_delimiters, theLLAMA_MAX_EXPERTSbump to 1024, the converter fixes and the archs-test support, and one commit says so directly:fix: apply nits from @ngxson and text fixes from @danielhanchen.What it does not have is the vision half.
MoonViTappears there only inconversion/kimivl.py; there is notools/mtmd/models/kimik3.cpp, so the image path is absent, and the full-size loading fixes are missing too.This is only that remainder, on top of 26185's current head (
a614fab10b), so the diff is what we still add and nothing that is already upstream:kimi-k3 : fixes for loading and running the full-size model(conversion/base.py,src/models/kimi-k3.cpp)kimi-k3 : add the MoonViT-3d vision tower (image path)(the mtmd model, clip wiring, converter, constants)Ten files, +209/-2, against
unslothai#48's stack which duplicates the text-side work and has to be re-reconciled with 26185 every time either side moves.One resolution worth noting: cherry-picking the vision commit brought a
models/inkling.cppline intotools/mtmd/CMakeLists.txt, since our old branch had Inkling merged. That file does not exist here and would have broken the build, so onlymodels/kimik3.cppis added. I checked everymodels/*.cppthe CMakeLists references resolves to a real file.The base branch
kimi-k3-text-upstreamis 26185 ata614fab10b, so this PR reads as just our delta.