Skip to content

Fix the default IS mode in the GRPOConfig docstring - #7751

Merged
sergiopaniego merged 1 commit into
mainfrom
fix-is-mode-docstring
Oct 19, 2026
Merged

sergiopaniego merged 1 commit into
mainfrom
fix-is-mode-docstring

Conversation

@sergiopaniego

@sergiopaniego sergiopaniego commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

The GRPOConfig docstring labels token_truncate as the default vllm_importance_sampling_mode, but the default is sequence_mask, as the field and the docstring header already say. This removes the stale label.

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

  • 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
Docstring-only change with no effect on code paths or configuration defaults.

Overview
Corrects a documentation-only mismatch in GRPOConfig: the vllm_importance_sampling_mode enum list no longer marks "token_truncate" as the default.

The actual default remains "sequence_mask", as already stated on the parameter line and in the dataclass field. The "token_truncate" bullet is also lightly reworded (drops the stale “(default)” label and tightens the clipping range text).

No runtime or training behavior changes.

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

@bot-ci-comment

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@albertvillanova albertvillanova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

Same stale default in the TIP at docs/source/grpo_trainer.md:221: "By default, Truncated Importance Sampling is activated". Since #4530 the default is sequence-level Masked IS. Could you fix it here too?

@qgallouedec qgallouedec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👌

@sergiopaniego
sergiopaniego merged commit 0d5389d into main Oct 19, 2026
11 checks passed
@sergiopaniego
sergiopaniego deleted the fix-is-mode-docstring branch October 19, 2026 19:54
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