[Feat] Add Dense MTP and Qwen3.5 VL Dense 4B/27B configs - #2103
PengchengShi00 wants to merge 2 commits into
Conversation
|
@claude review |
|
Claude: Summary本 PR 把 MTP 从 MoE 下沉到 Main Flowchart after this PRflowchart TD
A["Dense.__init__"] -->|mtp_config 非 None| B["build_mtp_block()"]
C["Dense.forward"] --> D["decoder layers"]
D --> E["norm + lm_head → loss"]
E --> F["_maybe_forward_mtp()"]
F --> G["MTPBlock → list[MTPDepthOutput]"]
G --> H["output['mtp_loss'] = mean(depth losses) × loss_scaling_factor"]
I["Dense.fully_shard"] --> J["decoder 分支:layer_idx < int(num_hidden_layers × recompute_ratio)"]
I --> K["MTP 分支:_should_recompute(mtp_idx=...)"]
J -. 同一规则两套口径 .- K
classDef changed fill:#d5f5d5,stroke:#2e7d32
classDef problem fill:#ffd6d6,stroke:#c62828
class B,F,G,H,K changed
class J problem
核心原理实现与单测
抽象与信息隐藏评估
单测建议
其他 Issues
VerdictREQUEST_CHANGES |
| MoEGate, | ||
| ) | ||
| from xtuner.v1.module.mtp import MTPBlock, MTPConfig, MTPLayer | ||
| from xtuner.v1.module.mtp import MTPBlock, MTPLayer |
There was a problem hiding this comment.
Claude: [兼容性] 这里移除了 MTPConfig 的导入,但仓库内 ci/config/qwen3_5_moe_35BA3.py:14、examples/v1/config/sft_qwen3p5.py:9、tests/model/test_qwen3_5.py:15 仍在 from xtuner.v1.model.moe.moe import MTPConfig,会直接 ImportError。请同步改为从 xtuner.v1.module.mtp 导入,或在 moe.py 保留 re-export。
| self.lm_head.weight = self.embed_tokens.weight | ||
| return self | ||
|
|
||
| def _should_recompute( |
There was a problem hiding this comment.
Claude: [设计] 新增的 _should_recompute 目前只被 MTP 分支调用(本文件 400 行),主 decoder 循环仍沿用旧口径 num_recompute_layers = int(self.config.num_hidden_layers * self.fsdp_config.recompute_ratio)(361 行)配合 layer_idx < num_recompute_layers(370 行)判断。
Problem:同一条 recompute 规则在 Dense.fully_shard 中存在两套口径,与这里 docstring 宣称的「decoder 层与 MTP 层视为同一序列,recompute_ratio 作用于 num_hidden_layers + mtp_layers」不符。开启 MTP 后两者分母不同,decoder 段实际重算层数会多于本方法定义的全局规则,decoder 段也永远不享受「序列最后一层不 checkpoint」的规则。对照 MoE 侧,decoder 循环与 MTP 循环都统一走 self._should_recompute(...)(xtuner/v1/model/moe/moe.py:1324 与 :1380),Dense 这里属于迁移不完整。
Solution:让 decoder 循环也经由同一个 Seam 决策,并删除局部的 num_recompute_layers:
if self._should_recompute(layer_idx=layer_idx, mtp_idx=None):
layer = apply_activation_checkpointing(...)Benefits:recompute 规则收敛到 _should_recompute 一处(Locality),Dense 与 MoE 行为一致,后续调整 ratio 语义只需改一处实现,该规则也可通过单一入口被测试。
| last_decoder_layer = list(self.layers.values())[-1] | ||
| if self.mtp_block is not None: | ||
| for mtp_idx, mtp_layer in enumerate(self.mtp_block.layers): | ||
| if self._should_recompute(layer_idx=None, mtp_idx=mtp_idx) or ( |
There was a problem hiding this comment.
Claude: [测试] Dense 的 MTP 分片路径(share_weights 强制 checkpoint、reshard_after_forward、prefetch 链接)无任何测试覆盖,上面 recompute 规则的双口径问题因此无法被回归发现。建议补一个 FSDP 用例断言各 MTP layer 的 checkpoint 与分片决策。
Summary
mtp_configlives onTransformerConfig,Densebuilds/forwards an MTP block, andTrainEnginecan pick upmtp_lossfromModelOutputs.MTPDepthOutput):hidden_statesis required; router fields are optional so dense MTP no longer pretends to be a MoE decoder.mtp_block.*→mtp.*mapping, and updatemodel.md.Test plan
mtp_config=Nonedense / MoE training still works (no MTP modules, no extra loss).text_config.mtp_config = MTPConfig(...), confirmmtp_lossis logged and included in backward.from_hf/save_hf—mtp.*keys load when MTP is on; unusedmtp.*keys stay unexpected when MTP is off.Usage