Repository navigation
Conversation
Paddle-Bot Review Board (review完成)
可维护性建议(总分 85.2 / 100,只给建议,不影响评审结论)
各维度得分(仅评本 PR 改动,点击展开)
Powered by Nyanpasu claude with Opus 5.5 默认推理级别, please check the suggestions carefully. |
| # HSDP: count a replica once. | ||
| if getattr(p, '_hsdp_replica', False): | ||
| continue |
There was a problem hiding this comment.
HSDP 副本 rank 在 ClipGradByGlobalNorm._dygraph_clip 中会提前返回,导致集合通信不匹配而挂起,同时副本梯度不会被裁剪。
- 默认配置是 dense
no_shard、无 expert,或者 expert 也取no_shard。在这种配置下,_set_clip_attrs会给第二个及之后节点上的 rank(outer rank != 0)的全部 buffer 打_hsdp_replica,覆盖了optimizer.pyFSDP 路径传入的全部(data_buffer, grad)。 - 这些 rank 在本循环里全部
continue,三个sum_square_list*为空,于是第 786-792 行直接return params_grads,不会执行第 855 行对fsdp_group的all_reduce。outer rank 0 仍会进入这个all_reduce,结果是挂起。直接使用ClipGradByGlobalNorm、未经HybridParallelClipGrad包装的路径会触发这个问题,例如MixedInnerOptimizer、Muon。 - 即使有其它参数让流程继续,第二个循环只看
need_clip,但提前返回时副本不会乘clip_var,各副本参数会逐步分叉。
建议:副本参数不要跳过,改为对全局范数贡献 0(例如 sum_square = sum_square * 0,或 append 一个 0 标量),保证每个 rank 都走 all_reduce 并完成裁剪。另外,nn/clip.py 直接读取 FSDP 私有属性 _hsdp_replica,建议在这里加注释说明该属性的来源和语义。
| comm_unit_size: int | None = None, | ||
| reshard_policy: str = "after_forward", | ||
| reshard_filter: str | Callable[..., bool] = "all", | ||
| hsdp_degree: int | None = None, |
There was a problem hiding this comment.
本 PR 新增了公开参数 hsdp_degree / hsdp_dense_outer / hsdp_expert_outer,以及 HSDP 建组、参数 broadcast、outer 梯度 all_reduce 和裁剪去重逻辑,但没有新增任何测试。这些路径出错时通常只会得到错误的梯度或 global norm,不会报错。
建议:
- 在
test/legacy_test/test_fsdp_fully_shard_fusion.py中补充_check_hsdp_strategy和 auto parallel 拒绝分支的单进程用例。 - 在
test/collective/fleet/dygraph_group_fsdp.py中增加一个 HSDP 多卡用例(例如 4 卡、hsdp_degree=2),对比纯 FSDP 基线的 loss 和参数。该用例需要同时覆盖ClipGradByGlobalNorm和hsdp_expert_outer的两种取值。
There was a problem hiding this comment.
新增的单测已覆盖 check_outer_strategy、auto parallel 拒绝 HSDP 参数,以及 _outer_replica 的裁剪计数,这部分已解决。
仍缺多进程覆盖:HSDP 建组(_build_dense_groups / _build_expert_groups)、params 的 outer broadcast、outer 梯度求和(包括新增的提前 outer sync 和 outer_eager=False 的回退路径)都还没有用例。这些路径出错时只会得到错误的梯度,不会报错。建议在 test/collective/fleet/dygraph_group_fsdp.py 中增加 4 卡、hsdp_degree=2 的用例,与纯 FSDP 基线对比 loss 和参数。
There was a problem hiding this comment.
更正:最新提交已回退上一轮新增的 HSDP 单测,当前 head 上 HSDP 仍没有任何测试,上面说的“单进程部分已解决”不再成立。请补回 check_outer_strategy、auto parallel 拒绝分支、_outer_replica 裁剪计数的单测,并增加 HSDP 多卡用例。
There was a problem hiding this comment.
最新提交把建组移到了 #79939,并删掉了 overlap 路径,上面提到的 _build_dense_groups、hsdp_degree、_outer_replica 等已不存在。但本 PR 剩下的 HSDP 逻辑仍没有任何测试:
outer_group的挂载,以及参数在 outer 组上的 broadcast;_set_clip_attrs在 outer 组非 0 号 rank 上置is_firstly_shared=False(keep_whole 时包括 params);finish_grads_sync中 outer all_reduce 的过滤条件;nn.ClipGradByGlobalNorm新增的置零分支。该分支在非 HSDP 场景同样生效:FSDP 已有的“复制 grad buffer 只由 rank 0 计数”标记,现在也会被普通 clip 读取。
CI 只有 2 卡时,可参考 #79939 的 TestFSDPGroups,在单进程内 mock HCG 和集合通信,校验上述分支的通信组、调用参数和标记结果;4 卡以上的 loss 和参数对比可以放到 fleet 侧验证。
| inners = [ | ||
| paddle.distributed.new_group(ranks[i : i + inner_size]) | ||
| for i in range(0, len(ranks), inner_size) | ||
| ] |
There was a problem hiding this comment.
这里新建的 inner/outer 通信组只覆盖当前 sharding group 内的 rank。当存在多个 sharding group(mp>1 或 pp>1)时,不同 sharding group 会用相同的 gid 创建成员不同的组。
- 动态图下
new_group的 gid 来自全局递增的_new_ring_id()。例如 mp=2 时,sharding group[0,2,4,6]和[1,3,5,7]分别在同一 gid 上创建[0,2]和[1,3]。 - NCCL unique id 存放在全局 TCP store 的
NCCLCommContext/nccl_ids/<gid>/0键下(process_group_nccl.cc的GetStoreKey,comm_context_manager.cc的CreateNCCLCommContext)。rank 0 和 rank 1 都会写同一个键,rank 2 和 rank 3 可能读到对方组的 id,结果是 NCCL 初始化挂起或通信器串线。第 767-770 行的outers也有同样问题。 - 注释中的 "Every rank creates every group, in the same order" 只在单个 sharding group 内成立。
topology._set_comm_group的做法是由每个 rank 遍历topo.get_comm_list(...)创建全局所有组。
建议:遍历 self.hcg._topo.get_comm_list("sharding"),为每个 sharding group 依次创建 inner/outer 组,只保留当前 rank 所属的组。如果确实只支持单个 sharding group,请显式校验并报错。
| if not ( | ||
| grads_buffer.is_sharded | ||
| or grads_buffer.fsdp_degree == 1 | ||
| or group.whole_shard_numel is not None | ||
| or group.whole_owner_rank is not None | ||
| ): | ||
| continue |
There was a problem hiding this comment.
💡 可维护性建议(逻辑清晰度,不阻塞合入):这里的四项取反过滤没有说明意图,读者很难判断哪些组会被跳过、为什么跳过。例如 whole_owner_rank 组的非 owner 实际上已经由上面的 _data_released 条件排除。建议把这段条件提成一个命名谓词,或者加一行注释说明“哪些 buffer 需要 outer 求和”,并删去被其它条件蕴含的分支。
| grad_comm_dtype=self.grad_comm_dtype, | ||
| comm_stream=self._comm_stream(group.fsdp_group), | ||
| ) | ||
| group.grads_buffer.outer_group = outer |
There was a problem hiding this comment.
💡 可维护性建议(中层优雅度,不阻塞合入):同一份“buffer 组 → outer group”关系现在有两处来源:一是 _hsdp_outers 加 _outer_group_of(),二是这里动态挂到 grads_buffer 上的 outer_group。后者没有在 TensorFusionBuffer.__init__ 中声明,所以第 1521 行只能用 getattr(..., None) 兜底。建议在 BufferGroup 中声明 outer_group 字段,在这里写 group.outer_group = outer,让 _set_clip_attrs 和 finish_grads_sync 统一读取 group.outer_group。
|
|
||
| def _init_hsdp_groups(self, hsdp_degree, dense_strategy): | ||
| ranks = list(self._fsdp_group.ranks) | ||
| if len(ranks) % hsdp_degree or len(ranks) // hsdp_degree < 2: |
There was a problem hiding this comment.
💡 可维护性建议(错误处理一致性,不阻塞合入):传入 hsdp_degree=0 时,_check_hsdp_strategy 会放行,随后这里的 len(ranks) % hsdp_degree 抛出 ZeroDivisionError,与其它非法配置抛出的 ValueError 不一致。建议在 _check_hsdp_strategy 入口统一校验 hsdp_degree 为正整数。
| f"size {len(ranks)} and leave at least 2 ranks per group." | ||
| ) | ||
| inner_size = len(ranks) // hsdp_degree | ||
| self._hsdp_inner_size = inner_size |
There was a problem hiding this comment.
💡 可维护性建议(初始化耦合,不阻塞合入):self._hsdp_inner_size 只在这里赋值,却由 _init_hsdp_expert_groups 读取,两个方法之间形成了隐式的调用顺序约定。建议让 _init_hsdp_groups 返回 inner_size,再显式传给 _init_hsdp_expert_groups;或者在 __init__ 中与 _hsdp_outers 一起初始化这个字段。
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (45.39%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #79924 +/- ##
==========================================
Coverage ? 45.39%
==========================================
Files ? 4
Lines ? 141
Branches ? 0
==========================================
Hits ? 64
Misses ? 77
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Paddle-Review-Bot
left a comment
There was a problem hiding this comment.
当前 head 仍有未解决的 P2:HSDP 缺少测试覆盖,请补充 HSDP 建组、outer 广播/求和与裁剪计数的单测(详见 Review Board)。
| else: | ||
| fsdp_group = self._fsdp_group | ||
| return color["group"] | ||
| return self._ep_fsdp_group if is_expert else dense_inner |
There was a problem hiding this comment.
💡 可维护性建议(初始化耦合,不阻塞合入):group_of 的 dense 分支读取 dense_inner,但 dense_inner 到第 506 行 build_hybrid_groups 返回后才赋值,而第 500-505 行的预扫描已经调用了 group_of。现在能跑通,只是因为预扫描总传 is_expert=True;以后预扫描若涉及 dense 参数,会在这里抛 UnboundLocalError。建议预扫描不经过 group_of(直接解析 color["group"] 或用 self._ep_fsdp_group),或把 dense 组作为参数传给 group_of,消除这层定义早于赋值的顺序依赖。
| *args, | ||
| reshard_policy=reshard_policy, | ||
| reshard_filter=reshard_filter, | ||
| hsdp_degree=hsdp_degree, |
There was a problem hiding this comment.
💡 可维护性建议(错误处理一致性,不阻塞合入):这三个 HSDP 参数只转发给手动并行分支,本次又删除了第 333-334 行 auto parallel 分支的检查,现在 auto parallel 模式下传入 hsdp_degree 等参数会被静默忽略,训练退化为普通 FSDP,用户看不到任何提示。同文件的手动并行专属参数 grad_comm_dtype 在第 175-176 行会抛 NotImplementedError。建议恢复为仅当 hsdp_degree not in (None, 1) 或传了 outer 策略时报错,与 grad_comm_dtype 保持一致,也不会误伤文档中“None 或 1 关闭 HSDP”的用法。
There was a problem hiding this comment.
最新提交回退了 auto parallel 下拒绝 HSDP 参数的校验,当前 head 上这三个参数在 auto parallel 模式下又会被静默忽略,此建议重新打开。
Paddle-Review-Bot
left a comment
There was a problem hiding this comment.
当前 head 仍有未解决的 P2:HSDP 缺少测试覆盖,请补充 HSDP 建组、outer 广播/求和与裁剪计数的单测(详见 Review Board)。
| grads_buffer.data_buffer.scale_(grads_buffer.grad_scale) | ||
| grads_buffer.grad_scaled = True | ||
| buf = grads_buffer.data_buffer | ||
| grads_buffer.outer_task = run_on_stream( |
There was a problem hiding this comment.
提前发出的 outer sync 默认 buffer 的梯度此时已经是最终值,但现有代码允许 buffer 被 flush 之后继续收到梯度:_ensure_grads_writable 会等 RS 完成,get_main_grad 重新分配 tmp buffer,之后 _flush_expert_grads_after_unit 或 finish_grads_sync 会再做一次 RS。当一个参数被多个 FSDP unit 共享时会走这条路径:param_to_unit_id 把它归到 id 最大的 unit,但 id 较小的 unit 在反向时仍会为它产生梯度。
在这种情况下:
- 第二次
accumulate_reduced_grad在计算流上执行data_buffer.add_,而 outer all_reduce 可能仍在通信流上读写同一个 buffer。两者之间没有等待outer_task,形成数据竞争。 - 1-rank buffer 的梯度直接累加到
data_buffer,问题相同。它在第 1288 行已经 scale 过,后来的梯度没有 scale。 finish_grads_sync遇到outer_task后直接continue,后来的梯度不会再做 outer 求和,各副本的梯度静默不一致。
修改前,finish_grads_sync 先完成所有累加,再同步求和,没有这个问题。建议:若 buffer 在 outer_task 已发出后又收到梯度(例如在 _ensure_grads_writable 中检测),则显式报错,或者让这类 buffer 回退到 finish_grads_sync 中的同步路径。
Paddle-Review-Bot
left a comment
There was a problem hiding this comment.
当前 head 仍有未解决的 P2:HSDP 缺少测试覆盖,请补充 HSDP 建组、outer 广播/求和与裁剪计数的单测(详见 Review Board)。
Paddle-Review-Bot
left a comment
There was a problem hiding this comment.
当前 head 仍有未解决的 P2:HSDP 缺少多进程测试,请补充 HSDP 建组、outer 广播与梯度求和的多卡用例(详见 Review Board)。
Paddle-Review-Bot
left a comment
There was a problem hiding this comment.
当前 head 仍有未解决的 P2:HSDP 缺少测试覆盖,请补充 HSDP 单测与多卡用例(详见 Review Board)。
liym27
left a comment
There was a problem hiding this comment.
1、建议先统一 HSDP 的 Inner/Outer 拓扑和参数布局,再让 Dense/Expert、Optimizer、Grad Clip 和 Checkpoint 基于同一份布局信息工作。
2、第一版建议先实现正确的同步式 HSDP,Outer 通信 overlap 可以后续独立优化。
3、增加完整的多卡正确性验证
|
|
||
| def _build_dense_groups(sharding_group, hsdp_degree, dense_strategy, outers): | ||
| ranks = list(sharding_group.ranks) | ||
| if len(ranks) != paddle.distributed.get_world_size(): |
There was a problem hiding this comment.
HSDP 与原有 FSDP/EP 拓扑耦合过强,这里进行了限制。我们需要考虑多维并行组合:
Global Device Topology
├── TP
├── PP
├── CP
├── EP
└── DP
├── Inner Sharding
└── Outer Replication
There was a problem hiding this comment.
1、HSDP 的 Inner/Outer 的拓扑布局已单独在 #79939 中构建
2、overlap 优化已暂时去除
3、完整的多卡验证需要在fleet下进行,paddle ci目前仅有2卡
PR Category
Distributed Strategy
PR Types
New features
Description
Description
Support HSDP in FullyShardFusion
依赖 #79939:该 PR 在 HCG 中新增
fsdp_outer_degree和get_fsdp_group/get_fsdp_inner_group/get_fsdp_outer_group。拓扑(HCG)
[dp, pp, sharding, sep, cp, mp],FSDP 组为 sharding 组。[dp, pp, moe_sharding, ep, ...],其中ep × moe_sharding = sharding。EP > 1 时,FSDP 组为 moe_sharding 组。get_fsdp_group(is_expert)获取 FSDP 组。HSDP 切分
每个 FSDP 组 reshape 成
[outer, inner],每一行是一个 inner 组,每一列是一个 outer 组。以 2 机 16 卡、fsdp_outer_degree=2为例:[0..7]和[8..15],outer 为[r, r+8]。[0,2,4,6]、[8,10,12,14]等,outer 为[r, r+8]。[r, r+8],也就是整个 moe_sharding 组。get_fsdp_inner_group(is_expert)和get_fsdp_outer_group(is_expert)获取。FSDP 中的实现
is_firstly_shared=False,同一份梯度只计入一次 global norm(nn/clip.py)。sharding_configs.fsdp_outer_degree > 1后,FSDP 切换为 HSDP:参数在 inner 组内分片,在 outer 组间保持副本。dense 和 expert 参数都适用,fully_shard的接口不变。是否引起精度变化
否