model: add sarvam_moe architecture support - #20275
Conversation
|
There seems to have been a bad merge, please clean up your PR. |
5d09203 to
f11a024
Compare
Cleaned up the PR, Thank you. |
|
Really waiting for the official llama cpp support for Sarvam |
47fb411 to
77cc1dc
Compare
|
I tried compiling this branch on my local (Strix Halo, linux, vulkan).
|
77cc1dc to
ba1367d
Compare
|
Thanks for reporting this and providing the fix! You're right — n_embd_head_v and n_embd_head_k were changed from member variables to functions in upstream llama.cpp. I've pushed the fix to the PR. Also nice to see it running on Strix Halo with Vulkan!
Thanks for reporting this and providing the fix! You're right — n_embd_head_v and n_embd_head_k were changed from member variables to functions in upstream llama.cpp. I've pushed the fix to the PR. Also nice to see it running on Strix Halo with Vulkan! |
|
The updated code compiles and runs fine |
cc7fc57 to
981c80f
Compare
|
@sumitchatterjee13 Does this PR support for quantization in Q2_K_S or Q2_K ? |
|
@ggerganov and @CISC what can we do to help this get merged in? are there other plans to support the Sarvam models? |
I will get to it eventually, you are number 321 in line. :) In the mean time it's probably useful to rebase and look at some of the recent changes to master, I think some of them may have made some of the changes in this PR redundant. |
|
@sumitchatterjee13 how can i help? and just making sure that your implementation will also cover the official gguf that they have since released as well right? https://huggingface.co/sarvamai/sarvam-30b-gguf |
981c80f to
1f5eca3
Compare
Hey @fahdad , thanks for offering to help! To clarify: Sarvam's official GGUFs on sarvamai/sarvam-30b-gguf use the existing bailingmoe2 architecture (general.architecture = bailingmoe2 in the GGUF metadata), not a new arch. So those files will continue to load via the existing bailingmoe2 code path in llama.cpp — they don't need this PR. This PR adds a dedicated sarvam_moe architecture that properly handles Sarvam-specific details: Full RoPE (Sarvam uses full rotary; bailingmoe2 uses partial with partial_rotary_factor=0.5) |
|
Please rebase and adapt to refactorings in |
|
Tokenizer parity is broken on Indic scripts (and silently wrong on English) Built this PR (commit The GGUF tokenizer disagrees with the HF tokenizer on every probe. Generation in any non-Latin script is byte-fallback garbage as a result. English - leading-space convention disagrees with HF 236743 appears as a separate token between every word, and the "no-leading-space" variants of This means the runtime pre-tokenizer is splitting on whitespace and emitting the space as its own token, instead of using the GPT-2 byte-level convention the converter advertises. Indic - content tokens missing entirely Same pattern for Tamil, Bengali, Marathi, Telugu, Gujarati, Punjabi, Malayalam, Odia. The Indic content tokens are gone; the GGUF tokenization collapses to Verdict
So the model was trained against tokenization X, and llama.cpp on this PR is feeding it tokenization Y. Repro With either of:
Suspected location
|
ce20e48 to
b0e8404
Compare
Thanks @pkhara31, your report was excellent — pinpointed the exact issue and made the fix straightforward. Root cause confirmed. Your "Suspected location" section was spot-on: tokenizer.ggml.pre = "sarvam-moe" was routing to a custom regex that didn't match any of the byte-level custom implementations in unicode.cpp, so it fell through to std::regex which doesn't handle Unicode word boundaries properly — and worse, my converter was force-converting SentencePiece tokens to GPT-2 byte-level encoding, which collided with byte-fallback tokens. Fix (pushed in the latest commit): Switched to SPM-style BPE handling, mirroring LLAMA_VOCAB_PRE_TYPE_GEMMA4: Verification. I ran a parity test across 22 official Indian languages + English + mixed-script + edge cases (50 probes total): 50/50 probes passed Updated GGUFs are on the HF repo (BF16 / Q8_0 / Q6_K / Q4_K_M). Inference now generates fluent Hindi instead of byte-fallback garbage. Thanks again for the detailed bug report — really appreciated. |
Done, Thank you |
CISC
left a comment
There was a problem hiding this comment.
Remove no longer needed arch/model code.
b0e8404 to
0e4eabf
Compare
0e4eabf to
99a7b1a
Compare
|
this calls for some clebration @sumitchatterjee13 ! kingfisher? or taj? |
Thank you. @fahdad 😊 Eather is fine... 😁 |
Add support for
sarvam_moearchitecture (sarvamai/sarvam-30b).SarvamMoEForCausalLMis a straightforward extension ofBailingMoeForCausalLM(see vLLM PR #33942)routed_scaling_factor=2.5Test plan
AI disclosure
AI tools were used in an assistive capacity for this contribution.