Skip to content

fix: fix param grad sync under SP - #224

Open
Chamberlain0w0 wants to merge 3 commits into
masterfrom
fix/sp-gradient-finalize
Open

Chamberlain0w0 wants to merge 3 commits into
masterfrom
fix/sp-gradient-finalize

Conversation

@Chamberlain0w0

Copy link
Copy Markdown
Contributor

Comment thread infini_train/include/tensor.h Outdated
Comment thread infini_train/src/nn/modules/normalization.cc
Comment thread example/gpt2/main.cc Outdated
} else {
optimizer = optimizer_creator(named_parameters);
}
optimizer->set_model_grad_finalizer(nn::parallel::FinalizeModelGrads);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FinalizeModelGrads 绑定到 optimizer 上感觉职责不太合适。optimizer 接收到的 grad 应该已经是 finalize 后可用于更新的完整梯度,本身只负责后续的 clip / step 等操作。

另外,将 FinalizeModelGrads 放到 optimizer->Step() 内部也容易影响 clip 执行顺序(理论上应该保证:
FinalizeModelGrads -> clip -> step,因为 clip 需要基于最终同步完成的梯度计算)。

建议目前在 optimizer->Step() 之前显式调用 nn::parallel::FinalizeModelGrads(named_parameters);后续如果抽象统一的 training/schedule 入口,再将这部分逻辑收进去。Megatron 也是在 forward/backward schedule 结束后调用 finalize_model_grads,而不是绑定到 optimizer:

(no pipeline schedule 情况,带 pipeline 时类似)https://github.com/NVIDIA/Megatron-LM/blob/0ac6ffd3859fef41bbfcd92a67d56861a79d4b34/megatron/core/pipeline_parallel/schedules.py#L866

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改了一下。现在是 training loop 里面需要显式调用 nn::parallel::FinalizeModelGrads,但是传入的是 model chunks vector。在 backward 完成、optimizer 更新前,统一调用 FinalizeModelGrads(model_chunks):先完成 DP 同步,再执行 SP 梯度的 TP SUM。

另外对应做的修改:

  1. 原位于 DistOpt 里面的 StartGradSync() 和 FinishGradSync() 交由 DDP model 管理;同时删除 StartGradSync(),因为 DDP 里面实际同步开启时机相对固定,简化起见目前不需要额外留这个用户接口;

  2. 未做引入 main_grad 基建的设计,而是让 bucket group 新增 LocalGradShard 映射表:DistOpt 在原有 BuildShardParamsAndBindGrads() 中登记原参数与本地梯度 view,finalizer 据此处理 ZeRO-1/2,尤其解决 ZeRO-2 原参数 grad() 为空时的梯度访问问题。只保存引用,不新增显存 buffer。

    问题简述:
    我们现在实现没有一个统一的 main_grad 的全局 buffer,并且 zero2 里面切梯度,只在反向过程中会算临时 full grad,一方面不会把这个 full grad 写入 tensor.grad,RS 之后也不会把 grad shard 写到 tensor.grad 里面(不符合 tensor.shape == grad.shape 的约束)。之后 finalize 统一入口没法通过 param 获取对应的 param.grad,可能需要额外的维护一个 map

@Chamberlain0w0
Chamberlain0w0 force-pushed the fix/sp-gradient-finalize branch from e8f6bf2 to df9871b Compare September 22, 2026 08:10
Comment thread infini_train/src/tensor.cc
Comment thread example/llama3/main.cc
@Chamberlain0w0
Chamberlain0w0 force-pushed the fix/sp-gradient-finalize branch from df9871b to 7ed52c9 Compare September 28, 2026 01:15
@Chamberlain0w0

Copy link
Copy Markdown
Contributor Author
image image image image image

由于精度修复,gpt2/llama3 测例开了 SP 的情形(case 5/8)都有一定程度偏移,qwen 的带有 tp 的测例也有偏移

@Chamberlain0w0

Copy link
Copy Markdown
Contributor Author
image

std::unordered_set<const Tensor *> qk_norm_params;
for (const auto &[name, param] : model_chunk->NamedParameters()) {
if (name == "q_norm.weight" || name.ends_with(".q_norm.weight") || name == "k_norm.weight"
|| name.ends_with(".k_norm.weight")) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是针对 Qwen 结构的特殊逻辑,建议将这部分封装为一个通用工具函数,放到这个文件上方的匿名空间里,函数命名带上 Qwen 标识,且在注释中进行说明;另外,为什么不直接判断 name.ends_with("q_norm.weight"),是会存在类似 xxxq_norm.weight 的参数命名吗?

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.

2 participants