Skip to content

[training_utils, trainer, tests] feat: raw log-prob mismatch metrics for rollout vs actor - #8025

Merged
wuxibin89 merged 1 commit into
verl-project:mainfrom
cr-gao:feat/rollout-logprob-mismatch-metric
Oct 22, 2026
Merged

wuxibin89 merged 1 commit into
verl-project:mainfrom
cr-gao:feat/rollout-logprob-mismatch-metric

Conversation

@cr-gao

@cr-gao cr-gao commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds three raw log-prob metrics next to the existing training/rollout_probs_diff_*:

  • training/rollout_logprobs_mismatch_count: valid response tokens whose rollout_log_probs != old_log_probs
  • training/rollout_logprobs_diff_max / _mean: |Δ| of the raw log-probs

rollout_probs_diff_* compares exp(log_prob) and cannot see a 1-ULP log-prob difference (float32 -0.10000000149 vs -0.10000000894 give the same prob). Exact 0-diff train-rollout work (#7688, verl-project/verl-omni#637) needs a gate that only passes at zero elements, so the count is exposed directly. In the V1 trainer the count is aggregated by sum, not mean.

Emitted from calculate_debug_metrics, so every existing caller (v1 trainer, legacy trainer, separation trainer) gets it under rollout.calculate_log_probs=True with no config change.

Checklist Before Starting

Test

pytest tests/utils/debug/test_metrics.py tests/trainer/ppo/v1/test_metrics_aggregator_on_cpu.py -q
# 23 passed

New cases: 1-ULP float32 pair where rollout_probs_diff_max == 0 but mismatch_count == 1; all-masked batch returns valid=0, mismatch_count=0; aggregator sums the count across iterations.

API and Usage Example

No API change. Three new keys in the metrics dict when actor_rollout_ref.rollout.calculate_log_probs=True.

Design & Code Changes

  • verl/utils/debug/metrics.py: compute the raw comparison alongside the prob-based one; default keys on an all-masked batch.
  • verl/trainer/ppo/v1/utils.py: rollout_logprobs_mismatch_count uses the sum aggregation rule.
  • tests as above.

Checklist Before Submitting

  • Read the Contribute Guide.
  • pre-commit run --files <changed files>: ruff, ruff format, mypy, license, docstring, structure checks pass.
  • Documentation: docstring only; no docs page lists per-metric keys for this file.
  • Unit tests added (CPU).

AI assistance was used for this change. I reviewed every line and ran the tests.

…for rollout vs actor

Signed-off-by: cr-gao <gaochenrui@sjtu.edu.cn>

@SamitHuang SamitHuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Useful gate for exact 0-diff train-rollout work: rollout_probs_diff_* really can hide 1-ULP log-prob differences, and summing mismatch_count in the V1 aggregator is the right aggregation for a zero-element check.

One correctness nits on the inline comment: keep the mismatch count on the same .float() path as rollout_logprobs_diff, otherwise mixed bf16/fp32 batches can disagree between the count and the abs-diff metrics.

Also:

  • test_all_masked_returns_invalid only checks mismatch_count == 0; please also assert the new *_diff_max / *_diff_mean stay nan on the early-return path.
  • The checklist says no docs page lists these keys, but docs/ascend_tutorial/zh/dev_guide/model_dev/parameter_and_metrics.md already documents training/rollout_probs_diff_* — worth adding the three new keys there (or correcting the checklist).

rollout_logprobs_diff = calculate_log_prob_diff(
actor_old_log_probs.float(), rollout_old_log_probs.float(), response_mask_bool
)
logprobs_mismatch = torch.masked_select(actor_old_log_probs != rollout_old_log_probs, response_mask_bool)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

!= compares the native dtypes, while rollout_logprobs_diff above casts both sides with .float() first. If old_log_probs / rollout_log_probs ever differ in dtype (bf16 vs fp32 is common), the mismatch count and the abs-diff metrics can disagree. Prefer deriving the count from the already-casted tensor, e.g. int((rollout_logprobs_diff != 0).sum().item()), so both metrics share one comparison path.

@wuxibin89
wuxibin89 merged commit 16ec093 into verl-project:main Oct 22, 2026
57 of 66 checks passed
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