Repository navigation
fix(officeqa): ignore sentence-ending periods when normalizing answers - #379
Open
Lanre Shittu (Shizoqua) wants to merge 1 commit into
Open
Lanre Shittu (Shizoqua) wants to merge 1 commit into
Lanre Shittu (Shizoqua) wants to merge 1 commit into
Conversation
normalize_answer keeps "." so decimals such as 1234.5 survive, but that also kept periods that end a sentence. A correct answer written as "42." did not match the gold "42", and as a single token it also got zero F1. hard is the exact-match result and soft is F1, so these answers received no reward during training and counted as misses in reported scores. Drop a period unless a digit follows it. Decimal points, signs and percent signs still affect the match.
This branch has not been deployed
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.
Summary
Fixes #377.
OfficeQA answer normalization kept a sentence-ending period, so a correct answer such as
42.did not match the gold42and scored zero exact match and zero F1. A period is now dropped unless a digit follows it, so decimals are unaffected.User impact:
hardis exact match andsoftis F1, so a correct numeric answer that ended with a period got no reward during training and counted as a miss in reported scores.Proof: the regression tests fail on upstream
6ee29a0and pass with this change.Root cause
normalize_answer()inskillopt/envs/officeqa/evaluator.pyremoves punctuation except characters in_NUMERIC_CHARS(0123456789.-) and%. Keeping.is needed for decimals, but it also kept periods that end a sentence.42.stayed42., which differs from42for exact match and is a different token for F1.Changes
normalize_answer()removes any.that is not followed by a digit (re.sub(r"\.(?!\d)", "", text)).1.5vs15,0.5vs5and-3vs3still score 0.tests/test_officeqa_evaluator.py, testing throughevaluate(), which is what the rollout calls.Reproduction
Before:
After:
1.0 1.0for all four.Testing
python -m pytest tests/test_officeqa_evaluator.py -q: on upstream6ee29a0, 6 failed (assert 0.0 == 1.0and similar) and 4 passed; with the fix, 10 passed.python -m pytest tests/test_officeqa_evaluator.py tests/test_officeqa_parallel_search.py tests/test_benchmark_task_output_paths.py -q: 58 passed.python -m pytest -q: 2079 passed, 12 skipped, 4 xfailed.python -m ruff checkon the changed files: oneI001import-spacing finding on lines that are unchanged frommain.python -m mkdocs build --strict: passed.Risk and compatibility
Normalization is applied to both prediction and gold, so any pair that matched before still matches. Abbreviations such as
U.S.now normalize tous, which makesU.S.andUSmatch. This raises OfficeQA scores for runs whose target model ends answers with a period, so results from before and after this fix are not directly comparable. Numeric equivalence such as3.20vs3.2is unchanged. No backend, config key, file format or CLI change.