Skip to content

[CI] assert train_rollout_logprob_abs_diff <= 0.1 (regression guard) - #94

Merged
CalvinXKY merged 2 commits into
mainfrom
sync-slime-logprob-abs-diff-assert
Jun 2, 2026
Merged

[CI] assert train_rollout_logprob_abs_diff <= 0.1 (regression guard)#94
CalvinXKY merged 2 commits into
mainfrom
sync-slime-logprob-abs-diff-assert

Conversation

@aoshen02

Copy link
Copy Markdown
Collaborator

What

Backports slime #1968.

We already compute and report train/train_rollout_logprob_abs_diff (the mean abs diff between training-time and rollout-time log-probs) in loss.py, but nothing asserts on it. This adds the CI guard:

if args.ci_test and "train/train_rollout_logprob_abs_diff" in log_dict:
    assert log_dict["train/train_rollout_logprob_abs_diff"] <= 0.1, f"{log_dict=}"

Why

vime #69 ("[Bugfix] Fix high train_rollout_logprob_abs_diff") fixed exactly this divergence. Without an assertion, the same class of regression can creep back in silently. This is the missing regression guard for #69--ci-test runs now fail fast if train/rollout logprobs diverge past 0.1.

Decoupling

Independent of #92 (GLM bridge) and #93 (R3 KL guards). Single 3-line insert, no overlap with the lines #93 touches in this file.

Test

ast.parse clean. Guard only fires under --ci-test when the metric is present.

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a CI test assertion in slime/backends/megatron_utils/model.py to ensure that the absolute difference in training rollout log probabilities (train/train_rollout_logprob_abs_diff) is less than or equal to 0.1. There are no review comments, and I have no feedback to provide.

Backports slime #1968. We already report train/train_rollout_logprob_abs_diff
(loss.py) but never assert on it. This adds the CI guard so the train/rollout
logprob divergence that #69 fixed cannot silently regress.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
@aoshen02
aoshen02 force-pushed the sync-slime-logprob-abs-diff-assert branch from 658a412 to 765122d Compare May 31, 2026 02:56
@CalvinXKY
CalvinXKY merged commit 9defcfa into main Jun 2, 2026
13 of 17 checks passed
momo609 pushed a commit that referenced this pull request Jun 8, 2026
…94)

Backports slime #1968. We already report train/train_rollout_logprob_abs_diff
(loss.py) but never assert on it. This adds the CI guard so the train/rollout
logprob divergence that #69 fixed cannot silently regress.

Signed-off-by: aoshen02 <aoshen@inferact.ai>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@aoshen02
aoshen02 deleted the sync-slime-logprob-abs-diff-assert branch June 8, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants