Skip to content

perf(grpo): skip the redundant pre-update policy forward when it cannot differ - #10348

Closed
rrrxxx0510 wants to merge 1 commit into
modelscope:mainfrom
rrrxxx0510:perf/grpo-skip-redundant-old-logps
Closed

rrrxxx0510 wants to merge 1 commit into
modelscope:mainfrom
rrrxxx0510:perf/grpo-skip-redundant-old-logps

Conversation

@rrrxxx0510

@rrrxxx0510 rrrxxx0510 commented Oct 13, 2026 •

Copy link
Copy Markdown

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

PR information

Fixes #10358

_prepare_batch_inputs 每次都会跑一遍前向来填 old_per_token_logps。但默认设置下(num_iterations=1,且 rollout 那批样本在同一个 optimizer step 内就消费完),"更新前的策略"和"当前策略"是同一份权重——这次前向算出来的结果和 per_token_logps.detach() 完全一样,PPO 比值恒为 1,clip 项不起作用。算 loss 那里本来就靠这一点兜底(old_per_token_logps = per_token_logps.detach() if ... is None else ...),所以它根本不需要这个字段。

这次给那遍前向加了开关 _need_old_per_token_logps,复用了 trainer 里现成的 old_policy(),并在下列情况保留它——它们的共同点是直接读这个字段、没有 per_token_logps.detach() 兜底:

  • kl_in_reward(KL 直接用这个字段算),只在 beta != 0 时
  • teacher KL / RLSD / OPD(teacher_per_token_logps)
  • Liger 路径和 rollout 重要性修正(use_liger_kernel、rollout_per_token_logps)
  • async_generate

Experiment results

在 H200 上插了临时探针:改动前 old_per_token_logps 不是 None,且每一步都与 per_token_logps.detach() 逐位相同(torch.equal,max_abs_diff=0.0)。改动后该字段为 None、走兜底,日志里所有指标不变——loss、grad_norm、completions/{mean,min,max}_length、clipped_ratio、reward、reward_std、kl、全部 clip_ratio/*,峰值显存也一致。

省掉的是什么 —— 在 H200 上用 Qwen2.5-0.5B 实测,一次「completion 前向 + 算 logps」的开销:

batch × seqlen tokens 耗时 该次前向的峰值
8 × 1024 8192 65.2 ms 5.80 GiB
16 × 1024 16384 56.3 ms 10.64 GiB
16 × 2048 32768 109.2 ms 20.31 GiB

峰值的大头是 [B, T, V] 的 logits,不随模型变小而缩水;计算量随模型变大而增长,7B 上省的时间比这些数字大得多。

测试见 tests/general/test_grpo_old_logps_gate.py,纯 CPU,不需要 GPU 或模型。

Checklist

  • I have linked the related issue above, if there is one.
  • I have reviewed and understand every changed line, and I can explain how the change works and why it is correct.
  • I have run the relevant tests or reproductions locally, and included the commands and results above (if applicable).
  • I am responsible for the content of this PR, regardless of how it was produced.

@rrrxxx0510

Copy link
Copy Markdown
Author

Hi @hjh0119 @tastelikefeet - sorry to ping directly. Same situation as #10337 from this fork: every workflow run is parked at action_required, so CI never runs.

This one drops the pre-update policy forward in GRPO when the weights cannot have moved (num_iterations=1 and the rollout batch consumed within one optimizer step). A probe confirmed it is bit-for-bit identical to per_token_logps.detach() (max_abs_diff=0.0), and the logged metrics are unchanged after skipping it.

Could one of you approve the workflow runs when you get a chance?

@hjh0119

hjh0119 commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

@hjh0119 hjh0119 closed this Oct 13, 2026
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.

perf: GRPO runs a redundant pre-update policy forward under the default setup

2 participants