Fix multi-shard moe-split tensor counting - #4
Merged
Conversation
i386
force-pushed
the
codex/fix-moe-split-n-tensors
branch
from
April 17, 2026 08:31
4f91049 to
6cd1324
Compare
i386
changed the title
michaelneale
marked this pull request as ready for review
April 17, 2026 08:45
i386
pushed a commit
that referenced
this pull request
Aug 29, 2026
* Get started with Onyx * Add architecture * Skip keys handled in super() * Loading tensors * Shorten * Graph * Apply suggestion from @pcuenca * Remove norm now embedding in transformers weights * Add eot * Explicit output_multiplier * Handle post_norm_eps * No super call; unhardcode eot. The pattern `self._set_vocab_gpt2()` seems preferred throughout the codebase, and it allows `set_vocab()` to be called from a different part of the Python class hierarchy: the drafter model converter that we may need eventually. * Register for drafting * DFlash: inherit rope type from the linked target. Another option would be to store it in the gguf file itself. * mmproj conversion Note: some fields to be renamed after the implementation works. We are keeping compatibility with the reference Meta gguf for testing purposes. * "clip" header declarations * Load mmproj * Pre-processing * Graph * Go back to using delimiters. Otherwise our generations are worse. Transformers does not use them. We need to trace inputs to verify whether they are equivalent. * downsample_factor -> merge_size * Add vision graph lol, forgot from a previous commit * Additional renames, align with llama.cpp / transformers * Prefer _size instead of independent _h and _w * Fix token layout Co-authored-by: Young Han <younghan@fb.com> * onyx: bring the chat parser onto the onyx branch common/chat.cpp on this branch has no Onyx handling, so a converted model serves malformed chat: the assistant preamble leaks into content ("to=self<|message|>...") and tool calls fail with HTTP 500 "The model produced output that does not match the expected peg-native format" common_chat_params_init_onyx exists on onyx-fair-patch, added there by 8bb73dd3d. It was never on this branch, so this is not a regression -- the two lines developed independently. The code here is taken verbatim from that commit. It is the clean side of `git merge origin/onyx-fair-patch`: chat.cpp is one of the files that merges without conflict. The full merge is not viable -- it produces 13 conflicts, including add/add on conversion/onyx.py and src/models/onyx.cpp where the q_norm-folding and metadata-scale approaches contradict each other, and #4/#7 are stacked on this branch's side of that. Verified on this branch: builds with 0 errors, converts an Onyx checkpoint, and serving it gives "4" for "What is 2+2?" plus a correct get_weather {"city":"Paris"} tool call, where the unported branch gives the two failures above. No converter or runtime changes are included, so this should not interact with the q_norm work. Co-authored-by: Beto de Paola <betodepaola@meta.com> * Less params, bilinear pos-emb interpolation as a graph op instead of CPU * Map to symbolic V_MMPROJ instead of strings * Make a couple params explicit * Patchify via build_inp() * No param for rope_theta * Small cleanup * Restore blank line * Unpermute, to adapt to the latest transformers checkpoint * Apply norm after token embeddings This follows the latest transformers approach. * Remove duplicated function * build_vit * onyx: use the model rope theta on sliding-window layers * DFlash: conversion from transformers drafter * Revert rope_type derivation from target NOTE: this breaks compatibility with Meta's distributed DFlash GGUFs, as the Q/K are stored in "NEOX" (rotated half) format, like in transformers. * Apply suggestion from @pcuenca * Set model type * Remove comment that will become obsolete * Hardcode post_norm_rms_eps instead of new param * Derive SWA+RoPE pattern from gguf array or scalar * Fix model type <-> number of layers * Reorder * Rename * Fix typo * DFlash: seed the draft KV cache from multimodal embedding batches `common_speculative_impl_draft_dflash::process()` returned early on any batch carrying embeddings, so an image prefill never had its target-layer features fused through the DFlash encoder and injected into the draft's KV cache. That left a hole spanning the image's positions, and the next injection at a post-image position failed to initialize its batch: ``` decoding image batch 1/1, n_tokens_batch = 256 decode: failed to initialize batch llama_decode: failed to decode, ret = -1 process: llama_decode(ctx_dft) failed rc=-1 (n_tokens=17, offset=0) srv decode: failed to process speculative batch ``` Every image request with `--spec-type draft-dflash` failed with HTTP 500. Text-only was unaffected, since those batches carry token ids and were let through. Restore the earlier condition, which admits a batch that is either tokens or embeddings and skips only the degenerate neither/both cases. The rest of `process()` is already layout-agnostic -- it gathers features via `llama_get_embeddings_layer_inp()` and indexes `batch_in.pos[]` / `batch_in.seq_id[]`, none of which assume token ids -- so this is the whole fix. Validated against `muse-glimmer-30B-bf16.gguf` + `mmproj-muse-glimmer-30B-bf16.gguf` + a DFlash draft head, on an image describe-the-shapes request: - before: HTTP 500, `failed to process speculative batch` - after: HTTP 200, draft acceptance 0.34012 (167 accepted / 491 generated), mean len 3.04 Output equivalence holds, which is the property that matters: at temperature 0 the drafted response is byte-identical to the same request served with no draft attached (1213/1213 chars), so the draft is drafting correctly through the image context rather than merely not crashing. * Conversion: prefer rewrite to mapping * Revert "Conversion: prefer rewrite to mapping" This reverts commit a92d0ac. * fix lint * sliding_window metadata is not optional * disable state save/load * Apply suggestion from @pcuenca --------- Co-authored-by: Young Han <younghan@fb.com> Co-authored-by: Beto de Paola <betodepaola@meta.com> Co-authored-by: Daniel Han <michaelhan2050@gmail.com> Co-authored-by: ruanrms <ruanslv@gmail.com> Co-authored-by: Xuan Son Nguyen <son@huggingface.co> Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@huggingface.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes
llama-moe-spliton the currentmasterbranch.Users building Mesh-LLM's forked
llama.cppcan now compile the MoE split tool again instead of failing with an undeclaredn_tensorserror in the multi-shard write path.Root Cause
write_group()was updated to operate oninput_shards, but the context-size calculation still referenced a singlen_tensorsvariable from the older single-input implementation.That made the current multi-shard path fail to compile on
master.Change
gguf_get_n_tensors(...)across allinput_shardsValidation
cmake --build build --target llama-moe-split -j4