fix(model): map the fused input norm for MLA with q_lora_rank=None - #5295
Closed
bzantium wants to merge 1 commit into
Closed
fix(model): map the fused input norm for MLA with q_lora_rank=None#5295bzantium wants to merge 1 commit into
bzantium wants to merge 1 commit into
Conversation
With q_lora_rank=None there is no query LoRA, so Megatron-Core builds a single linear_q_proj and Transformer Engine fuses the input norm into it. No standalone input_layernorm module exists, so the registry's existing entry for it matches nothing and every layer's input norm is left at its random initialization. Nothing raises: the parameter simply has no mapping, so the conversion never visits it. DeepSeek-V2 236B and V3 both set q_lora_rank, which is why the path went unnoticed; DeepSeek-V2-Lite and kanana-2-30b-a3b-thinking do not. Also guards the two task loops against the None slots build_conversion_tasks already returns. Without that, an unmapped parameter surfaces as an AttributeError naming no parameter, which hides the cause of exactly this bug. Signed-off-by: Minho Ryu <ryumin93@gmail.com>
Contributor
Author
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
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.
What does this PR do?
Maps the fused input norm that MLA uses when
q_lora_rankis None, so those checkpoints stop loading with every layer's input layernorm left at its random initialization.Fixes #5294.
Why
With
q_lora_rank=Nonethere is no query LoRA to build, so Megatron-Core creates a singlelinear_q_projand Transformer Engine fuses the input norm into it. The parameter is thenand no standalone
input_layernormmodule exists to satisfy the registry's existingdecoder.layers.*.input_layernorm.weightentry.models/deepseek/common.pyalready maps the weight of that single-matrix projection but not the norm that comes with it.Nothing raises. The parameter has no mapping, so the conversion never visits it and the model trains from a partly random initialization. DeepSeek-V2 236B and V3 both set
q_lora_rank, which is why the headline models never exercise this;deepseek-ai/DeepSeek-V2-Lite(27 layers) andkakaocorp/kanana-2-30b-a3b-thinking(48 layers) do not set it, and both go through this shared mapping list.What changes
One registry entry in
models/deepseek/common.py. Both spellings target the same HF tensor and only one exists for a given model, so the pre-existinginput_layernormentry is unaffected. MTP mappings are rewritten from the same dict, so MTP-enabled models pick the entry up without further changes.The PR also guards the two loops in
models/conversion/model_bridge.pythat consume conversion tasks.build_conversion_tasksis declaredList[None | WeightConversionTask]and does leaveNoneslots for unmapped global parameters, but both loops readtask.megatron_moduledirectly, so an unmapped parameter surfaces asnaming no parameter, despite the builder having already warned about each one. That is what made this bug hard to place. Worth noting the more dangerous variant the same gap allows: had the mapping been wrong rather than absent, there would be no
Noneslot and no error at all.Tests
Three cases in
TestCommonMappingSingleMatrixQProjection, no weights needed:linear_q_proj.layer_norm_weightresolves toinput_layernorm.weightinput_layernorm.weightentry still resolves (regression guard for theq_lora_rankset case)Removing just the new mapping line fails the first and third and leaves the second passing, so the tests track the fix rather than the file.
tests/unit_tests/models/deepseek/test_deepseek_bridges.pypasses 25/25.ruff checkandruff formatclean.Verified end to end on 8x B300, TP1/PP1/EP8, with a DeepSeek-V3-shaped 48-layer / 128-expert / MTP=3 checkpoint that sets
q_lora_rank=None: 51 unmapped parameters before, 0 after, weight load completing where it previously raised.