Skip unparseable gold solutions in cosine_scaled_reward - #734
Open
shaurya416 wants to merge 1 commit into
Open
shaurya416 wants to merge 1 commit into
shaurya416 wants to merge 1 commit into
Conversation
When the gold solution could not be parsed, cosine_scaled_reward returned 1.0, the maximum reward, for every completion, so a malformed or missing gold answer rewarded the model for any output. accuracy_reward already returns None for that case since huggingface#566, which lets the trainer skip the example. Return None here too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #732
Problem
get_cosine_scaled_rewardreturns1.0for a completion whenever the gold solution cannot be parsed (rewards.append(1.0) # Skip unparseable examples).1.0is the defaultmax_value_correct, so an example with a malformed or missing gold answer gives the model the maximum reward for any output, including one that is plainly wrong. The comment says the example is skipped, but a numeric reward is not a skip.accuracy_rewardhandles the same case by returningNone, so the trainer skips the example (#566). This change does the same incosine_scaled_reward.Change
rewards.append(None)instead of1.0whenlen(gold_parsed) == 0, and the return annotationlist[Optional[float]]as onaccuracy_reward.tests/test_rewards.py: an unparseable gold gives[None, None]for completions of different lengths, and in a mixed batch only the unparseable example isNone.Not changed:
len_rewardhas the same shape (correctness.append(True)), buttest_unparseable_solutionpins its current 0.5, so I left it for a separate decision. The optional follow-up from the issue (droppingextraction_config=[LatexExtractionConfig()]on the gold parse so bare answers stay scorable) is also left out.Validation
I ran the real
tests/test_rewards.pyagainst the realrewards.pywithmath-verify==0.5.2andlatex2sympy2_extended, from a stub package that replaces only the modules the reward functions under test never call (code_providers,competitive_programming,configs).TestGetRewardFuncswas deselected because it needs the realconfigs.mainThe one failing existing test,
TestRepetitionPenaltyReward::test_full_repetition_with_language, fails the same way before and after becausetransformersis not installed in this environment.ruff format --check --line-length 119passes on the two files.