Skip to content

docs: fix the clip explanation in the GRPO trainer page - #7190

Open
JiangLLM wants to merge 1 commit into
huggingface:mainfrom
JiangLLM:fix/grpo-clip-old-policy
Open

docs: fix the clip explanation in the GRPO trainer page#7190
JiangLLM wants to merge 1 commit into
huggingface:mainfrom
JiangLLM:fix/grpo-clip-old-policy

Conversation

@JiangLLM

@JiangLLM JiangLLM commented Sep 12, 2026

Copy link
Copy Markdown

What does this PR do?

The GRPO trainer page explains the clip term with this sentence:

where clip(·, 1−ε, 1+ε) ensures that updates do not deviate excessively from the reference policy by bounding the policy ratio between 1−ε and 1+ε.

The ratio inside the clip is π_θ / π_θ_old. So the clip term limits how far the update moves from the old policy, not from the reference policy. The reference policy is a separate model in this loss. It appears only in the KL penalty, which the page covers one section above.

I changed "reference policy" to "old policy" and added the symbol. One line, docs only.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Was this discussed/approved via a GitHub issue? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

AI writing disclosure

We welcome the use of AI tools to help with contributions. For transparency and to help us improve our review process, please indicate the level of AI involvement in this PR.

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Note

Low Risk
Documentation-only wording fix with no runtime or API impact.

Overview
Corrects a terminology mistake in the GRPO trainer docs for the clipped surrogate objective: the explanation of clip(·, 1−ε, 1+ε) now says it limits how far updates move from the old policy ( \pi_{\theta_{\text{old}}} ), not the reference policy.

That matches the ratio ( \pi_\theta / \pi_{\theta_{\text{old}}} ) in the displayed loss; the reference policy remains only in the separate KL term above.

Reviewed by Cursor Bugbot for commit 5a5b6b0. Bugbot is set up for automated code reviews on this repo. Configure here.

@JiangLLM
JiangLLM force-pushed the fix/grpo-clip-old-policy branch from 6d33646 to 5a5b6b0 Compare September 12, 2026 07:29
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.

1 participant