Skip to content

fix(whiten): scale-only path when shift_mean is false - #2356

Open
YeonwooSung wants to merge 1 commit into
THUDM:mainfrom
YeonwooSung:fix/1369-shift-mean-scale-only
Open

fix(whiten): scale-only path when shift_mean is false#2356
YeonwooSung wants to merge 1 commit into
THUDM:mainfrom
YeonwooSung:fix/1369-shift-mean-scale-only

Conversation

@YeonwooSung

Copy link
Copy Markdown

Fixes #1369.

distributed_masked_whiten(..., shift_mean=False) used to center, scale, then add the mean back:

whitened_values = (values - global_mean) * rsqrt(var)
if not shift_mean:
    whitened_values += global_mean

That is not a scale-only path. This change makes shift_mean do what it says:

  • shift_mean=True (default): (values - global_mean) * rsqrt(global_var + epsilon) — zero-mean, unit-variance
  • shift_mean=False: values * rsqrt(global_var + epsilon) — scale only; do not subtract the mean, and do not add it back

Both paths still share the same Bessel-corrected global variance estimate.

The live GRPO caller in slime/backends/megatron_utils/loss.py already uses shift_mean=True, so the production whitening path is unchanged.

A CPU unit test (NUM_GPUS = 0) mocks torch.distributed.all_reduce as a no-op so it can run without a process group, and pins the two formulas plus the shared variance.

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.

【BUG】wrong implementation of zero-centered mean when normalizing advantages

1 participant