Skip to content

fix(mtp): rename MTP submodule transformer_layer -> mtp_model_layer#54

Open
Zhichenzzz wants to merge 1 commit into
fix/1292-gdn-bshd-packedfrom
fix/1289-mtp-naming
Open

fix(mtp): rename MTP submodule transformer_layer -> mtp_model_layer#54
Zhichenzzz wants to merge 1 commit into
fix/1292-gdn-bshd-packedfrom
fix/1289-mtp-naming

Conversation

@Zhichenzzz

@Zhichenzzz Zhichenzzz commented Jun 8, 2026

Copy link
Copy Markdown

Problem

This fork names the MTP inner module self.transformer_layer, but megatron-bridge registers all MTP weight mappings under mtp_model_layer. So megatron_to_hf_lookup returns None for every MTP param → 'NoneType' object has no attribute 'megatron_module' on bridge checkpoint load, and ... has no attribute 'param_weight' on export (Qwen3.6-27B GDN). Upstream NVIDIA renamed transformer_layermtp_model_layer; this fork is the outlier.

Fix

Rename the MTP instance attribute self.transformer_layerself.mtp_model_layer in MultiTokenPredictionLayer (the ModuleSpec field name is unchanged). This makes the live param names match the bridge.

⚠️ Cross-repo: merge together with the miles PR

Renaming the live param names breaks miles' raw (non-bridge) converters that key on the old name. The paired miles PR updates them (qwen3_5.py / qwen3_next.py / update_weight/common.py). These two PRs must be merged together.

Validation

Real megatron-bridge Qwen3.5 mapping_registry.megatron_to_hf_lookup: all 8 MTP param types flip from None (old transformer_layer) to a resolved mapping (QKV/Auto/GatedMLPMapping) under the new mtp_model_layer name.

Fixes radixark/miles#1289


Update: full upstream alignment + e2e validation

Upstream Megatron-LM renamed the whole MTP naming chain, not just the module attribute. This PR now tracks all of it:

  • MultiTokenPredictionLayerSubmodules.transformer_layermtp_model_layer (dataclass field)
  • get_mtp_layer_spec(transformer_layer_spec) / get_mtp_layer_spec_for_backend(...)mtp_model_layer_spec (kwarg; only in-tree call site get_gpt_mtp_block_spec updated)
  • self.transformer_layerself.mtp_model_layer (module attribute — checkpoint param names derive from this)
  • remaining in-tree consumers of the old attribute: pipeline_parallel/schedules.py (set_current_microbatch raised AttributeError with MTP enabled), cuda_graphs.py, fine_grained_callables.py, model_chunk_schedule_plan.py

Impact / compatibility notes for reviewers:

  1. Affects every MTP model (DeepSeek V3/V4, Qwen3-Next, Qwen3.5/3.6, GLM MoE): new checkpoints save MTP params as mtp.layers.N.mtp_model_layer.*.
  2. Old torch_dist checkpoints containing MTP params will not load after this rename (key mismatch). Upstream made the same break with no compat shim; non-MTP checkpoints are unaffected.
  3. Bridge mappings that already expect the new name (qwen35_vl_bridge, deepseek/common.py) are fixed by this PR — they were silently broken before (load_hf_weights crashed with NoneType has no attribute megatron_module on mtp.layers.0.transformer_layer.*).
  4. Companion PRs: fix(mtp): track megatron mtp_model_layer rename in raw converters miles#1307 (converters, dual-name), [model] fix: register both MTP submodule spellings in qwen3_next_bridge Megatron-Bridge#11 (qwen3_next_bridge, dual-name).

e2e validated:

  • Qwen3.5-35B-A3B VL, bridge mode: load_hf_weights 0 missing / 0 "No mapping found", geo3k RL step train_rollout_logprob_abs_diff=0.0129, reward 0.46 (and a separate 200-step run with the attribute rename: reward 0.17→0.95+, abs_diff flat 0.013–0.020).
  • Qwen3.5-35B-A3B, raw mode with MTP enabled: model build through the renamed spec field, torch_dist load, MTP expert weight sync clean, set_current_microbatch passes (crashed before the consumer fix).

Align the MTP submodule attribute name with upstream Megatron-LM, which
renamed MultiTokenPredictionLayer.transformer_layer to mtp_model_layer.
Updates the submodule spec field and all consumers (schedules, cuda graphs,
fine-grained callables, schedule plan, layer specs).
@Zhichenzzz
Zhichenzzz force-pushed the fix/1289-mtp-naming branch from 5a01ba3 to d4f926b Compare June 18, 2026 21:39
@Zhichenzzz
Zhichenzzz changed the base branch from miles-main to fix/1292-gdn-bshd-packed June 18, 2026 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MTP naming mismatch: Megatron-LM fork uses transformer_layer but megatron-bridge expects mtp_model_layer (breaks Qwen3.6-27B GDN weight conversion)

1 participant