-
Notifications
You must be signed in to change notification settings - Fork 99
fix magic send sp/cp logic #1466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -900,25 +900,6 @@ def forward(self, dict_args: dict): | |
| :, (depth + 1) : (depth + 1 + seq_len) | ||
| ].contiguous() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Bug 删除这里对
建议修复方式: if input_ids is not None and self.sequence_parallel and self.config.expert_model_parallel_size > 1:
b, _ = input_ids.shape
input_ids = ScatterOp.apply(input_ids.reshape([-1])).reshape([b, -1])并补一个能走到真实 |
||
|
|
||
| # Scatter input_ids to match decoder_input's local shape (CP/SP) | ||
| if cp_world_size > 1 and self.config.experimental_dataflow: | ||
| # ContextParallelScatterOp expects 3D; unsqueeze then squeeze | ||
| mtp_input_ids_local = ContextParallelScatterOp.apply( | ||
| mtp_input_ids_local.unsqueeze(-1), | ||
| axis=1, | ||
| mode=self.config.cp_balance_mode, | ||
| ).squeeze(-1) | ||
| if self.config.sequence_parallel: | ||
| # [B, S] -> [B, S/tp]: flatten, scatter, reshape back | ||
| B_local = mtp_input_ids_local.shape[0] | ||
| mtp_input_ids_local = mtp_input_ids_local.reshape( | ||
| [-1] | ||
| ).unsqueeze(-1) | ||
| mtp_input_ids_local = ScatterOp.apply(mtp_input_ids_local) | ||
| mtp_input_ids_local = mtp_input_ids_local.squeeze(-1).reshape( | ||
| [B_local, -1] | ||
| ) | ||
|
|
||
| # Trim rotary embeddings to seq_len (once; seq_len is constant across depths) | ||
| _rotary_keys = ( | ||
| "rotary_pos_emb", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问题: 这里现在把
mtp_input_ids_local保持为全局[B, S]后直接传给 MTP transformer,但decoder_input/hidden_states在 sequence parallel 下已经被切到[S/tp, B, H]。当 MTP 层后面是 EP MoE(expert_model_parallel_size > 1)时,MoELayer.forward不会先 gather SP hidden states,TopKRouter.forward的 3D SP 分支也没有对input_ids做 TP scatter,会在形状断言处看到[B, S]vs[B, S/tp]并失败;当前测试用 mock 的_proj_and_transformer_layer只数了 MTP 层内 scatter 次数,没覆盖到这个 router 断言。影响:
enable_mtp_magic_send + sequence_parallel + tensor_model_parallel_size > 1 + expert_model_parallel_size > 1的 MoE 训练会在路由阶段因输入 id 形状不匹配中断,或者无法正确屏蔽 padding。期望处理: 请保留“避免重复 CP/SP scatter”的目标,但把 SP 本地化补到实际消费
input_ids的路径;例如在 router 的 3D SP 分支中,仅当input_ids.shape[1]仍是全局长度时再做一次 scatter,并为上述组合增加回归测试。处理要求:请针对该评论修复并提交新的 commit。
参考实现形态: