Skip to content

【Task.27】feat(algorithms): add GDPO multi-reward advantage estimator - #277

Open
Men1scus wants to merge 22 commits into
redai-studio:mainfrom
Men1scus:pr2/gdpo
Open

【Task.27】feat(algorithms): add GDPO multi-reward advantage estimator#277
Men1scus wants to merge 22 commits into
redai-studio:mainfrom
Men1scus:pr2/gdpo

Conversation

@Men1scus

@Men1scus Men1scus commented Aug 14, 2026

Copy link
Copy Markdown

【Task.27】接入 GDPO:多奖励解耦归一化

RFC: #218 · 依赖 #276(注册表),请先合 #276
Base: #276(refactor(algorithms): declare each algorithm once in a registry),
该 PR 已 rebase 到 main@4899b8f 并把 RLOO(#205) 迁进注册表;本 PR 随之 rebase。

注意本 PR 现在的 diff 是 8140 行 / 43 文件,其中包含 #276 的全部 3131 行。 GitHub 的 base 只能是
上游仓库里的分支,而 #276 的 head 在我的 fork 上,所以 base 只能设成 mainGDPO 自身的净增量是
5113 行 / 37 文件
git diff pr1/algorithm-registry pr2/gdpo)。合入 #276 之后本 PR 的 diff 会自动
收缩到这个数。评审顺序按您给的来:先 #276


算法

GDPO(arXiv 2601.05242)对每个奖励分量分别做组内标准化再合并,而不是先求和再归一化。

更正:RFC #218 与本 PR 的早期版本把动机写成「各分量不同但总和相同的组,GRPO 丢弃、GDPO 保留」。后半句是错的,而且是数学上不可能成立的:总和恒定意味着 r₂ = C − r₁,于是 std(r₂) = std(r₁)z₂ = −z₁,等权重下完全抵消为零,与 GRPO 给出同样的结果。这是一次多模型会诊查出来的,已在代码、测试与文档中全面更正,并留了一个专门的回归测试防止它被当作"修复"重新引入。

真实的收益有两条,都实测过:

一、组间的相对强度。 组内标准化把每组都拉到单位方差,于是「只有一个分量在变」的组与「两个分量都在变」的组在 GRPO 下完全相同。GDPO 让各分量先各自标准化,后者幅度是两倍;第三步的 batch 白化是跨组的,所以差异保留到最终 advantage:

组 A(一个分量变化) 组 B(两个分量都变化)
GRPO ±0.707 ±0.707(分不出)
GDPO(含第三步) ±0.548 ±1.095

二、分量间的尺度差异。 correctness ∈ {0,1} 与取值上百的分量(论文实验用响应长度)相加时,和的方差几乎全部来自后者,GRPO 的方向由它单独决定。极端情形:correctness=[1,1,0,0]length=[0,100,200,300](排序相反)时,GRPO 给答错的长响应最高 advantage,GDPO 不会。

反过来,若所有分量在组内都恒定,GDPO 与 GRPO 一样返回零,不会无中生有。

三步,落在已有的两个阶段上:

  1. 逐奖励组内标准化 + 2. 加权合并 → 在 rollout 侧的 reward normalizer 里,输出每样本一个标量,所以 TransferQueue schema 不变。
  2. batch 白化 → 在 advantage 阶段,那里才有 data-parallel 通信域。

第三步按训练批分段(相对既有方案的主要差异)

论文 Eq. 6 在一个训练批上归一化。但调用方为效率会先用 concat_rollout_batchesnum_rollout_minis 个训练批合并再进 advantage 阶段。

在合并后的整体上白化不是精度差异,是另一个优化目标:两个批被对着共同均值中心化,手算的 8 样本用例里有 4 个符号翻转

_whiten_by_segmentmini_batch_sizesROLLOUT_MINI_LOCAL_SAMPLE_COUNTS_KEY,colocate 与 hybrid 三条路径都写)把它们切回来,每段各自白化并各自跨 DP all-reduce。因此 Eq. 6 在 num_rollout_minis > 1 时依然成立,不需要 rollout_batch_size × n_samples == global_batch_size 这条硬约束。示例脚本把 4 × 8--global-batch-size 32 设成相等只是让例子最简单,不是必需。

分段的死锁面,以及为此加的检查

每段一次集合通信,意味着「各 rank 段数必须一致」从一句注释变成了一个安全条件。原先只写了「num_rollout_minis 来自 minibatch plan,所以一致」——expected 不等于 checked,而猜错的后果是死锁:一个 rank 以为两段、另一个以为三段,双方都卡在第三次 collective 上,没有 traceback、没有退出码。

单 rank 元数据损坏更糟:本地 raise 会把同伴留在 collective 序列里。

现在两个判定合并进一次 MAX all-reduce,在任何分段 collective 之前完成,每个 rank 读到同样的数字、在同一处一起失败。测试覆盖跨 rank 不等分片(DP 切分平衡的是 token 不是样本数)、段数不一致、None vs 分段、以及单 rank 元数据损坏——每个用例都断言两个 rank 都返回,所以回归会超时而不是悄悄通过。

capability flag

本 PR 只新增 3 个。 另外 3 个(forbids_normalize_advantages
requires_rewards_normalizationmin_group_size)随 RLOO 的注册化一起移到了 #276——评审意见 2
的原则是「既有算法的通用注册迁移属于基础 PR」,而 RLOO 恰好也需要这三个,它们因此不再是 GDPO 独有的。

flag 归属 为什么
supports_fully_async=False 本 PR 那条路径把 advantage 交给单副本服务,它没有 DP 通信域、每次只见一个切片;切片为 1 时白化恒为 0,训练安静地在零信号上跑完
uses_reward_components 本 PR 驱动 --gdpo-reward-keys / --gdpo-reward-weights 校验
allows_reward_post_process_hooks=False 本 PR --custom-reward-post-process-path--agentic-custom-advantage-path 会在归一化器之前从 post_process_rewards 返回,静默跳过前两步。上游对 reinforce_plus_plus_baseline 也是成对拒绝这两个的,所以合成一个 flag 而不是两个
forbids_normalize_advantages #276 第三步已按序列白化,--normalize-advantages 会再叠一层 token 级;RLOO 则是它刻意保留了 reward 的尺度
requires_rewards_normalization #276 前两步就在 reward 归一化里;RLOO 的 leave-one-out 基线同理
min_group_size=2 #276 组内无偏标准差在 G=1 时无定义;RLOO 除以 G−1

supports_fully_async#276requires_on_policy_updates 都能拒绝 --fully-async,这不是重复:
前者说的是「advantage 依赖批级统计量,而那条路径只给一个切片」,后者说的是「目标函数没有比值修正」。
两个不同的理由,spec 里分别写明。

数值决策

  • GDPO_EPS = 1e-4,不是 GRPO 路径冻结的 1e-6。与参考实现(TRL GRPOTrainerscale_rewards GDPO 分支)一致,且 GDPO 是新算法,没有既有行为会被破坏。差别只在近乎塌缩的组上显现:二值 reward、组大小 8 时组内标准差约 0.4,两者差 0.02%;但连续 reward(论文数学实验用响应长度)可能让某组标准差落到 1e-3 量级,此时两者对尺度因子的影响相差约 10%。
  • 塌缩用精确相等判定,不用容差。任何足以捕捉浮点误差的相对容差,也会丢掉真实信号——std <= 1e-6 * max|x| 会把完全有信息的 [10000, 10000.005, 10000.010, 10000.015] 判成塌缩。
  • distributed_mean_std 用 float64 两遍法。一遍法 E[x²] − E[x]² 在数值远离零时是两个相近大数相减:[1000.0, 1000.01, 1000.02, 1000.03] 返回方差恰好 0(真实 std 1.29e-2),[10000.0, 10000.001, 10000.002, 10000.003] 返回 std 4.6 而非 1.3e-3(差 3660 倍)。两者都不响:前者静默把整批 advantage 归零,后者静默缩放。这类量级很常见——论文自己的数学实验就用响应长度做 reward,token 数在千级。
  • float32 溢出显式失败extract_reward_components 原先只查 float64 的 isfinite,1e300 能通过、cast 后变 inf,随后被读成非有限 std、整批归零、run 干净退出。现在在边界处拒绝,报错能指向产生它的 reward 函数。1e30 仍然放行——门槛是 float32 的可表示范围,不是对 reward 量级的意见。

验证

单卡 H100 smoke,Qwen3-0.6B × GSM8K,correctness + format 双奖励,跑的就是本 PR 的代码。

两种配置各跑一次,因为它们压的是不同的代码路径

配置 num_rollout_minis optimizer steps 压到的路径
--global-batch-size 32(示例默认,4×8 == gbs 1 4 _whiten_by_segment 的单段分支
--global-batch-size 16 2 8 多段分支,即本 PR 相对既有方案的主要差异

第二行的 8 步就是分段生效的运行时证据:4 个 rollout × 2 个 mini。

最终一次(PR HEAD,含段数一致性检查与 reward-vector 化的消费者):

RELAX_REVISION=9f1e02e580514beaa5502beac0bca92164e864ec   (干净工作树,无 -dirty)
Ray Job 'raysubmit_q7LP79imwNPhW8aw' succeeded             exit 0
8 optimizer steps

train/loss        0.33690  0.09120  0.03300  0.08550  0.42790  0.27200  0.37290  0.19110
train/grad_norm   2.49280  2.82850  2.77020  3.37180  2.08420  2.54560  2.09180  1.78740
rollout/raw_reward    0.6875  0.75  0.5  0.34375
zero_std/count_*      有输出(改写后的 metrics 路径在真实 rollout 中正常工作)

全部有限,无 NaN / Inf。train/loss 有负值是正常的——pg_loss 带符号,advantage 有正有负。

最后一行值得单独说:_compute_zero_std_metrics 跑在 rollout 进程里,单元测试覆盖不到那条路径,所以这次 smoke 的作用之一就是确认改写它没有把 rollout 打挂。

关于 rollout/advantages 均值为 0:这是构造使然(组内标准化后每组均值为 0,batch 白化后整批均值为 0),不是训练跑空。真正证明有梯度的是旁边两个数——train/pg_losstrain/grad_norm 都非零且在变化;若 advantage 真的全零,这两个会恒等于 0。

在第三轮修复之后重跑的单卡 smoke--global-batch-size 16num_rollout_minis=2

RELAX_REVISION=2a499709d340ba2288494abed2061c9b60fe98d7   (-dirty 仅因一个未跟踪的 uv.lock)
Ray Job 'raysubmit_4fmJST2QXRnKud1r' succeeded            exit 0
8 optimizer steps

train/loss       0.308  -0.090  0.420  0.190 ...           全部有限
train/grad_norm  2.04   2.76    2.43   1.48  ...

这一跑不是走过场。第三轮把 mini_batch_sizes is None 从「静默退回整段白化」改成报错
所以只要 actor 的任何一条 RL 训练路径其实没写 rollout_mini_local_sample_counts,它就会当场
炸。八步全过 = 生产路径确实都写了,这个 invariant 可以安全地强制。

同时确认第三轮的另外三处改动在真实 reward 上不误触发:日志里既没有 combined to exactly zero
(噪声地板),也没有 zero-std metrics: skipping(metrics 三态)。

第五轮之后重跑(d11e247),并第一次跑了 2 卡

前四次冒烟都是单卡,而单卡下 DP world size 是 1——所有 all-reduce 都是空操作。第五轮我往白化
路径里加了一条新的 collective(any_rank_has_non_finite,用来让所有 rank 一起失败而不是一个
raise、其余卡死),如果只跑单卡,等于「修死锁的补丁只在不可能死锁的配置里被验证过」。

1×H100   raysubmit_Lap4dJNZgGvFqyjw  succeeded   8 步
         train/loss      0.403  0.091  0.138  0.219
         train/grad_norm 1.83   2.94   2.15   1.49

2×H100   raysubmit_TGe58MhMv62FZTb7  succeeded   4 步
         relax.core.controller:270  Using SeqlenBalancedSampler with dp_size=2
         train/loss      0.140  0.354  0.292  0.217
         train/grad_norm 1.58   1.76   1.37   1.17

两次都没有触发本轮改动的任何一条告警:does not vary within the groupnon-finite advantage
unreadable_rewardmini_batch_sizes is None

rollout/rewards = −1.6e−08、rollout/advantages = 7.5e−09——正好是第五轮新钉住的那个不变量
(Eq. 4 逐组中心化 ⇒ 组内和为零 ⇒ 批均值 ≈ 0)在真实 rollout 上的读数。

一次操作失误,如实记

2 卡第一次跑用的是 modal run 而没有 --detach。Modal 的 ephemeral app 绑在本地客户端上,
客户端一断作业就被拆掉——日志停在模型加载、没有 traceback,看起来像跑挂了,其实是被连带杀的。
白烧 $0.76。重跑加了 --detach 就正常了。

我当时还据此得出过一个相反的「教训」(说远端活过了客户端),也是错的,一并记在这里。

双卡(dp_world=2)smoke,补上单卡覆盖不到的跨 rank 路径:

RELAX_REVISION=9f1e02e580514beaa5502beac0bca92164e864ec
Ray Job 'raysubmit_r2WTQMVz53Tp28rA' succeeded            exit 0
8 optimizer steps(dp_world=2 且 num_rollout_minis=2,两条路径同时压满)

Batch statistics reduce over dp_world=2 (this rank is dp_rank=0)
Batch statistics reduce over dp_world=2 (this rank is dp_rank=1)

train/loss       0.0923  0.0580  0.3266  0.3536  0.2878  0.2557  0.4238  0.0682
train/grad_norm  2.6117  2.5490  2.7010  2.5510  1.7818  1.9935  2.2990  1.6947

那两行来自 numerics.py::_log_group_once,它就是为这件事写的:统计量是全局的还是每分片的,在 loss 曲线里看不出来——一个把多余 GPU 让给张量并行的错配置会让 DP 组退化成 1、all-reduce 变成 identity,而训练照跑不误。两个 rank 各自报告 dp_world=2,说明跨 DP 归约这次是真的在做,不是恒等变换。

单卡 smoke 无法覆盖这一段(dp_world=1 时所有 all-reduce 都是 identity),这也是本 PR 早期版本的证据缺口。

单元测试1799 passed, 323 skippedtests/algorithms 单独跑 807 passed)。失败集合与 main@4899b8f 逐条一致——2 failed + 2 errors,验证方式是把 base 检出到一个 detached worktree 里跑同样的用例,而不是 stash(本分支的改动已经提交,stash 什么都不动)。两个 megatron 纯 CPU 测试单独跑也通过:第二轮那个 BLOCKER 正是被全量运行时的 stub 掩盖的。

已知边界

  1. 单个奖励时 GDPO 不退化为 GRPO。step1 除以 std_g + 1e-4、GRPO 除以 std_g + 1e-6,各组 std_g 不同 → 尺度因子逐组不同;step3 还会再做一次 batch 白化。要 GRPO 语义就用 --advantage-estimator grpo
  2. --n-samples-per-prompt 2 时幅度信息丢失:任意两个不同值标准化后恒为 ±0.7071,分量间的区分度只剩权重。示例用 8。
  3. --fully-async 被参数校验拒绝,理由见上表。注意 --hybrid 不受影响:它用 colocate 角色集,advantage 在 Megatron worker 里算,DP 通信域存在。

第四轮:review #277 的三条 + 一次会诊推翻的两条

[P1] 移除合并阶段的 noise floor —— 采纳

combine_group 末尾有一个 all-or-nothing 阈值:合并结果低于 8 · Σ(|wₖ|·noiseₖ) 时整组返回零。
评审人指出它不属于 Eq. 7,而且我自己写的特征测试已经证明它会吞掉真实信号(两分量
base=4.05e13:地板 0.148 vs 真实信号 0.033;十六个各自「干净」的分量:地板 0.82 vs 信号 0.03)。
这条我认。我把「诚实记录一个已知缺陷」当成了「解决它」——docstring 里写着「一个组可以每列都离干净差
百分之一,仍然静默丢掉梯度」,然后把它留在了代码里。

combine_group 现在严格是 Eq. 7,另配一份纯 Python 的 Eq.4+Eq.7 oracle 逐位比对,并有一条测试证明
「分母加 GDPO_EPS」是实现相对论文的唯一偏差。

但我第一版的替代方案也是错的,是一次多模型会诊查出来的

移除 floor 之后我做了一个拆分:训练值保持 Eq. 7 原值,而 filter
group_carries_reward_signal)保留一个相对幅度容差——「合并结果比它自己各项的尺度低六个数量级
即判为舍入」。理由是「筛选和训练值是两个不同的问题」。

这个判据错得比 floor 更根本:分子正比于权重之,分母正比于权重的大小,所以它测的是
权重配置而不是数据;G = 2 时精确退化为 |w₁−w₂|/(|w₁|+|w₂|),与组内任何奖励值都无关。实测它在
两个方向同时判反——扔掉一个最终 advantage 0.43 的真信号,放行一个 1.08 的纯舍入结果。

最干净的一击来自本分支自己的 fixture:test_weights_closer_than_float32_stay_distinct 断言
combined = [-0.577, 1.155, -0.577] 是必须保留的真信号,同一份数据在 filter 里的 ratio 是 2.98e-8
判为残差丢弃。

而且这不是阈值问题:G ≥ 3 时中心化子空间至少二维,可构造 z₂ = -z₁ + δuu ⊥ z₁,δ 任意小),
所以真实信号的比值没有正下界,任何固定阈值都存在反例。

判据已删除。group_carries_reward_signal 现在问的和单奖励分支完全相同、并且在训练实际使用的
float32
上问:min != max。零权重静音和精确抵消仍然判得出来(它们是精确的零),近似抵消判不出来
——这一点现在是写明的代价,不是被含糊过去的。

[P1] 权重保持 float64 直到 transport cast —— 采纳

[16777216, 16777217] 是两个不同的配置值,第二个是 float32 表示不了的第一个整数。两个 float32 检查
保留,但含义变成「这套权重能不能活过那次 cast」的校验,不再是乘法的 dtype。

[P2] 非有限 advantage 显式报错 —— 采纳,但我的第一版修法本身是个 bug

我把「有限输入产生非有限 std」那半守卫删掉了,理由是 distributed_mean_std 在 float64 累加、
需要约 1e153 个样本才可能溢出。这个理由是错的,五家专家独立指出:distributed_mean_std 末尾
return mean.to(values.dtype), std.to(values.dtype) 把 std 转回输入 dtype[-FMAX, FMAX]
两个有限的 float32 值,float64 std 是 √2·FMAX,cast 回去就是 inf,除法返回全零、不报错——
正是我声称已经消灭的失败模式。第二条路径连非有限 std 都不需要:[FMAX]*10 + [-FMAX] 的均值和标准差
都有限,float32 的 values - mean 照样溢出成 -inf。(那条注释里的量级也算错了:约 1.5e231,不是
1e153。)

现在整个白化在 float64 里完成,最后把已归一化的结果转回调用方 dtype,两条路径一起关掉。

顺带修掉的三条

  • --custom-config-path 能绕过三条 RLOO 约束【Task.27】refactor(algorithms): declare each algorithm once in a registry #276 引入)。我把校验按推导顺序拆成四个函数,
    却只把其中两个接回 YAML 覆盖路径,于是 YAML 可以先切到 rloo、再设 --kl-coef
    --num-steps-per-rollout 4、或破坏 one-update 等式的 global_batch_size。四个现在全部重跑。
    变异验证:撤掉重新接上的三个,3 条测试变红。
  • filter 的 drop 标签能打挂 rollout。它在 drop 分支上读 --reward-key,而上面的信号判定只读
    分量 key——所以一个 reward dict 带 --gdpo-reward-keys 但不带标量 key 的多奖励 run 会在这里
    KeyError。改用 zero_std_group_label,metrics 侧本来就用它挡住了同样的输入。
  • 两处 rebase 留下的伤疤min_group_sizeAlgorithmSpec 里声明了两次(第二次静默覆盖,
    第一处 docstring 成孤儿);一处 docstring 仍指向上一轮已删除的 component_noise_scale

补的接线测试

tests/algorithms/test_gdpo_loss_wiring.py:stub 掉 mpu 的四个入口,用真实 rollout_data
loss.compute_advantages_and_returns,检查出来的数字。变异验证:删掉 mini_batch_sizes= 转发
→ 2 条红;把 gdpoadvantage_fn 改成 grpo_broadcast → 3 条红。

仍未解决,如实记录

恒和分量的残差大致是 ulp(C) / 组内展布随基数增长C = 1e9 时到达 optimizer 是 2.6e-3;
C = 1e13、或两列都很大且跨 binade 时是 O(1)——与当年 floor 声称要防的同一量级。没有任何机制
识别它。
上一轮我写的「float64 已经把假梯度压到 1e-6」只在 C ≲ 1e9 成立,那条叫
「最坏情形」的测试其实只钉了一个构造点,现在改成横跨三个数量级并断言其增长。

两个试过的机制都已证明不可行:条件数界无法证明一个小结果不是信号;按白化后幅度筛选需要完整
batch,放在 prompt 级 filter 里是循环依赖。所以这条留作已知限制。

另外必须说明口径:上面所有 O(1) 数字都是整批都是这类组时测的。Eq. 7 逐组中心化,混批时健康组
决定白化尺度,把残差除以它们的标准差——本例是几百倍的常数。这不改变随 C 的增长:C=1e9 混批
是 3.1e-7,C=1e13 混批仍有 5.2e-3。而且「白化单元」是训练批而不是整个 rollout,退化组独占一段
时完全没有这个除法。有一条专门的测试钉住这个区别。

会诊里被我核实为错误的两条(记下来免得重复调查)

  • 「GDPO + 内置 filter + eval 用不同 reward model 会崩」:不成立。两个 generate_rollout 入口都在
    filter 之前分支到 eval_rollout,eval 路径根本不调 dynamic filter。两家独立收敛到同一个错误结论。
  • 「旧 floor 的失效案例原样迁移到了新判据」:不成立。那两个案例在新判据下的 ratio 是 1.6e-2 和
    1.9e-3,离 1e-6 差四个数量级。这是我为了让反方论证有力做的过度推断。

第五轮:会诊审这次修复本身,又推翻了我三条

第四轮改完之后又做了一次五模型会诊(codex / grok / GLM / MiniMax / DeepSeek 各正反,kimi 配额未恢复),
这次审的是修复本身。结果是修复的方向都对,但我在修复里引入了三个新缺陷、写了四句被实测推翻的话。

引入的缺陷

1. 多 rank 死锁。 上一轮我给 whiten_scalar 加的 isfinite raise 是纯本地判断,而紧随其后的
is_collapsedprocess_group 时要做 all-reduce。一个 rank 的分片含非有限值就抛异常退出,其余
rank 卡死。is_collapsed 自己的注释就警告过这件事,而 _agree_on_segmentation 正是本分支前面为消灭
这个模式写的。现在改成一次 MAX all-reduce 传 flag,全 rank 一起失败。

这条有真正的分布式测试tests/algorithms/test_distributed_whitening.py 起两个 gloo 进程、只让
其中一个持有 inf,断言两个都返回且都拒绝。变异验证:换回本地判断,测试挂死(75 秒被杀,对照
整文件 30 秒跑完)。上一轮之所以没发现,是因为单进程测试走 process_group is None 分支,那条
all-reduce 整个被跳过——修死锁的补丁只在不可能死锁的配置里被验证过。

2. 合法配置被误杀。 我把 validate_batch_shape 接回 YAML 覆盖路径,但它读的 global_batch_size
是合并点之前num_steps_per_rollout 派生的。YAML 从 grpo@4-steps 切到 rloo@1-step 本应得到
rollout × n = 128,却被拿去和残留的 32 比较然后拒绝。派生已抽成函数,两条路径共用。

3. 两个契约洞。 whiten_scalar 对整数输入返回全零(末尾那次 .to(values.dtype) 造成的截断),
对接近上限的 float64 仍静默全零。两者现在都 raise。

4. 缺 --reward-key 从「响亮崩溃」变成「并进 zero_std 桶」。 第四轮我为了消除 KeyError 改用了
宽容的 metrics helper,结果把「这组分数都一样」和「这个 reward 缺一个本 run 要求的 key」放进了同一个
桶——一个系统性缺 scalar 的 reward schema 会永远丢弃并重采,而指标看起来只是 prompt 平坦。现在它有
独立的原因码 unreadable_reward

被实测推翻的四句话(都是我写的)

我写的 实测
「混批时同样的残差约 1e-7」 只是 C=1e9 那一格。共享白化单元只是除以健康组的标准差(几百倍常数),不改变随 C 的增长C=1e13 共享单元仍有 5.2e-3。而且「白化单元」是训练批,退化组独占一段就完全没有这个除法
「G=2 时 ratio 精确退化为 |w₁−w₂|/(|w₁|+|w₂|),与 reward 值无关」 只对反向列成立;同向列 ratio = 1.0。真正打死旧判据的是 G≥3 的构造
「this function exists to have no hole」 只堵算法校验器那一类
测试名 grows_without_bound 实际会饱和:1e8→1e9 是 11 倍,1e11→1e13 只有 6 倍

第五轮那次会诊里,五家 CLI 的十份报告中有六份独立指出第一条。 它出现在我专门为「如实记录未解决限制」写的那一段里。docstring 现在直接放
实测表格,让「只有一个数据点」在文本里藏不住。

一个借来的不变量,现在钉住了

min != max 之所以等价于「有信号」,完全因为 Eq. 4 逐组中心化把 combined 锁成组内和为零,于是
「全相等」蕴含「全为零」。这个承重性质此前没有任何测试保护。配套还有一条测试说明它为什么承重:
step 3 按训练批白化而不是按组,所以一个非零常量组不会被白化掉——我原先给新判据写的理由
(「step 3 会把它白化成零」)本身是错的。

顺带

silent_groups 此前用 float64 的 .any(),与 filter 的 float32 min != max 是两个谓词,同一组会
得到不同答案;已统一。GDPO 声明了 forbids_reward_side_kl——advantage_gdpokl 交给
get_grpo_returns,那里只用它取形状,值被丢弃,所以 --kl-coef 只换来一次参考前向。整个
grpo_broadcast 家族都有这个性质而只有 rloo 声明过;对新算法说真话不会拒绝任何现存配置,
四个老算法故意不动(拒绝一个它们今天接受的 flag 是上游的决定)。

仍未关闭,如实列出

  • YAML 合并点之前还有六处非算法校验会被绕过:--ref-load 存在性、kl_coef/kl_loss_coef 互斥
    assert、_normalize_sync_ppo_kl_args、fully-async 的 resource 检查、rollout_batch_size 派生、
    over-sampling assert。都不是本 PR 引入的。根治要把 YAML 合并挪到校验之前,比本 PR 大。
  • 删掉 residual warning 之后没有替代观测:近抵消的组带着舍入残差进训练,运行时沉默。
  • 大基数恒和组的 O(1) 假梯度仍未解决(见上文「仍未解决」一节)。

被我核实为错误的两条会诊结论

[-FMAX, FMAX] 仍全零(distributed_mean_std 末尾那次 cast 的形参是它自己的,修复后传进去的已是
float64 副本);(1e4, -1e4) 权重推翻混批结论(反号权重配反相关分量是增强,那是满量程真信号)。

第三轮会诊修掉的四条

第三轮刻意换了角度:不再审 GDPO 三步数学(前两轮已饱和),改查 PR1 对既有算法的回归面、
metrics/filter 路径、以及测试的可失败性。五位专家中有两条是单独一家发现、经复现成立的,
恰好是本轮最重的两条。

1. 恒和分量在 float32 下产出 O(1) 假梯度(此前作为「已知偏差」写在文档里,定性错了)。
两个分量和为常数时数学上应精确抵消。这里的标准化对这种输入是病态的——要除以接近零的 std,
相对误差被放大 max|x| / std 倍。残差量级 1e-1,比信号本身还大,第三步再除以同量级的
batch 标准差,输出就是 O(1):实测和为 308.95172119140625 的一组奖励,advantage 是
[-0.5770, 1.1539, -0.5770],有限、合理、方向由舍入的最后几位决定。

两处改动,责任划分要说清楚,因为顺手的那句总结(「地板挡住了假梯度」)不是实际发生的事:

  • 分量全链路 float64extract_reward_components)。这是起主要作用的一半:残差降到 1e-10,
    第三步分母里的 GDPO_EPS 又钳住放大倍数,最坏输出只剩 ~1e-6。假梯度到这一步就没了。
  • combine_group 的噪声地板,让抵消变成精确的零。1e-6 不是零,而下游多处按「等于零」
    判断——filter 会保留一个不贡献梯度的组。阈值是算出来的不是调出来的:component_noise_scale
    返回 eps · max|x| / (std + GDPO_EPS),即标准化的条件数,良态输入下比信号低十五个数量级,
    病态输入下与实测残差吻合(界 5.1e-10,实测 4.1e-10)。

2. zero-std metrics 能打挂 rollout。 distributed/ray/rollout.py 没有过滤
reward is None,而它在 agentic/rollout.py 的孪生函数有——共享 helper 于是拿 None 去建
float32 张量,TypeError,从一个日志函数里抛出来。reward is None 是可达的:--group-rm
组奖励一次性赋值,rollout abort 时整段被跳过,这也正是 sglang_rollout.py 那句
"reward is not None" 的 assert 专门为 group_rm 开豁免的原因。影响所有单奖励算法,含默认的 GRPO。

另外 eval 可以用与训练不同的奖励模型(EvalConfig.rm_type),其 reward 合法地不必带
--gdpo-reward-keys。metrics 在那里执行了一条并不生效的训练契约并因此打挂 eval。
两个判断现在都收进 metrics_group_verdict,返回三态且不依赖 sglang 即可 import——
那两份拷贝已经走岔过一次,而且都测不到。

3. mini_batch_sizes is None 改为报错(原「明确不修」第 1 条)。它不是同一个目标的粗糙版本:
仓库自己的 test_merging_the_batches_would_flip_signs_not_just_rescale 证明它会翻转一半样本的符号,
而所有指标都正常。现在经由已有的 MAX all-reduce 让整组一起失败,不会单 rank 退出集合通信。

4. --gdpo-reward-weights 的非有限值与 float32 全零在启动校验就报(原「明确不修」第 2 条)。

补上的测试守卫(每一条都做了变异验证:删掉守卫,对应测试变红)

  • advantage_normalization 此前没有任何测试钉住取值。删掉 REINFORCE++ spec 里的
    "token_global"、或把 loss.py 的比较反过来,整套测试全绿——而这个字段是既有算法每一步都读的
    最后一段数学分派。现在两个调用点和确切集合都从 main 转录。
  • advantage_gae 只有 co_names 检查,现在与 main 内联分支的转录做数值比对。PPO 是本次重构
    改动最大的既有算法,也是唯一没有 GPU 冒烟的。
  • test_grouping_uses_group_index 只构造过连续布局,按位置分组的实现照样能通过。现在交错两个组。
  • 两份 _compute_zero_std_metrics 此前零测试——这正是问题 2 溜过前两轮的原因。

文字与代码不符(前一轮的更正只落到了主实现)

冒烟脚本页头仍挂着 9f1e02e 已撤回的动机;filter 的 docstring 写「每个分量都平」而代码判的是
「加权合并后是否抵消」;一处测试 docstring 引用了发布脚本并不使用的 --global-batch-size
溢出注释描述的均值溢出在改成 float64 后已不会发生;.detach() 被称作 "load-bearing",
而两个调用点传的都是 list、根本走不到那条分支。七处已全部更正。

上游消费者的 reward-vector 化(review 建议 6,已在本 PR 完成)

有三处消费者坐在 advantage 阶段之前,各自问「这个 prompt 组还有信号吗」,而三处都用 --reward-key 的单个标量回答:

  • engine/filters/dynamic_sampling_filters.pycheck_reward_nonzero_std——功能性,它决定组被不被丢弃
  • agentic/rollout.pydistributed/ray/rollout.py_compute_zero_std_metrics——可观测性,不影响训练但会误报

对多奖励算法这是错的问题:--reward-key 标量是否变化,与「GDPO 最终会不会产出非零 advantage」既不充分也不必要。标量恒定的组可能仍有信号(分量尺度悬殊时),标量变化的组也可能没有(分量抵消或被零权重静音)。不修的话,reward 阶段算对了,采样器按另一个标准取舍。

group_carries_reward_signal 按当前算法该问的方式问:单奖励在 float32 上看 --reward-key 标量是否变化;多奖励直接算出 GDPO 前两步的组合结果、看它是否非零。三处共用它。

顺带两处附带修正:

  • 两处 metrics 原本 gate 在 advantage_estimator == "ppo",又是硬编码算法名。改读 needs_critic——那正是这个 gate 的本意(value-based 估计器不做 group-relative)。算法集合相同,行为不变。
  • filter 用 std > 0、metrics 用精确相等,对同一个问题给出两套判据。统一到 float32 张量上的 min != max:它与 std > 0 在所有 float32 输入上一致(min == maxstd == 0),同时与 is_collapsed 用同一种判定。

--dynamic-sampling-filter-path 的 warning 现在只对自定义 filter 触发;内置的已经正确,不再需要警告。

判据本身也在会诊后修正过一次。 最初 group_carries_reward_signal 的多奖励分支问的是「有没有任一原始分量在变」——这跟「算法最终会不会产出非零 advantage」不是同一个问题:某个权重为 0 会让变化的分量失声,两个分量标准化后互为相反数会抵消。两种情况下它都会保留一个实际零梯度的组。现在它直接算出前两步的组合结果、按其是否非零判定,与训练拿到的信号严格一致(有一个测试逐例比对这两者)。

单奖励分支同样修正过:一度改成对 Python float 做精确不等,并在注释里声称「与之前的 std > 0 等价」。并不等价——[0.1+0.2, 0.3, ...] 在 float64 里有差异、cast 到 float32 后塌缩,NaN 组更会被读成「有信号」。现在在 float32 张量上判 min != max,与旧的 std > 0 在所有输入上一致。

@li126com

Copy link
Copy Markdown
Member

整体架构方向是正确的:算法名称、能力和三段实现已经集中到 AlgorithmSpec;CLI、reward、advantage、policy loss 和角色拓扑均由注册表驱动;没有发现 if algorithm == "gdpo";多 reward key、示例和文档也比较完整。现有算法的 kernel 没有被改写,相关 parity 测试提供了较好的回归保障。

不过,当前 GDPO 数值实现还有以下问题需要解决:

[P1] Noise floor 会清零真实的 GDPO 信号,偏离公式

combine_group (https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/rewards.py#L314-L321) 在 Σ_k w_k z_ik 之后增加了一个 all-or-nothing
noise floor:

if max(abs(combined)) <= floor:
return zeros

这不是 GDPO 的标准化与加权合并公式,而且当前测试已经证明它会把真实的非零结果清零:

这些 characterization tests 实际上是在固定已知公式偏差,而不是验证 GDPO 公式正确性。建议移除或重新设计该 heuristic:训练目标应严格保留有限、非退化的 Σwz,数值异常可以 fail-fast 或单独做
诊断,但不应静默修改为零。

[P1] Reward 权重被提前量化为 float32,可能改变 Eq.7 的结果

weight_tensor 当前使用 float32 (https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/rewards.py#L356-L383),而 reward component
和标准化运算已经是 float64。这会在加权前丢失配置中的有效差异。

可复现例子:

  • 两个标准化后互为相反数的分量;
  • 权重配置为 [16777216, 16777217];
  • 转为 float32 后两个权重都变成 16777216;
  • 当前实现得到 [0, 0, 0],而按原始配置计算的结果约为 [0.9999, 0, -0.9999]。

建议权重也保持 float64,至少到加权合并以及 batch whitening 完成后再转换训练所需 dtype,并补对应回归测试。

[P2] Float64 合并结果可能在隐式 float32 交接时溢出并静默归零

Reward 侧只检查 combined 在 float64 中有限,但 advantage 侧会在 _as_reward_tensor
(https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/advantages.py#L68-L90) 无条件转成 float32;随后 whiten_scalar
(https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/advantages.py#L60-L65) 会把非有限 std 当成 collapse,返回全零。

例如两个同向分量 [0,1,2]、权重 [3e38,3e38],reward 侧产生有限的约 ±5.9994e38,转 float32 后变成 ±inf,最终整批 advantage 被静默清零。

建议让 whitening 保持 float64,或在实际 dtype handoff 后检查 finite;非有限输入应该显式报错,不能解释成零方差。

测试与文档建议

当前 755 个算法测试覆盖面很好,但 Megatron 接线测试仍主要依赖源码正则断言
(https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/tests/algorithms/test_dispatch_parity_vs_main.py#L356-L380),分布式测试则直接调用纯 whitening
函数。建议补一个真正调用 loss.compute_advantages_and_returns -> registry -> gdpo 的 CPU/Gloo 测试,避免真实接线被删除或参数漏传后纯函数测试仍全部通过。

另外,PR body 和 RFC #218 §2.2 (#218) 仍声称等权重下 (1,0) 与 (0,1) 这种恒和分量能被 GDPO
保留;实际上两列标准化后互为相反数,等权重仍会抵消为零。仓库内 GDPO README 已经正确说明这一点,建议同步修正 RFC 和 PR 描述,保证设计依据一致。

已验证通过的部分

  • tests/algorithms:755 passed
  • pre-commit run --all-files:全部通过
  • GitHub Python 3.10/3.11/3.12、lint 和 pre-commit checks:全部成功
  • 示例脚本 bash -n:通过
  • 未发现 GDPO 名称特判;注册、CLI 和常规分发路径符合题目要求

修复上述公式与 dtype 问题并补回归后,这个 PR 的整体架构方案具备验收基础。

@li126com

Copy link
Copy Markdown
Member

GDPO 的整体接线完成度较高:没有新增 if algorithm == "gdpo",多 reward key、缺失/非数值 reward、零方差和 reward collapse 均有覆盖;每个 reward 组内标准化、合并及 optimizer-batch 分段白化的正常输入路径也基本正确。focused CPU tests 共 755 passed,示例和中英文文档也比较完整。

当前仍建议 Request changes,原因如下:

  1. [P1] 合并阶段增加了非 GDPO 公式的 noise floor,并会静默清除真实信号。

    combine_group (https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/rewards.py#L314-L321) 在计算 Σ_k w_k z_ik 后又增加了 all-or-nothing 阈值。该阈值不属于 RFC/论文公式,而且现有测试本身(https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/tests/algorithms/test_gdpo.py#L876-L917)已经证明:两个 reward key 时,未截断结果的最大绝对值大于0.03,当前实现仍将整组变成全零;reward key 增加后阈值还会继续累积。

    这类病态输入可以显式报错或记录诊断,但不能静默改变有限、非退化的 GDPO advantage。建议移除该 heuristic,并用独立 Eq.7 oracle 验证结果。

    weight_tensor (https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/rewards.py#L356-L383) 被固定为 float32,而 reward components 使用float64。可复现输入:float32 会把两个权重都表示成 16777216,当前实现返回全零;按照用户实际配置的权重计算应得到约 ±0.9999。建议至少保持 float64 到 component 合并和 batch whitening 完成,并增加回归测试。

  2. [P2] float64 合并结果可能在训练数据交接时溢出,然后被静默当成 collapse。

    当前只验证 float64 的 combined reward 有限,但生产路径会在 dict_to_tensordict (https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/utils/utils.py#L232-L249) 中转成 float32;后续 whiten_scalar (https://github.com/redai-infra/Relax/blob/53ad6534231523371c46368e88bb8cf893fd6510/relax/algorithms/advantages.py#L60-L90) 又会把 non-finite std 当成 zero signal 返回全零。例如两个同向分量配 [3e38,3e38],float64 结果有限,但 float32 变为 inf,最终整组 advantage 为零。

    请在真实传输 dtype 上验证 finite,或者让 GDPO reward 保持 float64 至 whitening 完成;non-finite 输入应显式失败,不能解释为正常 collapse。

  3. 本 PR 还需要等待 【Task.27】refactor(algorithms): declare each algorithm once in a registry #276 完成最新 main/RLOO 的注册迁移。

    【Task.27】feat(algorithms): add GDPO multi-reward advantage estimator #277 是严格堆叠在 【Task.27】refactor(algorithms): declare each algorithm once in a registry #276 上的,不能独立解决这个问题。建议先更新并合入 【Task.27】refactor(algorithms): declare each algorithm once in a registry #276,再将 【Task.27】feat(algorithms): add GDPO multi-reward advantage estimator #277 rebase,使本 PR 的最终 diff 只包含 GDPO 实现、测试、recipe 和文档。

另外建议补一个真正经过 loss.compute_advantages_and_returns -> registry -> gdpo 的 CPU/Gloo 测试。当前大部分分布式测试直接调用纯函数,backend 接线主要通过源码检查;这不足以锁住未来的 dispatch、DP segment 和 dtype 回归。

最后,RFC #218 (#218) 和 PR body 关于等权重 (1,0)/(0,1) 恒和 reward 的描述也需要修正:标准化后两列仍互为相反数,等权重 GDPO 同样会抵消为零。仓库 GDPO 专项文档已经写对,建议同步 RFC 和 PR body。

建议处理顺序:#276 rebase 最新 main 并注册化 RLOO → 合入 #276#277 rebase → 修复 GDPO 公式与 dtype 问题 → 重跑 CPU tests 和 pre-commit。

Copilot AI lite review requested due to automatic review settings August 24, 2026 03:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a declarative algorithm registry (AlgorithmSpec / ALGORITHM_SPECS) to make --advantage-estimator a single source of truth across reward normalization, advantage computation, policy loss dispatch, and controller role wiring, and adds GDPO as a multi-reward advantage estimator with per-component group standardization plus per-batch whitening.

Changes:

  • Adds an algorithm registry (relax/algorithms/*) and migrates existing estimators to registry-driven dispatch (reward/advantage/policy loss/role mapping).
  • Implements GDPO end-to-end: reward-component extraction + decoupled group normalization (rollout side) and segmented batch whitening (advantage side, including distributed correctness).
  • Adds extensive characterization, wiring, and distributed tests, plus runnable GDPO example and documentation updates.

Reviewed changes

Copilot reviewed 42 out of 43 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
relax/algorithms/init.py Exposes registry API while keeping top-level imports lightweight.
relax/algorithms/spec.py Defines AlgorithmSpec and ALGORITHM_SPECS as the single source of truth.
relax/algorithms/advantages.py Centralizes advantage estimators and shared dispatcher (incl. GDPO whitening + segmentation).
relax/algorithms/policy.py Registry-driven policy loss dispatch adapters.
relax/algorithms/numerics.py Shared numeric helpers/constants for whitening/collapse detection and distributed stats.
relax/algorithms/rewards.py Registry-driven reward normalizers, including GDPO decoupled multi-reward normalization and shared signal/metric helpers.
relax/utils/utils.py Replaces estimator-name whitelists with registry-driven reward normalization dispatch; fixes raw_reward scalar fallback behavior.
relax/utils/types.py Adds Sample.get_reward_components() for multi-reward algorithms.
relax/utils/metrics/metric_utils.py Gates RLOO diagnostics based on declared reward normalizer rather than estimator name.
relax/engine/filters/dynamic_sampling_filters.py Makes the “nonzero std” filter algorithm-aware (component-aware for GDPO), and aligns labeling with shared helpers.
relax/components/advantages.py Replaces local estimator branching with shared compute_advantages_and_returns() dispatch.
relax/backends/megatron/loss.py Uses registry-driven advantage + policy loss dispatch; forwards DP group and per-mini segmentation metadata for GDPO.
relax/core/registry.py Derives ALGOS role mapping from the algorithm registry; keeps sft separate.
relax/agentic/rollout.py Aligns zero-std metrics with shared helpers and avoids estimator-name branching.
relax/distributed/ray/rollout.py Aligns zero-std metrics with shared helpers and avoids estimator-name branching.
examples/gdpo/reward_gdpo.py Provides a two-component example reward (correctness/format) with GSM8K label normalization.
examples/gdpo/run-qwen3-0.6B-1xgpu-gdpo.sh Adds a runnable single-GPU GDPO training script wiring reward + keys/weights.
examples/gdpo/README.md Documents GDPO behavior, constraints, and known numerical limits for the example.
examples/gdpo/init.py Adds package marker for example module import paths.
examples/algorithms/README.md Documents GDPO option and links to the GDPO example directory.
examples/generate_reward_model/post_process_genrm_swap.py Updates “GRPO normalize” helper to be registry-driven and robust to new algorithms.
docs/zh/guide/configuration.md Updates estimator options text to reflect registry-driven choices; includes GDPO.
docs/en/guide/configuration.md Same as zh configuration update.
docs/zh/guide/adding-an-algorithm.md Adds a guide describing how to extend algorithms via the registry.
docs/en/guide/adding-an-algorithm.md English version of the “adding an algorithm” guide.
docs/zh/examples/algorithms.md Adds GDPO documentation section, parameters, and constraints.
docs/en/examples/algorithms.md English version of GDPO documentation section.
docs/.vitepress/config.mts Adds navigation entries for the new “adding an algorithm” docs pages.
tests/algorithms/test_algorithm_registry.py Validates registry contents, invariants, and identifier resolution.
tests/algorithms/test_reward_normalizers.py Bitwise characterization tests to ensure refactor preserves legacy normalizer behavior.
tests/algorithms/test_post_process_rewards_dispatch.py Ensures post_process_rewards dispatch is registry-driven and preserves raw/normalized behavior.
tests/algorithms/test_policy_loss_dispatch.py Ensures policy loss selection and call sites are registry-driven and not name-branched.
tests/algorithms/test_advantage_estimators.py Golden-value tests for extracted advantage estimators including GDPO whitening.
tests/algorithms/test_multi_reward_consumers.py Ensures upstream consumers (filters/metrics) ask the correct algorithm-dependent “signal” question for GDPO.
tests/algorithms/test_gdpo_loss_wiring.py End-to-end Megatron loss entry point wiring test for GDPO segmentation/whitening.
tests/algorithms/test_distributed_whitening.py Real 2-process gloo tests validating distributed whitening + deadlock avoidance for segmentation mismatch/malformed metadata.
tests/algorithms/test_algos_roles.py Ensures controller role mapping derives from the registry and matches critic needs.
tests/algorithms/test_example_reward_gdpo.py Validates the shipped GDPO example reward and launch script wiring and doc consistency.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread relax/algorithms/rewards.py Outdated
"""
try:
return group_carries_reward_signal(args, samples)
except (TypeError, ValueError) as exc:
`--advantage-estimator` was interpreted independently in six places: role
lookup, reward normalisation, the advantage formula, the policy loss, and two
rounds of argument validation. Adding an algorithm meant finding all of them,
and missing one failed late -- `reinforce_plus_plus` was accepted by argparse
while absent from `ALGOS`, crashing in `controller.register_all_serve`.

`AlgorithmSpec` states those facts once. It holds string identifiers rather
than callables: the advantage formula runs in the `Advantages` Ray Serve
deployment while the policy loss runs in the Megatron worker, and those two
processes import different module subsets, so each resolves the name against
its own table. That also keeps the module free of heavy imports, which is what
lets the registry be tested on a CPU-only runner.

Only fields with a consumer are included. Role topology comes from
`needs_critic`, so PPO keeps its Critic and nothing hard-codes the name;
`process_role` is untouched, since it selects the role *iteration order* and
that is the controller's orchestration surface.

Both advantage call sites now share one handler while keeping their real
differences: the Megatron path passes `padded_total_lengths` (GAE slices CP
shards at padded offsets, and passing nothing there reads the wrong token
positions rather than raising), the Advantages deployment cannot compute it and
passes nothing.

RLOO (redai-studio#205), which landed on main after this branch opened, is migrated in
rather than merged alongside: keeping either side of that conflict was wrong,
since taking this branch drops RLOO and taking main restores the algorithm-name
branches. Its reward stage becomes the `group_leave_one_out` normaliser, its
unclipped objective a `POLICY_LOSS_FNS` entry, and its eleven startup
constraints six capability fields. `requires_on_policy_updates` is one field
for five of those knobs because they have one cause -- an objective with no
importance-ratio correction cannot account for the policy having moved -- and
the spec says so, including that RLOO is currently its only member.

Two duplicate REINFORCE++ name sets in `loss.py` (advantage normalisation at
main's 691, the loss reducer at 851) become `advantage_normalization`. They had to
stay in step because token-global normalisation is only correct together with
the mask-safe reducer, and nothing enforced that. The test that was supposed to
catch them was blind: it banned `args.advantage_estimator in [` while the
implementation wrote `in {`. It is now a regex over every spelling, with its
own test, and reintroducing `in {` turns it red.

No behaviour change is intended: the normalisers reproduce the previous
arithmetic constant for constant, including the 1e-6 group epsilon and the
`--disable-grpo-std-normalization` gate, and RLOO's reward output is compared
against a transcription of main's inline branch rather than against the helper
it shares.

`apply_custom_config_overrides` re-runs every algorithm validator, not two of
them. `validate_reward_side_kl`, `validate_update_schedule` and
`validate_batch_shape` were split out of `validate_algorithm_args` because
validation has a derivation order, and only two of the four were wired back
into the override path -- so a YAML file could select rloo and then set
`--kl-coef`, `--num-steps-per-rollout 4`, or a `global_batch_size` that breaks
the one-update guarantee, with nothing objecting. `derive_global_batch_size` is
extracted for the same reason: `validate_batch_shape` reads the value that
derivation writes, and re-running the validator without it rejected a
legitimate config (a YAML moving 4 steps to 1 got compared against the batch
size derived from 4). The comment above the calls lists what this still does
*not* cover -- six non-algorithm checks that run before the merge -- because
closing that class means merging the YAML before validation, which is larger
than this change.

Provenance in `test_dispatch_parity_vs_main.py` was wrong and is corrected:
the header named a main SHA that does not exist in the repository, `MAIN_SHA`
held a third, unrelated commit, and the transcribed line numbers pointed at a
July revision with no rloo in it. All of them now name
main@4899b8f3a90489840a736897b4c341d87c6267cf and its actual lines; nothing
between 98a7234 and that commit touched advantages.py, loss.py, utils.py or
ppo_utils.py.

Docs: the estimator table listed every registered algorithm except the one
this branch adds, and the module tree named `numerics.py`, which does not
exist here.

Tests: 1580 passed, 323 skipped. The 2 failures + 2 errors are identical to
main@4899b8f on this machine (no /dev/shm on macOS; one pre-existing
reward_router failure), verified in a detached worktree at that commit.
`pre-commit run --all-files` clean.
GDPO (arXiv 2601.05242) standardises each reward component within its prompt
group before combining them. A group whose rollouts share the same summed
reward but differ in their components -- (correct, badly formatted) and
(wrong, well formatted) both sum to 1 -- carries no signal under GRPO and is
discarded; GDPO keeps both components' signal.

Three steps, split across the two stages that already exist: steps 1 and 2
(per-component group standardisation, weighted combine) run in the reward
normaliser on the rollout side and collapse to one scalar per sample, so the
TransferQueue schema is unchanged. Step 3 (batch whitening) runs in the
advantage stage, where the data-parallel group exists.

Step 3 is whitened per training batch, not per merged rollout: the caller
concatenates `num_rollout_minis` batches before the advantage stage, and
whitening across the join centres both against a shared mean -- 4 of 8 samples
flipped sign in a hand-checked case, which is a different objective rather than
a rounding difference. `_whiten_by_segment` splits them back using
`mini_batch_sizes`, so Eq. 6 holds regardless of `num_rollout_minis`.

The six capability flags added here all have GDPO as their consumer:
`forbids_normalize_advantages` (step 3 already whitens per sequence),
`requires_rewards_normalization`, `supports_fully_async=False` (that mode's
single-replica service sees one slice and a slice of one sample whitens to
zero -- silently), `uses_reward_components`, `min_group_size=2` (an unbiased
group std is undefined for one sample), and `allows_reward_post_process_hooks`
(both hooks return ahead of the normaliser and would skip steps 1 and 2).

Numerics: GDPO_EPS is 1e-4, matching the reference implementation, and
deliberately not the 1e-6 the GRPO path is frozen at. Collapse is detected by
exact equality rather than a tolerance -- any relative tolerance wide enough to
catch float error also discards the genuinely informative
[10000, 10000.005, 10000.010]. `distributed_mean_std` is two-pass in float64;
the one-pass form returns variance 0 for [1000.0, 1000.01, 1000.02, 1000.03].

Verified on a single H100: Ray Job succeeded, 4 optimizer steps, loss /
grad_norm / advantages all finite, no NaN or Inf
(RELAX_REVISION=3caa34cc4c4b8c2f0c9aa654ddb911e1a639d9a2). rollout/advantages
averaging 0 is by construction; the non-zero pg_loss (0.18-0.34) and grad_norm
(1.29-2.22) are what show the gradient is real.

Tests: 1637 passed, same 2 failures + 2 errors as main@98a1274 on this machine.
… overflow

Two failure modes that were silent rather than loud.

Segmentation. `_whiten_by_segment` runs one collective per segment, and the
comment claimed every rank would agree on the count because `num_rollout_minis`
comes from the minibatch plan. Expected is not checked, and the consequence of
being wrong is a deadlock: a rank expecting two segments and a rank expecting
three both block on the third collective, with no traceback and no exit. The
same applies to a rank whose `mini_batch_sizes` is malformed -- raising locally
strands its peers inside the collective sequence, which is worse than not
validating at all.

Both verdicts now travel in one MAX all-reduce ahead of any segment, so every
rank reads the same numbers and raises together. Tests cover unequal shards
across ranks (a DP split balances tokens, not sample counts), mismatched
counts, the `None`-versus-segmented case, and malformed metadata on one rank;
each asserts that *both* ranks return, so a regression times out instead of
passing quietly.

Overflow. `extract_reward_components` checked `math.isfinite` on the float64
value, then cast to float32. A reward of 1e300 passes that check and becomes
`inf`, which reads as a non-finite std one stage later, zeroes the batch, and
lets the run finish cleanly having trained on nothing. Rewards above
float32's range are now rejected at the boundary, where the message can name
the reward function that produced them. 1e30 still passes: the bound is the
representable range, not an opinion about reward magnitude.

Tests: 1645 passed, same 2 failures + 2 errors as main@98a1274 here.
Three consumers sit ahead of the advantage stage and ask "did this prompt
group carry signal?" -- the dynamic-sampling filter, and the zero-std metrics
on both the agentic and the Ray rollout paths. All three answered it with the
single scalar `--reward-key` selects.

For a multi-reward algorithm that is the wrong question, and wrong in the
direction that undoes the algorithm. A group of `(1, 0)` and `(0, 1)` rollouts
has an identical summed reward on every sample, so the filter reads it as dead
and drops it before training -- while GDPO would have extracted signal from
both components. Those are precisely the groups the estimator exists to keep,
so leaving this unfixed means the reward stage does its job and the sampler
throws the result away.

`group_carries_reward_signal` asks the question the configured algorithm
actually needs: the `--reward-key` scalar for single-reward algorithms, every
component for multi-reward ones (a group is dead only when all of them are
flat). The three call sites now share it.

Two incidental fixes that fall out of touching these lines:

- The zero-std metrics gated on `advantage_estimator == "ppo"`, two more
  hard-coded algorithm names. They now read `needs_critic`, which is what the
  gate meant: value-based estimators do not compute group-relative rewards.
  Same set of algorithms, no behaviour change.
- The filter used `std > 0` while the metrics used exact equality, for the
  same question. Unified on exact equality, matching `is_collapsed` and for
  the reason argued there. The two differ only for a group that is exactly
  flat yet whose computed standard deviation is not exactly 0.

The `--dynamic-sampling-filter-path` warning now fires only for a *custom*
filter; the built-in one is correct here and no longer warrants one.

Also fixes a flake I introduced in the previous commit: the segmented
whitening tests derived a fixed port from `hash(mode)`, which is salted per
interpreter and collided with `test_gdn_cp_reassembly`'s dynamically chosen
port. Both now ask the OS for a free port.

Tests: 1653 passed, twice in a row; same 2 failures + 2 errors as
main@98a1274 here.
A multi-CLI review found that the case this feature has been advertised on
since the RFC is mathematically impossible, and that several guards were
narrower than their own comments claimed.

**The motivation was wrong.** "(correct, badly formatted) and (wrong, well
formatted) both sum to 1, so GRPO flattens the group and GDPO does not" is
false in its second half. If the components sum to a constant then
`r2 = C - r1`, hence `std(r2) == std(r1)` and `z2 == -z1`, so equal weights
cancel to exactly zero -- the same answer GRPO gives. No choice of samples
changes this; only unequal weights break the tie.

What GDPO actually buys, verified numerically and now documented:

- *Relative strength between groups.* Per-group standardisation forces every
  group to unit variance, so a group where one component varies and a group
  where both vary come out identical under GRPO (both ±0.707). Standardising
  per component makes the second twice the amplitude, and step 3 whitens
  across the batch, so it survives (±0.548 vs ±1.095).
- *Scale disparity between components.* correctness in {0,1} summed with a
  reward in the hundreds yields a sum whose variance is essentially the large
  component's. With the two ranked oppositely, GRPO gives the wrong-but-long
  responses the highest advantage; GDPO does not.

**The consumers encoded the wrong question too.** `group_carries_reward_signal`
returned True whenever any raw component varied, which disagrees with the
advantage actually produced whenever a weight mutes a component or two
components cancel -- it kept groups that then contribute no gradient. It now
computes steps 1 and 2 (cheap, per-group) and asks whether the result is
non-zero.

**Single-reward judgement is back to float32.** The previous commit compared
Python floats and described that as equivalent to the `std > 0` it replaced.
It is not: `[0.1 + 0.2, 0.3, ...]` survives in float64 and collapses on cast,
and a group of NaNs reads as varying. Both were kept and then contributed
nothing. Judged on a float32 tensor, `min == max` and `std == 0` agree on
every input, so this is now genuinely the old behaviour; non-finite groups are
dropped as they were.

**Overflow checks reached only the cast.** Individual rewards were bounded by
float32's range, but the mean, std and weighted sum are computed in float32
too: [3e38, 2e38, 1e38, 0] overflows to -inf and gets silently zeroed. Weights
had the same gap in both directions (1e300 casts to inf; [1e-50, 0] casts to
all-zero after passing the "not all zero" check). All three now raise.

**`_as_reward_tensor` leaked autograd.** `torch.tensor(x)` detached; `.to()`
returns the same object when dtype and device match, so a reward tensor with
`requires_grad=True` produced advantages carrying grad history. Detached.

Also, from the same review:

- `loss.py` chose the REINFORCE++ normalisation and mask-safe reducer by
  comparing algorithm names -- the last *maths* decision still made that way,
  and the one PR1 most conspicuously missed. Both now read a new
  `advantage_normalization` field. `loss.py` has no algorithm-name comparisons
  left.
- `apply_custom_config_overrides` re-ran only the spec validator, so a YAML
  file could switch to `reinforce_plus_plus_baseline` and enable a reward hook
  it forbids: the spec is deliberately silent there, and the frozen validator
  never ran. It now runs both, in the main path's order.
- With that fixed, `reinforce_plus_plus{,_baseline}` can declare the four
  constraints they were leaving at defaults. An undeclared field is not
  neutral -- it asserts the default -- so the spec had been stating four false
  things about them.
- Docs claimed the built-in filter judges by `--reward-key` (untrue since the
  previous commit), and claimed exact scale invariance (the additive epsilon
  makes it approximate). The test oracle used a relative collapse tolerance
  while the implementation uses exact equality; binary rewards hid the
  disagreement.

Tests: 1657 passed, same 2 failures + 2 errors as main@98a1274 here.
Both were flagged in review as tautologies, and both were.

`test_both_paths_delegate_to_the_shared_estimator` asserted only that the
import line exists. A file that imports the shared handler and then ignores it
satisfies that -- which is what a botched revert looks like. It now asserts the
call, plus the absence of any direct `ppo_utils` kernel in either path: if
either grows its own `get_grpo_returns` again, the estimator is being chosen
outside the registry. Verified by mutation: re-adding that import to loss.py
turns the test red.

`test_no_estimator_name_comparisons_remain` claimed nothing in arguments.py
compares algorithm names while checking two literals -- and
`_validate_reinforce_plus_plus_args` does compare names, deliberately, as a
frozen contract. Widening it to the whole file would either fail or need an
exception list that hides the next regression. It is now scoped to
`validate_algorithm_args`, named accordingly, and reads the function through
`ast.unparse` so the check sees code rather than the docstring's own
description of the pattern it removed. Any comparison inside that function
fails it now, not just two spellings.

Tests: 1657 passed, same 2 failures + 2 errors as main@98a1274 here.
…lying

Second review round. The first finding is a regression I introduced in this
branch; the rest are gaps the previous round left.

**`loss.py` became unimportable without a full Megatron install.** Importing
`ROLLOUT_MINI_LOCAL_SAMPLE_COUNTS_KEY` from `relax.backends.megatron.data`
pulls in `megatron.core.packed_seq_params` at module scope, and the CPU tests
for `loss.py` stub `megatron.core` without that submodule. One import for one
string cost two existing tests:
`test_reinforce_plus_plus_wiring.py` and `test_reinforce_plus_plus_loss.py`
went from 4 passed on main to 2 failed here.

The full suite hid it -- run together, some earlier test installs a stub that
makes the import succeed, so `pytest tests/` stayed green while
`pytest tests/backends/megatron/test_reinforce_plus_plus_*.py` failed. CI that
shards or selects tests would have caught what I did not.

`loss.py` now spells the key out, with `test_loss_py_uses_the_canonical_mini_batch_key`
comparing it against the definition as source text -- no import either way, and
it works on a runner with no megatron at all.

**PPO's spec claimed --fully-async support.** `validate_ppo_config` rejects
both --fully-async and --hybrid for PPO, but the spec left the field at its
default. This is the same class of error the previous commit fixed for the
REINFORCE++ variants, in the one algorithm that commit did not touch.
Declaring it also moves the failure from service registration to argument
validation.

**`advantage_normalization` accepted anything.** It is not a lookup key, so
`_assert_spec_implementations_resolve` could not cover it -- and its failure
mode is worse than a missing key: a typo does not raise, it selects the
default branch in `loss.py` and trains with different maths. Now validated
against its two legal values.

**Step 1 arithmetic moved to float64**, matching `distributed_mean_std` and
for the same reason. It also happens to fix an overflow the review found:
components like [3e38, 2e38, 1e38, 0] overflowed while computing their own
mean in float32 and tripped the combined-value guard even when their weight
was zero. In float64 they standardise normally and the zero weight mutes them
as it should.

**Documented one thing float64 does not fix.** Two components summing to a
constant should cancel exactly, and in float64 they do. But they arrive
already cast to float32, and `C - r` does not survive that cast unchanged:
the pair still sums to `C` (the addition rounds back) while the values have
drifted, leaving ~1e-4 after standardisation, which step 3 divides by a batch
std of the same order to produce advantages of order 1. A group with no signal
gets a confident gradient whose direction is rounding noise. GRPO does not
have this failure -- it standardises the sum, which is exactly constant.
Recorded in examples/gdpo/README.md; a real fix needs a wider reward dtype end
to end or a noise floor in step 3, neither of which belongs here.

Tests: 1659 passed, same 2 failures + 2 errors as main@98a1274 here, and the
two megatron CPU tests pass standalone again.
…ruths

The second review round showed the corrected motivation was still wrong, in
the opposite direction from the original. Both errors turn out to be the same
formula read at two extremes, so this replaces them with the formula.

The combined advantage is `sum_k w_k * z_k` with each `z_k` at unit variance,
so its variance is `sum_k w_k^2 + 2 sum_{i<j} w_i w_j rho_ij` -- `2 + 2*rho`
for two equal weights. GRPO standardises the summed reward instead and lands
on unit variance for every group regardless. What GDPO preserves is therefore
the **correlation structure** between components.

Both previous claims are special cases:

- "GDPO rescues groups whose components sum to a constant" is `rho = -1`.
  They cancel. GDPO included. (Found in round one.)
- "Two varying components mean a stronger signal" is `rho = +1`. True there,
  false at `rho = -0.8`, where the combination measures 0.63x a single
  component. (Found in round two.) The example used to justify it was G=2,
  where the correlation can only be +/-1 -- the one place the claim holds.

Measured across rho: +1 -> 2.00x, 0 -> 1.41x, -0.5 -> 1.00x, -1 -> 0.00x.

The scale-disparity half of the motivation was checked again and stands
unchanged: GRPO's direction is dominated by whichever component has the
largest spread, because it standardises the raw sum.

No code change -- `normalize_gdpo_decoupled` already computed this correctly.
What was wrong was every sentence describing it.
A third adversarial review round found two defects that two prior rounds
missed, both outside the GDPO maths those rounds concentrated on.

Two components summing to a constant should standardise to exact opposites
and cancel. Instead the ill-conditioned division left a residue that step 3
divided by the std of that very residue. Measured on rewards summing to
308.95172119140625, a group carrying no signal came out at
`[-0.5770, 1.1539, -0.5770]` — finite, plausible, and pointing wherever the
last bits of the reward happened to fall.

Two changes, and it is worth separating them because the tempting summary
("the floor stops a fake gradient") is not what happens:

- `extract_reward_components` builds float64. This is the half that matters:
  the residue drops from 1e-1 (larger than the signal) to 1e-10, and
  `GDPO_EPS` in step 3's denominator caps the amplification at ~1e-6.
- `combine_group` applies a noise floor, so cancellation is *exactly* zero.
  1e-6 is not zero, and consumers test for zero: the dynamic-sampling filter
  would keep a group contributing nothing. The threshold is computed, not
  tuned — `component_noise_scale` returns `eps * max|x| / (std + GDPO_EPS)`,
  the condition number of the standardisation, which is fifteen orders below
  the signal on well-conditioned rewards and matches the observed residue
  (bound 5.1e-10, measured 4.1e-10) on the pathological one.

This was previously documented as an unfixable deviation. That was wrong on
the diagnosis as well as the priority: it is not a precision limit, it is a
gap in the collapse check.

`relax/distributed/ray/rollout.py` did not filter `reward is None` while its
twin in `relax/agentic/rollout.py` did, so the shared helper built a float32
tensor from a None and raised `TypeError` — from a logging helper, on the
rollout's way out. `reward is None` is reachable: under `--group-rm` the group
reward is assigned in one shot that is skipped entirely when the rollout
aborts, which is why `sglang_rollout.py`'s "reward is not None" assert exempts
`group_rm` in the first place. All single-reward algorithms were affected,
including the default.

Separately, eval may run a different reward model (`EvalConfig.rm_type`), so
an eval reward legitimately need not carry `--gdpo-reward-keys`. Metrics were
enforcing a training contract that is not in force there, and failing eval for
it.

Both decisions now live in `metrics_group_verdict`, which returns a tri-state
and is importable without `sglang` — the two copies had drifted apart once and
neither was testable, let alone tested.

`_whiten_by_segment` treated `mini_batch_sizes=None` as "use one window". That
is not a coarser version of the same objective: the repository's own
`test_merging_the_batches_would_flip_signs_not_just_rescale` shows half the
advantages changing sign, with every metric finite. It now raises, through the
existing MAX all-reduce so the whole group fails together.

`--gdpo-reward-weights` is checked for non-finite and all-zero-in-float32
values during argument validation rather than at the first rollout.

- `advantage_normalization` had no test pinning its value — dropping
  `"token_global"` from a REINFORCE++ spec, or flipping the comparison in
  `loss.py`, left the suite green. Both call sites and the exact set are now
  transcribed from main.
- `advantage_gae` had only a `co_names` check. It is now compared numerically
  against a transcription of main's inline branch. PPO is the algorithm this
  refactor changed most and the one with no GPU smoke.
- `test_grouping_uses_group_index` only ever built a contiguous layout, so a
  position-based implementation satisfied it. It now interleaves two groups.
- Every guard above was mutation-tested: each one, removed, turns a test red.

The previous round's corrections landed in the main implementation and left
the periphery behind. The smoke script header still carried the motivation
retracted in 9f1e02e; the filter docstring described "every component is flat"
where the code asks whether the combination cancels; a test docstring cited a
`--global-batch-size` the shipped example does not use; the overflow comment
described a mean that no longer overflows; `.detach()` was called
"load-bearing" when both call sites pass lists and never reach it.

`tests/algorithms` 745 passed. Full suite 1686 passed, with the same 2
failures and 2 errors present on the base 98a1274 (verified in a detached
worktree). `pre-commit run --all-files` clean.
Two defects on either side of `combine_group`, both from a decision that was
made in the wrong place rather than made wrongly.

# 🐛 Bug Fix

## The zero-std metrics still died on an unscored sample

`metrics_group_verdict` moved the *verdict* into one testable place and left
the *label* behind in both copies of `_compute_zero_std_metrics`, where they
promptly disagreed: the agentic copy reads it off the first scored sample, the
distributed copy off `group[0]`, scored or not. The crash it was introduced to
remove did not go away, it moved one line down -- from
`torch.tensor([None], dtype=torch.float32)` to `round(None, 1)`, still a
`TypeError` out of a logging helper on the rollout's way out.

Two ways in, and they are not the same bug:

- A group where nothing was scored. The distributed copy counts those
  deliberately (`is not False`), then asks for a label that cannot exist.
  Reachable on `98a1274` too, where the old predicate also counted them:
  pre-existing, not a regression.
- A flat group whose first sample is the unscored one. `[None, 0.5, 0.5]`
  verdicts as flat and `group[0]` is the None. The old predicate returned
  False here and never reached the label, so this one *is* a regression.

`zero_std_group_label` now answers "which reward does this group get filed
under" in one place, off the first scored sample, returning `None` when there
is no scored sample at all -- not a third policy, just the absence of a label
to file it under. Both callers drop that group. The source assertion that
pinned "both copies delegate the verdict" now pins the label too, since
splitting one decision out and leaving its other half behind is how these
copies drifted the second time.

## The noise floor could zero a group that carries signal

`component_noise_scale` is `eps * max|x| / (std + GDPO_EPS)`, so it grows with
the ratio of a reward's magnitude to its own spread. The claim it shipped with
-- fifteen orders of margin, nothing real is ever suppressed -- holds on
well-conditioned input, which is where the only test measured it. It does not
hold generally:

| base | spread | floor  | \|combined\|max | zeroed |
| ---- | ------ | ------ | --------------- | ------ |
| 1e9  | 0.1    | 1.8e-5 | 0.999           | no     |
| 1e12 | 1e-3   | 1.65   | 0.907           | yes    |
| 1e15 | 1.0    | 1.78   | 0.9999          | yes    |

The last two carry an order-1 combined advantage and are zeroed anyway. Such a
reward is finite, well under `_FLOAT32_MAX`, and passes every check in
`extract_reward_components`.

Zeroing it is silent in both directions: groups below the floor are dropped by
the filter with no log at all, and when every group is, the batch warning names
two causes -- rewards that do not vary, weights that cancel -- neither of which
is what happened, so it reads as a reward-function bug that is not there.

`extract_reward_components` already refuses a reward that overflows float32
rather than casting it to `inf`, on the grounds that a silently zeroed
component is indistinguishable from a genuinely collapsed one. A reward with no
significant digits where it varies is the same failure one stage later, and now
gets the same answer.

`_MAX_COMPONENT_NOISE` is 0.01. The values it bounds have unit variance, so it
reads as a fraction: at 1% of the standardised value being rounding, the
gradient's direction is partly noise whether or not the floor zeroes the group.
It needs |reward| to exceed the group's own spread by about 1e14 to trip.

The check lives in `combine_group` rather than its callers for the same reason
the label now does: there are two of them, and a check in one is a check the
other disagrees with. Raising from there also keeps the existing division of
labour -- the filter, which decides whether a group trains, propagates it; the
zero-std metrics turn it into "cannot tell" via `observed_reward_signal` and
keep logging. The observer does not become the enforcer, and eval, which may
run a different reward model entirely, is not taken down by a training-time
contract.

---

# ✅ Tests

## The label, not just the verdict

- A flat group beginning with an unscored sample is counted *and* filed under
  the reward its scored samples carry.
- A group with nothing scored has no label.

`relax/distributed/ray/rollout.py` imports `sglang` and cannot be loaded on
CPU, so these run against the shared helper -- which is why it exists.

## Both directions of the threshold

- A reward with no significant digits raises, and the message names the
  offending component rather than an index.
- A reward that is merely large (1e9 with a spread of 0.1, eight significant
  digits) is left alone.
- The constant-sum group the floor exists for still reaches the floor and
  still comes out as exactly zero.
- The metrics report the unreadable group as unknown while the filter refuses
  it.

Every guard above was mutation-tested. Restoring `group[0]` reds the label
tests with the production `TypeError`, not an assertion mismatch; removing the
noise check reds three; tightening the threshold to 1e-18 reds ten
*pre-existing* tests. The constant is bounded from both sides, not chosen.

## Verified on GPU

Three Modal H100 smokes on this tree, 8 optimizer steps each: GDPO on 1 and 2
GPUs at `--global-batch-size 16` -- the first runs to reach
`_whiten_by_segment`'s multi-segment path and its MAX all-reduce -- and GRPO on
1 GPU for the registry refactor's default algorithm. Both label paths produced
real counts (`zero_std/count_1.0`, `count_1`, `count_0`) rather than raising,
and no noise check fired on GSM8K's `{0, 1}` rewards, thirteen orders below the
threshold.
# 📝 Documentation

## Point at the guarantee instead of restating the symptom

`_whiten_by_segment` lists "segment k holds training batch k on every rank" as
relied on and not verified, and said the orders agree because `actor.py`
fetches and appends in `batch_index` order. That is true and insufficient: two
loops both counting upwards is not a reason for their k-th items to be the
same batch.

The reason is that the fetch is addressed rather than popped.
`_get_data_from_transfer_queue` passes `batch_index` to the TransferQueue
sampler, which keys its replay cache on
`(partition_id, task_name, dp_rank, batch_index)`, so two ranks asking for the
same `batch_index` receive their own shards of the same logical mini-batch.

No behaviour change. It matters because the property is unverifiable from
inside this function -- it receives a flat tensor and a list of lengths, with
no batch identity in either -- so a reader who wants to know whether anything
holds it up has to go find that out, and the docstring as written suggested
nobody did.
An adversarial consult over 3c616d6 (four independent models, eight reports)
found the guard it added does not do what its commit message claims, and that
three sentences of that message are wrong. Every measurement below was
reproduced locally before being acted on.

# ⏪ Revert

## The per-column noise guard could not be made to hold

It raised when any column's `component_noise_scale` exceeded 0.01. Six
measurements, each reproducible on CPU:

- **It does not catch what it was added to catch.** At `base = 4.05e13` two
  columns measure `noise = [0.0090, 0.0095]` -- both under the threshold, so
  the guard passes -- and the floor, 0.148, still zeroes a real combined
  signal of 0.033. The guard sat just above the failure it was named for.
- **The floor outgrows any per-column bound.** The floor is
  `8 * sum(|w_k| * noise_k)`; a per-column test is a max. With 16 components
  at `noise_k = 0.0064` the floor reaches 0.82 and takes a signal of 0.03..0.07
  while every column reads cleaner than 1%. The two quantities do not scale
  together, so no constant relates them.
- **It fires on components that contribute nothing.** `weights = [0.0, 1.0]`
  with a broken column 0 raises, though `resolve_gdpo_weights` documents a
  zero weight as the way to mute a component and the floor correctly ignores
  it. An operator disabling a known-bad component still loses the run.
- **float32-provenance rewards walk straight past.** The estimate uses
  float64's eps. A column varying by exactly one float32 ulp at 1e9 -- its
  variation *is* rounding -- measures 1.9e-9 and standardises to ±2.
- **It weakens as the group grows.** The same offset and step trips at
  `n = 3` and passes at `n >= 100`, because `std` is a sample estimate.
- **The threshold was not bounded by anything.** 0.01 and 0.1 are
  indistinguishable to the suite; the window is roughly (5e-10, 0.222).

A guard that misses the failure it names, fires on configurations the same
file documents as supported, and is blind to the most common reward
provenance is worth less than the lines it occupies.

## Three claims in 3c616d6's message are wrong

- "bounded from both sides, not chosen" -- the mutation argument constrains
  the threshold to about nine orders of magnitude. Having a bound is not the
  same as the bound being tight.
- "reds ten pre-existing tests" -- it reds 23. The number came from reading a
  truncated `head -10` of the failure list.
- "eval ... is not taken down by a training-time contract" -- `KeyError` is
  outside the `(TypeError, ValueError)` that `observed_reward_signal` catches,
  so an eval reward model with a different schema takes the metrics down at
  the label, past the point that guards them. Fixed below.

---

# 🐛 Bug Fix

## The label helper called a sample scored when only its container was

`zero_std_group_label` tested `sample.reward is not None`. That is not the
same as the *value* being readable, and both gaps reach `round(None, 1)` --
the exact TypeError 3c616d6 set out to remove:

- `reward` is a dict and `--reward-key` selects a `None` out of it (a
  partially failed reward function). The dict is not None.
- `reward` is a dict without `--reward-key` at all, raising `KeyError`, which
  `observed_reward_signal` does not catch. Eval may legitimately run a
  different reward model (`EvalConfig.rm_type`).

The helper now looks for a sample whose reward it can actually read, and
returns None when there is none. It is a logging helper; the stage that
consumes the reward still refuses the same input.

---

# 📝 Documentation

## The floor's real limits, written down where it lives

`combine_group`'s docstring now carries the three measurements above -- the
floor outgrowing any per-column bound, the two-column case, and the float64
provenance assumption -- plus a fourth: `component_noise_scale` reads as a
fraction of the standardised value only while `std >> GDPO_EPS`. Below that
the denominator is clamped and a column measuring 0.0022 is 20.7% contaminated.

This is the part of the withdrawn guard that was worth keeping. The limitation
is real and now stated where the next person will find it, instead of being
half-mitigated by a check that raised on the wrong groups.

---

# ✅ Tests

## Characterisation, not aspiration

The five tests written for the guard are replaced by four that assert the
limitation itself -- each is a group the guard would have passed while the
floor zeroes real signal. They fail if someone fixes the floor, which is the
correct time to revisit them.

Two more pin the label holes: a dict holding a None, and a dict missing the
key entirely.

Mutation-tested both ways: disabling the floor reds five tests including all
the new characterisation ones; removing the `value is None` check reds the new
label test. 755 passed.
…ut it

Three findings from review, all in the same place: the boundary between what
GDPO computes and what it decides.

**The noise floor is gone.** `combine_group` ended with an all-or-nothing
threshold that returned zeros when the combined advantage fell below
`8 * sum(|w_k| * noise_k)`. It is not in Eq. 7, and the tests written to
document it proved it destroyed real signal: two components at `base = 4.05e13`
with every column measuring under 1% noise gave a floor of 0.148 against a true
signal of 0.033, and the floor grows with the component count while any
per-column measure is a max, so sixteen clean columns reached 0.82.

Pinning that as a "known limitation" was the wrong call. The argument for
keeping it was that float64 alone leaves 1.4e-10 on a cancelling group rather
than a hard zero -- but `GDPO_EPS` caps step 3's amplification, so what reaches
the optimizer is 1.2e-6, and the worst pathological case found is 2.6e-3. It
traded a 1e-3 error for a 1e-1 one, silently. Both numbers are now tests.

What replaces it is a split, not a smaller threshold. The reward stage returns
Eq. 7 untouched and *reports* a group whose combined signal is six orders below
the terms that produced it. The filter -- `group_carries_reward_signal`, which
decides whether a group is worth training on at all -- applies a tolerance,
because that is a filtering question and it is the same judgement GRPO's
`std > 0` already makes. One test pins the asymmetry so the two cannot be
collapsed back together.

**Weights stay float64 until the transport cast.** They were cast to float32
one stage before the arithmetic needed it, while the components had been
float64 since the previous round. `[16777216, 16777217]` are two distinct
configured weights that become the same float32: on a pair of anti-correlated
components the configured weights give about +-1 and the quantised ones gave 0.
The float32 checks stay -- the combined reward really is carried in float32 --
but as validation of what will survive that cast, not as the dtype of the
multiply.

**A non-finite advantage raises instead of reading as zero variance.**
`whiten_scalar` treated a non-finite std as collapse and returned zeros. The
reward stage verifies finiteness in float64; `dict_to_tensordict` and
`_as_reward_tensor` then cast to float32, so two same-direction components at
weight 3e38 combine to a finite 5.999e38 and arrive as `inf`. The batch was
zeroed and nothing was logged. The second half of that guard -- non-finite std
from finite input -- is deleted rather than converted, because
`distributed_mean_std` accumulates in float64 and it cannot fire.

Also: `combine_group` is now checked against an independent transcription of
Eq. 4 and Eq. 7 in plain Python, and a second test shows the clamped
denominator is the only place the implementation departs from the paper.

Tests: tests/algorithms 782 passed; full CPU suite 1774 passed, 323 skipped,
with the same 2 failures + 2 errors as main@4899b8f. pre-commit clean.
Every other step-3 test called `advantage_gdpo` or `whiten_scalar` directly, so
the wiring between them and `loss.py` was covered only by a regex over the
source. That regex pins the text; it says nothing about the numbers. Deleting
the `mini_batch_sizes=` argument, or pointing `gdpo` at a different
`advantage_fn`, left every numerical test green.

These four call `loss.compute_advantages_and_returns` with a real
`rollout_data` dict and check what comes back: whitened per training batch,
different from whitening the merged rollout, raising rather than defaulting
when the segmentation is absent, and not sharing GRPO's branch.

Megatron is stubbed rather than skipped. An `importorskip` would mean the file
never runs where the rest of the suite runs, which is the same hole in a
different shape; only the four `mpu` entry points this path touches are faked,
each returning the single-process answer.

Both mutations verified: removing the `mini_batch_sizes` forward turns 2 of the
4 red, routing `gdpo` to `grpo_broadcast` turns 3 red.

Tests: tests/algorithms 786 passed; full CPU suite 1778 passed, 323 skipped,
same 2 failures + 2 errors as main@4899b8f. pre-commit clean.
…asked

A four-model consultation on the previous commit found that two of its three
fixes were wrong, and that one of them introduced a silent failure. This
undoes both and closes three further holes found on the way.

**The relative "is this rounding?" ratio is gone.** It replaced the noise floor
last round and was worse, because it was wrong in kind rather than in
calibration. `magnitude` scales with the *difference* between the weights while
`sum_k |w_k| max|z_k|` scales with their *magnitude*, so the ratio measured the
weight configuration and not the data -- at G=2 it reduces exactly to
`|w1 - w2| / (|w1| + |w2|)`, independent of every reward value in the group.
Measured on real inputs it was inverted in both directions at once: it dropped
a group whose final advantage was 0.43 and kept one whose 1.08 was pure
rounding. The counterexample that settles it uses this branch's own fixture --
`test_weights_closer_than_float32_stay_distinct` asserts the combined value is
real signal that must survive, and the filter scored the same input at 2.98e-8
and threw it away.

No threshold fixes that. For G >= 3 the centred subspace is at least
two-dimensional, so `z_2 = -z_1 + delta*u` with `u` orthogonal to `z_1` is a
genuine signal with an arbitrarily small ratio.

`group_carries_reward_signal` now asks exactly what the single-reward branch
asks, on exactly the values the trainer receives: `min != max` in float32. Zero
weights and exact cancellation are still detected -- they produce exact zeros --
and near-cancellation is not, which is now stated as an accepted cost rather
than papered over.

**`whiten_scalar` was silently zeroing finite batches.** The comment removing
its second guard argued float64 accumulation cannot overflow. True and
irrelevant: `distributed_mean_std` returns `std.to(values.dtype)`, so
`[-FMAX, FMAX]` -- two finite float32 values -- gives a float64 std of
`sqrt(2)*FMAX` that becomes `inf` on the way back, and the division returned an
all-zero batch. A second path needs no non-finite std at all:
`[FMAX]*10 + [-FMAX]` has finite statistics and the float32 `values - mean`
still overflows to `-inf`. Whitening now runs in float64 throughout and casts
the normalised result back, which closes both. (The arithmetic in that comment
was also wrong: ~1.5e231, not 1e153.)

**`--custom-config-path` could bypass three RLOO constraints.** Splitting
validation into four functions for ordering reasons left only two wired into
the override path, so a YAML file could select rloo and then set `--kl-coef`,
`--num-steps-per-rollout 4`, or a `global_batch_size` that breaks the
one-update guarantee. All four now re-run; ordering does not apply after the
merge because every value they read is final.

**Two smaller ones, both from the rebase.** `min_group_size` was declared twice
in `AlgorithmSpec` (the second silently won and orphaned the first's
docstring), and a docstring still pointed at `component_noise_scale`, deleted
last round.

**The filter's drop label could crash the rollout.** It read
`--reward-key` off the first sample, on the drop path only, while the signal
test above reads only the component keys -- so a multi-reward run whose rewards
carry `--gdpo-reward-keys` but not the scalar raised `KeyError` out of the
rollout loop. It now uses `zero_std_group_label`, which the metrics side was
already using to refuse the identical input.

**What is not fixed, and is now documented as such.** A constant sum leaves a
remainder of roughly `ulp(C) / spread`, which grows with the magnitude the
components are centred on. At C=1e9 it reaches the optimizer at 2.6e-3; at
C=1e13, or with both columns large and straddling a binade, at O(1). Nothing
detects it. The previous commit claimed float64 had capped this at 1e-6, and
the test asserting so was named "worst measured" while pinning a single point;
it now measures three decades and asserts the growth. A companion test records
that these are batch-of-one figures -- in a batch that also holds healthy
groups the same remainder arrives around 1e-7.

Two claims from the consultation were checked and are wrong, recorded so they
are not re-investigated: the dynamic filter is not reachable from eval (both
`generate_rollout` entry points branch to `eval_rollout` before it), and the
removed floor's own failure cases score 1.6e-2 and 1.9e-3 under the ratio
criterion, so they were never inherited by it.

Tests: tests/algorithms 796 passed; full CPU suite 1788 passed, 323 skipped,
same 2 failures + 2 errors as main@4899b8f. The YAML fix is mutation-verified:
dropping the three re-added validators turns 3 tests red. pre-commit clean.
…measured

A five-model consultation on the previous commit. Three defects it introduced,
one it made worse, and four statements in it that measurement contradicts.

**The finiteness check was a deadlock.** `whiten_scalar` raised on a local
`isfinite` two lines above `is_collapsed`'s all-reduce, so a rank whose shard
held the bad value left while every other rank blocked in a reduction it never
reached. `is_collapsed`'s own comment warns about exactly this, and
`_agree_on_segmentation` was written earlier on this branch to prevent it. The
check is now one MAX all-reduce of a flag, so every rank raises together.

**A legitimate config was newly rejected.** Re-running `validate_batch_shape`
after the YAML merge was half a fix: it reads `global_batch_size`, which the
main path derives from `num_steps_per_rollout` *before* the merge. A YAML file
switching grpo@4-steps to rloo@1-step should get `rollout * n = 128` and
instead was refused against the stale 32. The derivation is now a function both
paths call, with the consistency assert skipped on the second run because the
value it would compare against is the pre-merge one.

**Two contract holes closed.** `whiten_scalar` truncated integer input to
all-zero (from the new cast back to the caller's dtype) and still returned
zeros for float64 near its own maximum, where the squares overflow inside the
reduction. Non-floating input and non-finite statistics now raise.

**A missing reward key is no longer filed as a flat group.** Routing the drop
label through the tolerant metrics helper fixed a `KeyError` but put "this
group scored the same everywhere" and "this reward is missing a key the run
requires" in one bucket -- a schema that always omits the scalar would drop and
resample forever behind a zero-std count. It gets `unreadable_reward` now.

**Four statements the consultation falsified, all mine:**

- "in a batch that also holds healthy groups the same remainder arrives around
  1e-7" -- six of the ten reports flagged this. It is the `C = 1e9` entry of a
  table, written as a general rule. Sharing a whitening unit divides by the
  healthy groups' standard deviation, a constant factor of a few hundred; it
  does not slow the growth with `C`, and at `C = 1e13` a shared unit still
  delivers 5e-3. Worse, the unit is the *training batch*, so a degenerate group
  isolated into its own segment gets no division at all. The docstring now
  carries the measured table and both caveats; the test is parametrised over
  three decades and a companion pins the segment case.
- "at G = 2 it reduces exactly to |w1 - w2| / (|w1| + |w2|), independent of
  every reward value in the group" -- only when the two standardised columns
  come out as exact opposites. Columns that move together give a ratio of 1.
  The G >= 3 construction is what settles it; the G = 2 identity was decoration
  and is now marked as the error it was.
- "this function exists to have no hole" -- it closes the algorithm-validator
  holes. The `--ref-load` check, the `kl_coef`/`kl_loss_coef` assert,
  `_normalize_sync_ppo_kl_args`, the fully-async resource checks and the
  `rollout_batch_size` derivation all still run before the merge. The comment
  now lists them instead of claiming coverage it does not have.
- `grows_without_bound` -- each standardised column is itself bounded, so the
  combination saturates: 1e8 to 1e9 is 11x, 1e11 to 1e13 only 6x. Renamed, and
  the assertions pin monotone growth plus the two ratios that hold rather than
  a per-decade factor that does not.

**The filter's safety is borrowed, and now pinned.** `min != max` is only
equivalent to "carries signal" because Eq. 4 centres every column, forcing the
combined values to sum to zero within the group so that "all equal" means "all
zero". Nothing enforced that. A companion test shows why it matters: step 3
whitens a training batch, not a group, so a constant nonzero group would *not*
be whitened away -- the reason the old comment gave for preferring `min != max`
was simply wrong.

Also: five comments that described the pre-fix behaviour (non-finite std read
as collapse; the criterion described as "non-zero").

Tests: tests/algorithms 802 passed; full CPU suite 1794 passed, 323 skipped,
same 2 failures + 2 errors as main@4899b8f. pre-commit clean.

Two consultation claims were checked and are wrong, recorded so they are not
re-investigated: `[-FMAX, FMAX]` is *not* still silently zeroed (the narrowing
cast is inside `distributed_mean_std`, whose parameter is now the float64
copy), and weights of (1e4, -1e4) on a constant-sum group do not expose the
mixed-batch claim -- opposite weights on anti-correlated columns reinforce, so
that group carries full-scale real signal rather than a residue.
…view left

The previous commit made the finiteness check collective so all ranks fail
together. Nothing exercised it: single-process tests take the
`process_group is None` branch, where the reduction is skipped entirely. So the
fix for a deadlock was only ever run in the configuration that cannot deadlock.

`test_a_non_finite_shard_fails_both_ranks_instead_of_hanging_one` spawns two
gloo ranks with the infinity on one of them and asserts both come back
refusing. Mutation-verified the only way this one can be: putting the local
`isfinite` back makes the test hang -- killed at 75s, against 30s for the
suite -- because rank 0 sits in `is_collapsed`'s all-reduce that rank 1 left.

**`silent_groups` asked a different question from the filter.** It counted
groups whose float64 combination was exactly zero while
`group_carries_reward_signal` asks whether the float32 view varies, so the
warning an operator reads and the sampler's decision could disagree about the
same group. Both now use the float32 predicate, and the message says what it
now means.

**GDPO declares `forbids_reward_side_kl`.** `advantage_gdpo` hands `kl` to
`get_grpo_returns`, which uses it for shape only -- the values are discarded,
so `--kl-coef` buys a reference forward pass and changes no advantage. The
whole `grpo_broadcast` family has this property and only `rloo` declared it;
saying so is free for a new algorithm because no existing configuration is
refused, and the four older members are deliberately left alone since rejecting
a flag they accept today is an upstream decision. `--use-kl-loss` is unaffected.

Tests: tests/algorithms 805 passed, including 9 in the two-rank gloo file.
The review asked for "a CPU/Gloo test that actually goes through
loss.compute_advantages_and_returns -> registry -> gdpo". What existed was two
halves of that and not their intersection: `test_gdpo_loss_wiring.py` walks the
chain with a stubbed `mpu` in one process, and the gloo cases in this file use
a real group but call `whiten_scalar` directly.

So a `process_group=` dropped from the call in `loss.py` passed both. Two ranks
would each whiten their own shard, every rank would come back with plausible
numbers, and nothing would say so.

This spawns two gloo ranks, points a fake `mpu` at the real group, and calls
the production entry point. Rank 1's rewards are twice rank 0's, so shared
statistics keep the ratio visible while per-shard whitening flattens both onto
the same values. Mutation-verified: setting `process_group=None` in `loss.py`
turns this one red and leaves the other nine green.

Tests: tests/algorithms 806 passed.
`observed_reward_signal` exists so a reporting stage cannot decide whether
training continues: it answers "cannot tell" where the strict version raises.
It caught `(TypeError, ValueError)`, which covers `get_reward_components` --
that one raises ValueError for a missing key -- but not the single-reward
branch, which goes through `Sample.get_reward_value`. That is a bare subscript
(`relax/utils/types.py:171`), so a `--reward-key` absent from the reward dict
raised KeyError straight through the handler and took the metrics down. Eval
may legitimately use a different reward schema (`EvalConfig.rm_type`), so this
is reachable, not hypothetical.

Widening the handler rather than narrowing `get_reward_value`: that accessor is
shared with the dynamic-sampling filters and the rollout metrics, so changing
what it raises is a contract change for callers outside this branch. The
comment at the call site records that, and the root cause's location.

A test asserts all three levels at once -- `group_carries_reward_signal` still
raises KeyError, `observed_reward_signal` returns None, `metrics_group_verdict`
returns None -- and reverting the except tuple turns it red. The docstring of
the neighbouring label test claimed the gap as a known limitation; it no longer
is, so it says what it actually covers.

Three stale claims in `rewards.py` docstrings are corrected in place rather
than retracted underneath themselves. `combine_group` stated the G = 2 ratio
identity as fact and took it back three paragraphs later, so a reader going top
to bottom met the false version first; it now states the correct, conditional
version once. The remainder table's caption and the filter comment at the
`min != max` test both quoted "around 1e-7" as the general mixed-batch case --
it is the C = 1e9 entry of a column that grows to 5.2e-3 at C = 1e13, and a
degenerate group isolated in its own training batch gets no division at all.
Copilot AI review requested due to automatic review settings August 24, 2026 04:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 42 out of 43 changed files in this pull request and generated 2 comments.

Comment thread relax/algorithms/numerics.py Outdated
Comment on lines +85 to +92
bad = torch.tensor(
float(not bool(torch.isfinite(values).all())),
dtype=torch.float32,
device=values.device,
)
if process_group is not None:
dist.all_reduce(bad, op=dist.ReduceOp.MAX, group=process_group)
return bool(bad.item())
Comment thread relax/algorithms/advantages.py Outdated
return torch.zeros_like(values)
work = values.double()
mean, std = distributed_mean_std(work, process_group=process_group)
if not (torch.isfinite(mean) and torch.isfinite(std)):
@Men1scus

Copy link
Copy Markdown
Author

三条意见和您要的那条接线测试都做了,当前 head 65ed22e

先说一句范围:这条 PR 的 base 还是 main,所以 GitHub 上看到的 diff 里包含 #276 的 3131 行。GDPO 本身的净增量是 5113 行 / 37 文件(git diff pr1/algorithm-registry pr2/gdpo)。#276 合入后这个 diff 会自动缩下去。

[P1] noise floor 删了

combine_group 现在只算 Eq. 7 的 Σₖ wₖzᵢₖ,没有别的。您的判断是对的——那些测试是在把一个已知的偏差固定下来当成正确行为。

我第一版的替代方案(按相对幅度判断「这是不是纯舍入误差」)也是错的,已经删掉。原因:那个比值的分子随两个权重的变化,分母随权重的大小变化,所以它量的是权重怎么配的,不是数据长什么样。实测两个方向都错——扔掉了一组最终 advantage 是 0.43 的真信号,又放行了一组 1.08 的纯舍入。而且这不是调阈值能救的:组内样本数 ≥ 3 时,可以构造出比值任意小但确实是真信号的输入,任何固定阈值都有反例。

所以现在不做任何「聪明」判断:filter 就在训练实际用的 float32 精度上问一个问题——这一组的值是不是全都一样(min != max),和单奖励那条分支问的是同一个问题。代价是两个分量几乎抵消的情况检不出来,这一点我写在代码里了,不再声称有判据。

[P1] 权重精度

配置里的权重和分量合并全程用 float64,只在交给训练的那一步转 float32。

[P2] 交给 float32 时显式失败

白化的统计和归一化都在 float64 里算,最后才转回调用方的 dtype。

有一点想请您重点看,因为这段是新写的:判断「有没有非有限值」现在是一次跨 rank 的 all-reduce,不是每个 rank 自己判。 原因是它后面紧跟着 is_collapsed,那里面有个集合通信;如果某个 rank 本地判出问题就直接 raise,其他 rank 会一直等在那个 all-reduce 上,整个训练挂死而不是报错退出。现在所有 rank 一起失败。撤回成本地判断,测试会挂住(75 秒被杀,正常跑完是 30 秒)。

接线测试

按您说的补了:test_the_megatron_entry_point_reaches_gdpo_with_a_shared_statistic。它起两个 gloo rank,用一个假的 mpu 指向真实的 process group,然后走生产入口 loss.compute_advantages_and_returns → 注册表 → gdpo。rank 1 的 reward 是 rank 0 的两倍:统计量共享时这个倍数关系保留得下来,如果每个 rank 各白化各的分片,两边就被拉平了。把 loss.py 里的 process_group 传参去掉,只有这一条变红,同文件另外九条照样绿。

还修了 Copilot 提的一条

observed_reward_signal 只接住 TypeErrorValueError。多奖励那条路走 get_reward_components,缺 key 时抛的确实是 ValueError;但单奖励那条路走 Sample.get_reward_value,它是直接 self.reward[args.reward_key] 取值、不判 key 在不在(types.py:171),缺 key 就抛 KeyError,这个 except 接不住,指标计算会把整个 rollout 带崩——而这个函数存在的全部意义就是防这件事。eval 用不同 schema 的 reward model 是合法的,所以这条路真能走到。

我选择加宽这里的 except,而不是去改 get_reward_value 抛什么异常:那个访问器还被动态采样 filter 和 rollout 指标共用,改它抛的异常类型对本 PR 之外的调用方是个契约变更。这个取舍和根因位置都写在调用点的注释里了。

验证

tests/algorithms 807 通过;全量 1799 通过 / 323 跳过,失败集与 main@4899b8f 逐条一致;pre-commit 通过;GitHub CI 五项全绿。H100 冒烟:1 卡和 2 卡(dp_size=2)GDPO 都跑通了。

还没解决的,如实列出——合入这个 PR 就等于接受这一条

两个分量之和恒定时(比如 correctness + format 总是 1),它们标准化之后正好互为相反数,等权重相加应该是 0,但浮点运算留下的舍入残差不是 0,而且这个残差随分量的基数增大。目前没有可靠办法把它和真信号区分开,两种试过的机制都被证伪了,所以我没有再加任何阈值去悄悄把它清零。

补一句量级,因为我之前在这里以偏概全过:如果这一组和健康组落在同一个训练批里,残差会被健康组的标准差除一下,是个几百倍的常数——但它不改变随基数增长的趋势。基数 1e9 时混批是 3.1e-7,基数 1e13 时混批还有 5.2e-3;而且「白化单元」是每个训练批,不是整个 rollout,退化组要是自己独占一个批,这个除法压根不发生。

文档

PR 描述里那句关于恒和分量的话已经改了。RFC #218 §2.2 里同样的说法也已更正,记在 #218 (comment) —— 那条评论给出了替换文本和在当前实现上的验证,不劳您代改。

Both guards on the whitening path forced the GPU to wait on the CPU once per
training batch, for answers the device could have produced itself.

`whiten_scalar` spelled its finiteness check `isfinite(mean) and isfinite(std)`.
Python's `and` calls `__bool__` on the first tensor and, when that is true, on
the second, so the readable spelling costs one or two device-to-host syncs.
Stacking the pair reduces once and reads once.

`any_rank_has_non_finite` was worse: it called `bool(...)` to decide what to
put *into* the flag tensor, so it synced before the collective and then copied
the answer back to the device to rebuild it. Building the flag with tensor ops
keeps the whole thing on device; the trailing `.item()` is the only sync left
and is unavoidable while the function returns a Python `bool`.

Neither guard changes behaviour, which is the problem with claiming so: mutation
testing found the `whiten_scalar` guard had no test at all. Deleting it left all
807 tests green. It is reachable only from a float64 caller near its own maximum
-- float32 cannot get there, 1.2e77 against float64's 1.8e308 -- and unguarded
it divides the batch by infinity and returns all zeros, which is indistinguishable
from "every sample scored the same". A test now pins that, and reverting the
guard turns it red.

The flag's cast off `bool` got the same treatment and survived, which first read
as "this line carries no meaning". It does not: MAX over 0/1 is dtype-independent,
but whether a backend *accepts* the dtype is not, and `relax/utils/device.py`
picks the backend from the accelerator -- hccl on NPU, xccl on XPU, neither
checkable from here. A two-process NCCL run on the production image (torch 2.11,
NCCL 2.28.9) confirms bool is fine on CUDA, so the cast is precaution for the
backends we cannot run, not a fix for anything reproduced. The test asserts
"not bool" rather than "== float32": float32 is only what the pre-rewrite code
sent, and int32 -- what every other flag in this repo uses -- must stay a legal
future choice. Pinning the exact dtype would copy the implementation into a test.

The docstring says all of this because an earlier draft of it said something
false. It justified float32 by rolling-upgrade dtype skew, which cannot happen
here: these are data-parallel ranks from one launch of one image.
…f stalling

These cases exist because one rank raising while another proceeds strands the
second inside a collective, and they record the exception rather than raising
so that both ranks coming back is distinguishable from one that never does.
`init_process_group` had no timeout, so gloo's 30-minute default decided how
long "never" takes.

Mutation testing made the cost concrete: deleting the `all_reduce` from
`any_rank_has_non_finite` -- a real defect, it loses the cross-rank signal
entirely -- did not fail the suite, it hung it past 300s. At the default that
regression reaches CI as a stalled job, which reads as infrastructure flake and
gets retried rather than investigated. With a 60s bound the same mutation fails
in 106s. Every collective here is sub-millisecond and world_size is 2, so the
bound is headroom, not a budget.

Scope: this file only. Six other bare `init_process_group` sites across
tests/distributed, tests/backends/megatron and tests/utils have the same
30-minute default and are left alone -- an earlier version of this claimed the
blast radius was a single site, which was wrong.
@li126com

Copy link
Copy Markdown
Member

此前 review 指出的 GDPO 数值问题已经处理:非公式 noise floor 已移除,reward weights 和合并使用
float64,float32 交接后的非有限值会让所有 DP rank 一致失败,并补充了真实 loss → registry → gdpo
的 Gloo 测试。当前未发现新的 GDPO 公式或 DP 分段错误。

不过当前 head b92ce2d 仍建议 Request changes。

1. [验收阻塞] 新增 Megatron 接线测试无法在项目标准环境运行

test_gdpo_loss_wiring.py::_load_loss_module()
(https://github.com/redai-infra/Relax/blob/b92ce2d56e3acfd7584568290a778c1246a38e4e/tests/algorithms/test_gdpo_loss_wiring.py#L36-L61)
只有在无法导入 megatron.core 时才安装 fake mpu。但项目 docker/Dockerfile
(https://github.com/redai-infra/Relax/blob/b92ce2d56e3acfd7584568290a778c1246a38e4e/docker/Dockerfile#L96-L106)
会安装 Megatron 并加入 PYTHONPATH,于是测试使用真实 mpu,却没有初始化 pipeline process group。

在当前项目环境中实测:

pytest -q tests/algorithms
4 failed, 805 passed

AssertionError: pipeline_model parallel group is not initialized

四个失败均来自 test_gdpo_loss_wiring.py。GitHub CI 全绿是因为其 CPU runner 没有 MCore,只覆盖了
测试的另一条分支。

请无论 Megatron 是否已安装,都显式 monkeypatch loss_module.mpu 为单进程测试实现,同时继续调用真
实的 loss.compute_advantages_and_returns,然后在项目镜像中重跑该文件和 tests/algorithms。这属
于“CPU 测试通过”的直接验收项。

2. RFC 正文与最终实现不一致

RFC #218 (#218) 正文仍有多处过期或错误内容:

  • §2.2 仍称等权重 (1,0)/(0,1) 恒和 reward 可被 GDPO 保留;实际上标准化后两列互为相反数,仍会抵消
    为零;

  • §7.4 写 mini_batch_sizes=None 会回退为单段,而当前实现会报错;

  • §8 写内置 filter 仍按单标量判断,而当前已经改成多 reward 感知;

  • 测试数量、运行证据和“GPU recipe 未运行”等描述也已过期。

只在 issue 评论里更正不足以形成稳定的设计记录。请更新 RFC 正文,或者在正文顶部明确标记已被最终
PR 文档取代并链接到唯一的最终说明。

非阻塞验证说明

当前没有完整 GDPO 训练曲线,只有单卡/双卡 4~8 step smoke。这不是题面硬性要求,不应单独阻塞验
收。不过这些 smoke 使用的是 force-push 前的 SHA,而当前 head 后来修改过每 batch 执行的分布式
guard;建议最终 rebase 后补一次当前 head 的短双卡 smoke,并保存可访问的日志摘要。

此外,1e300 这类 Python 中有限、转 float32 后为 inf 的 reward weight 会通过启动校验,直到首次
reward 处理才失败。当前不会静默训练错误,因此不是公式 blocker,但建议在参数校验阶段补
torch.isfinite(survives_cast).all(),避免集群启动后才报错。

最后请先完成并合入 #276,再将 #277 rebase 到新的 main,使 #277 的最终 diff 只保留 GDPO 增量。
请优先推进 #276 的工作。

…MCore is installed

`_load_loss_module` installed a fake `mpu` only when `import megatron.core`
raised. `docker/Dockerfile` installs Megatron and puts it on PYTHONPATH, so in
the project image the import succeeds, the real `mpu` answers, and nothing here
initialises a pipeline group -- all four cases died on `pipeline_model parallel
group is not initialized`. GitHub's runner has no MCore and only ever ran the
other branch, so `pytest -q tests/algorithms` reported `4 failed, 805 passed`
in the image while CI stayed green. Reported in review; this is that fix.

The import still needs the fake package when Megatron is absent, but which
`mpu` the code talks to is now decided per test by an autouse fixture, so both
environments take the same path.

It patches the functions *on* the `mpu` object rather than rebinding
`loss_module.mpu`, because rebinding one name is not enough: `loss.py` is not
the only module doing `from megatron.core import mpu`, and
`cp_utils.maybe_padded_total_lengths` -- reached from
`compute_advantages_and_returns` -- holds its own reference. With only the loss
module rebound, the suite still failed in the image, at cp_utils.py:31. Every
holder shares one module object, so patching its attributes reaches all of
them, and `monkeypatch` restores them afterwards rather than leaking a stub
into test modules that want the real thing.

Verified in both configurations, since passing in one is exactly what hid this:
a fake `megatron.core` whose every `mpu` call raises simulates the image on a
machine with no MCore. Reverting the fixture reproduces the reported split --
4 passed without Megatron, 4 failed with it. Full `tests/algorithms` is
809 passed either way.
@Men1scus

Copy link
Copy Markdown
Author

两项阻塞都已处理,head cb7d181,在 b92ce2d 上追加,没有 rebase。

1. [验收阻塞] 接线测试无法在项目标准环境运行

改成 autouse fixture,无论 Megatron 装没装都绑定单进程 mpu,loss.compute_advantages_and_returns 仍走真实实现。

一个细节值得单独说:我第一版只重新绑定了 loss_module.mpu,不够——loss.py 不是唯一 from megatron.core import mpu 的模块,cp_utils.maybe_padded_total_lengths(从 compute_advantages_and_returns 进去)持有自己的引用,那一版在镜像里仍然失败在 cp_utils.py:31。现在打的是 mpu 模块对象上的函数,所有持有者共享同一个对象,一次覆盖到位;monkeypatch 逐用例还原,不会把桩泄漏给其他需要真 mpu 的测试文件。

在项目镜像里实测,不是在本机模拟:ghcr.io/redai-infra/relaxrl:latest(README 让用户 pull 的那个;其 MEGATRON_BRIDGE_COMMIT=2faedbf6…docker/Dockerfile 里 pin 的一致,megatron.core 解析到 /root/Megatron-LM/megatron/core,megatron-core 0.18.0,torch 2.11.0+cu129),pytest tests/algorithms809 passed。撤掉 fixture 能精确复现您报的分裂:无 Megatron 4 passed、有 Megatron 4 failed。

2. RFC #218 正文已更新

四处都核实过,不是照单接受:

  • §2.2 的例子是错的,而且是必然错:两分量总和恒定时 format = C − correctness,标准化后两列精确互为相反数,等权合并恒为零——不只是 (1,0)/(0,1) 那个举例不成立,是这一类都不成立。原文引 test_gdpo.py:240 作佐证也引错了,那条测的是「一分量塌缩、另一分量有变化」,而那种情形下 GRPO 的总和其实也是变化的。已改写为四条边界,写明真正被救回的是权重不等、或三分量以上方差不同的情形。
  • §7.4:mini_batch_sizes=None 实测抛 ValueError,不是回退单段。已更正并链到 test_a_missing_segmentation_fails_instead_of_defaulting
  • §8:内置 filter 已是多 reward 感知,group_carries_reward_signal 按真正的合并 advantage 判定;它的 docstring 里正好记录了上面那个抵消情形。
  • §9:数字几乎全过期(test_reward_normalizers.py 从 7 到 398,总数 690 → 809)。改成钉 SHA 并注明会随提交移动、以 CI 为准,同时把「本机不装 Megatron 而镜像装」这个教训写进了环境说明。

非阻塞两条

双卡 smoke 已在当前 head 补跑,正如您指出的,旧 smoke 早于这次「每 batch 分布式 guard」的改动。2×H100、同一个 relaxrl:latest 镜像、cb7d181:

step loss grad_norm pg_clipfrac ppo_kl
0 0.1918 1.7357 0.001297 0.000678
1 0.2404 1.8117 0.001393 0.000534
2 0.2044 1.6130 0.000910 0.000435
3 0.2494 1.1951 0.001306 0.000623

rollout/advantages 均值约 -1.2e-08,这是 step 3 白化的设计结果,单看它无法与「整批塌缩为零」区分;能区分的是 grad_norm 1.6–1.8——若 advantage 全零,policy gradient 会是零。raw_reward 在 0.719 / 0.594 / 0.500 / 0.375 之间移动,reward 函数是活的。全程无 Traceback、无 CUDA error,GDPO 的两条 guard 均未触发。

日志与摘要:relax-modal 仓库 report/gdpo-2gpu-guard-rewrite-cb7d181/。要说明的是这只有 4 个训练 step,不是收敛曲线——它验的是改写后的 guard 能在真实 DP 组和真实 optimizer step 下工作,不代表最终质量。

另外顺带做了一次两进程 NCCL 冒烟(同镜像,2×H100),确认非有限值 flag 在 bool/uint8/int32/int64/float32 × 标量/[1] 形状下归约都正常、跨 rank 一致失败也成立。这不是训练 smoke,不能替代上面那条。

1e300 那条建议在参数校验阶段加 torch.isfinite(survives_cast).all(),合理,我会在下一轮补上。

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.

3 participants