Conversation
GRPO only decodes generated tokens, so templates that already open <think> in the prompt never reach the reward. Make the opening tag optional and keep the no-nested-think lookahead. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Verdict: correct, with two documentation changes requested. It resolves #6966 and lowers no score that was 1.0 before. What I checked
Issue #6966 minimal case, scored on the completion string GRPO decodes (no download needed): head No regression. Scoring both patterns over every string of up to 4 atoms from That follows from the structure: Mutants scored against the PR's own valid and invalid lists: All killed, so the test is a real oracle. Findings
Not reproducedNothing. Every claim above was executed. UnevaluableThe end-to-end GRPO run: I scored the string the trainer decodes rather than generating it, so the reward path is verified but not the tokenizer round trip on a real DeepSeek checkpoint. Out of scope: with |
|
@behroozazarkhalili |
81d1bbc to
814ae90
Compare
State that think_format_reward requires </think> with an optional leading <think>, including under templates that do not prefill. Drop the partial model list from the comment, and add deliberately accepted </think>-only strings to the valid tests.
a787c68 to
994fc59
Compare
|
@behroozazarkhalili Thx for review. I updated the docstring and added the two boundary test cases you suggested. |
|
@Ruinique after talking with @qgallouedec I reopened this work as #7141 from my fork so it can be reviewed. Your commits are cherry-picked there with your authorship intact and the PR body credits you; if you would rather carry it yourself, say so there and I will close mine. |
…the prompt GRPO decodes only the generated tokens, so models whose chat template prefills <think> into the prompt scored 0.0 on every well-formed completion. The opening tag is now optional, </think> is still required, and completions that start with <think> keep scoring 1.0. Based on the approach proposed in huggingface#6995. Fixes huggingface#6966.
What does this PR do?
think_format_rewardcurrently requires the completion to start with<think>. GRPO only decodes generated tokens (grpo_trainer.py), so models whose chat template already prefills that tag into the prompt — DeepSeek-R1 distill, and Qwen3.5/3.6 with thinking enabled — score0.0on every well-formed completion. A constant reward gives zero advantage, so the function contributes nothing for the whole run.This PR makes the opening
<think>tag optional, keeps the existing no-nested-<think>lookahead, and still requires</think>. Completions that already start with<think>(Qwen3) keep scoring 1.0.Fixes #6966
Before submitting
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.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.
Made with Cursor