fix(scorer): tag math() answer-extraction failures with reason metadata (#4026)#4091
fix(scorer): tag math() answer-extraction failures with reason metadata (#4026)#4091antnewman wants to merge 5 commits into
Conversation
…wer fails Sister fix to UKGovernmentBEIS#4048 (model_graded_qa / model_graded_fact), addressing the same silent-failure family flagged in UKGovernmentBEIS#4026. When extract_answer() returns None — the model output contains no parseable mathematical expression — the scorer's else-branch returned Score(value=INCORRECT, answer="None"). This conflated two semantically different cases: - "the model answered, and was wrong" - "the model didn't produce a parseable answer at all" In aggregate metrics the two became indistinguishable: a sample where the model failed to format a math answer was silently counted as a wrong answer, understating both true accuracy and parse-failure rate. The fix returns NOANSWER (the same sentinel pattern() uses for unparseable output, and the same convention UKGovernmentBEIS#4048 adopts for model_graded scorers) and preserves the original completion as the `answer` field for downstream observability. Two existing tests asserted the pre-fix behaviour and have been updated to expect NOANSWER instead: - test_no_boxed "not a number" case: directly exercises the parse-fail path, now correctly asserts NOANSWER. - test_infinite_float_does_not_crash: the intent was "no crash on inf" — extract_answer returns None for infinity, so the parse-fail branch applies; NOANSWER is the right semantic. Added test_parse_failure_yields_noanswer (parametrised over 4 representative parse-fail completions) and test_parse_success_with_wrong_value_yields_incorrect (sanity check that genuine wrong answers still get INCORRECT).
|
On further analysis, I'm not sure whether this is the right approach, though it's something of a judgement call. I think this was a mistake, and should potentially be reverted. My reasoning follows.
I think it's helpful to look at examples. For the question "What is 1+1? Answer with just the number", and a regex pattern of "\d+" we could have answers like:
Cases 1, 2, and 8 are clear-cut and easy to distinguish. Cases 3-7 are less clear, as they are non-trivial to distinguish from each other, and should ideally have different outputs. 3, 4 and 5 are "tried and failed", 6 and 7 are "didn't try". If we can identify that a response like "I cannot answer that" is a refusal, it should definitely be marked as It's non-trivial to distinguish between cases 3-7, particularly when using pattern-based scorers. They're all seen as a regex failure-to-match. If we use a model-based graders, it becomes possible to distinguish between them, but that increases the expense and duration of the eval. My thinking is that if the model is unable to follow the instructions in order to produce an answer in the requested format, the model has failed and should be marked as @dragonstyle do you have thoughts here? |
|
Thanks @MattFisher, this is a fair challenge, and having re-read it I think you're right about The thing I'd add is that "parse failure" hasn't meant the same thing in all the scorers this has been applied to. In Where I'd still gently push back is the model-graded scorers (#4048), because there the thing being parsed isn't the answer, it's the grader's verdict. When a small judge writes So my honest read is that the answer probably isn't uniform: INCORRECT for the answer-extraction scorers (this PR included), with NOANSWER kept for genuine refusals and for grader-side parse failures where there's a signal. Either way I'm sold on your metadata idea. Even the Happy to switch this PR to INCORRECT + |
I agree completely - an unparseable output from a grade model should not result in |
|
I think one other class of example that might be useful to consider is:
Especially in case 5, the answer doesn't seem incorrect though the format wasn't correct. It's not explicitly a refusal to answer, though perhaps failing to conform to the requested format could be interpreted as a refusal? The other question that occurs to me is whether it is more useful to distinguish between answers that are definitively right, definitely wrong, and wrong for some other reason (e.g. refusal, wrong format, etc..). In terms of metrics these all compute to the same value, but simply marking a failure to answer properly as wrong might have the effect of hiding the failure (masking it as just a wrong answer). |
|
That case 5 is the one that gets me too — Your second point is the one I keep coming back to: whatever single value we pick, it collapses "wrong answer", "wrong format", and "refused" into the same number, and that hides useful signal. So maybe the value and the reason are two separate questions. Where I've landed, trying to hold both your and @MattFisher's points: keep the value as Score(
value=INCORRECT,
explanation=f"Scoring pattern not matched in output: {state.output.completion}",
metadata={"reason": NO_MATCH},
)That way the headline metric behaves the way Matt argues it should, but anyone digging into a run can still separate "genuinely wrong" from "couldn't parse the format" from "refused" — you're not forced to choose one meaning for the value. If that's the direction you're happy with, I'll align What standard would you like to settle on? |
|
I agree something like The recent change here to |
|
I'm fleshing out my suggestion here and will link to this comment from other places. I think our best bet is introducing
If "reason" seems like too generic a word that other evals might already be using, "score_reason" is an alternative. |
|
See also UKGovernmentBEIS/inspect_evals#1933 |
|
Just noticed the |
…nswer-on-parse-fail # Conflicts: # CHANGELOG.md
When extract_answer() cannot parse a mathematical expression from the
completion, keep value=INCORRECT (a format violation is the model
failing the task and must not inflate accuracy by leaving the
denominator) and record metadata={"reason": "invalid_response_format"}
so analysis can separate "wrong answer" from "couldn't parse an
answer". Follows the convention discussed in UKGovernmentBEIS#4091 in place of the
earlier NOANSWER approach.
Also stop recording the literal string "None" as the extracted answer
on parse failure (answer is now None with an explanatory message).
|
The Two things I ran into while verifying the design that seem worth having on the table before the vocabulary ossifies: Score edits silently destroy metadata-carried reasons. On On unifying the key: #4048's One tiny vocabulary note: the docs example uses |
| return Score( | ||
| value=INCORRECT, | ||
| answer=None, | ||
| explanation="No mathematical expression could be extracted " | ||
| + f"from the model output: {state.output.completion}", | ||
| metadata={"reason": "invalid_response_format"}, | ||
| ) |
There was a problem hiding this comment.
I think it would be worth setting answer=state.output.completion here, as the scorer docs say:
If you are extracting an answer from within a completion (e.g. looking for text using a regex pattern, looking at the beginning or end of the completion, etc.) you should strive to always return an answer as part of your Score, as this makes it much easier to understand the details of scoring when viewing the eval log file.
and in that case it wouldn't need to be in explanation as well.
| return Score( | |
| value=INCORRECT, | |
| answer=None, | |
| explanation="No mathematical expression could be extracted " | |
| + f"from the model output: {state.output.completion}", | |
| metadata={"reason": "invalid_response_format"}, | |
| ) | |
| return Score( | |
| value=INCORRECT, | |
| answer=state.output.completion, | |
| explanation="No mathematical expression could be extracted from the model output", | |
| metadata={"reason": "invalid_response_format"}, | |
| ) |
There was a problem hiding this comment.
Applied in 5b12409dd — thanks for the docs pointer, I had missed that guidance. Completion surfaced as answer, explanation trimmed to the static message, and the test now asserts the round-trip.
…nswer-on-parse-fail # Conflicts: # CHANGELOG.md
…ilure Per review: the scorer docs ask extraction scorers to always return an answer, so the parse-failure branch now sets answer=state.output.completion and drops the completion from the (now static) explanation.
Sister fix to #4048 (
model_graded_qa/model_graded_fact), addressing the same silent-failure family flagged in #4026. Per the discussion there, this is the second-PR follow-up I committed to picking up.Problem
math()insrc/inspect_ai/scorer/_math.pyreturnsScore(value=INCORRECT, answer="None")whenextract_answer()returnsNone(no parseable mathematical expression in the model output). This conflates:In analysis the two become indistinguishable: a sample where the model failed to format a math answer is silently recorded as an ordinary wrong answer (with the literal string
"None"as its extracted answer), hiding the parse-failure rate entirely.Fix
On parse failure, keep
value=INCORRECT— a format violation is the model failing the task, and must not inflate accuracy by leaving the denominator — and tag the failure mode:This follows @MattFisher's proposed abnormal-score vocabulary in the review discussion (
invalid_response_formatfor model-under-test format violations), consistent with themetadata={"reason": ...}pattern already used in the docs' unscored example and adopted grader-side in UKGovernmentBEIS/inspect_evals#1933.Also fixes the latent
answer="None"artefact: on parse failureansweris nowNonerather than the stringifiedNone.Genuine wrong-answer cases — where
extract_answer()succeeds butcheck_answers()returnsFalse— are unchanged: plainINCORRECT, noreasontag.Tests
test_parse_failure_tags_reason_metadata— parametrised over 4 representative parse-fail completions (no numeric content, words only, empty completion, ambiguous equals); assertsINCORRECT+metadata={"reason": "invalid_response_format"}+answer is None.test_parse_success_with_wrong_value_has_no_reason— a parseable-but-wrong answer (\boxed{5}vs target42) stays plainINCORRECTwith no metadata.test_infinite_float_does_not_crash—extract_answerreturnsNonefor infinity, so the parse-fail branch applies (intent of the test, "no crash on inf", unchanged).All 43 tests pass locally (Python 3.13, Windows).
Relationship to other work
model_graded_qa/model_graded_factsilently default toINCORRECTwhen the judge's verdict doesn't match the grade regex #4026 (the issue): vladmesh's analysis covers both_model.pyand_math.pyas the two clear instances of this silent-failure pattern.Score.unscored()withmetadata["unscored_reason"]— a different key than this PR; see the review discussion for the standardisation question.metadata["reason"]with the grader-side vocabulary.This PR contains:
Does this PR introduce a breaking change?
The headline value is unchanged (
INCORRECTbefore and after), soaccuracy()/mean()and value-distribution dashboards are unaffected. The only observable changes areScore.metadatagaining{"reason": "invalid_response_format"}on parse failures, andScore.answerbecomingNoneinstead of the literal string"None".