fix(scorer): mark model_graded grade-parse failure as unscored (not INCORRECT)#4048
Conversation
avalyset
left a comment
There was a problem hiding this comment.
Change looks right to me. Returning NOANSWER from the parse-failure branch lines up with the existing pattern() convention in _pattern.py:101, and the distinction between "judge couldn't be parsed" and "judge said incorrect" is exactly what the aggregate needs to stay visible — losing it silently understates accuracy in a way that's hard to notice downstream.
One non-blocking observation on the test side: the parse-failure path is well covered by the four parametrised cases, but there's no companion assertion that a valid GRADE: I still resolves to INCORRECT. Adding that would make the suite symmetric and catch any future drift in the matched-grade branch. Happy either way — fine to leave as-is.
4c94697 to
6ba5fcc
Compare
…parser fails When the judge's output does not match DEFAULT_GRADE_PATTERN (or a user-supplied grade_pattern), model_graded_qa / model_graded_fact previously returned value=INCORRECT in the else-branch of _model.py. "Couldn't extract a verdict" and "verdict is incorrect" then became indistinguishable in the score distribution, silently inflating the INCORRECT count in aggregate metrics when judges flubbed the grade prefix word (e.g. GRID:, GRAND, ANSWER:, **Answer: C**). This matches the convention already used by pattern() in _pattern.py: when its regex fails to match, it returns NOANSWER rather than INCORRECT. value_to_float() maps both NOANSWER and INCORRECT to 0.0, so accuracy aggregations are unchanged; the difference is that judge-parse failures are now distinguishable from real INCORRECTs. Closes UKGovernmentBEIS#4026. Co-authored-by: Yana Bodrasheva <bodraski@gmail.com> Co-authored-by: Elena Arseneva <ea.arseneva@gmail.com> Co-authored-by: Anton Gornakov <gornakovanton@gmail.com>
6ba5fcc to
7cf102a
Compare
avalyset
left a comment
There was a problem hiding this comment.
Thanks for the parametrized test cases and the symmetry assertion — that closes the observability gap cleanly. Good call flagging _math.py for a follow-up; the pattern is identical and antnewman's audit on #4026 backs that scope. This looks ready from my end.
|
We're currently addressing similar circumstances in
|
|
The
For a grader that fails to emit a parseable verdict, that difference is the whole question:
|
|
Thanks @MattFisher. |
|
We need to be a bit careful here - the pattern scorer returns In this case, the Grader model is effectively part of the scoring instrumentation, and a failure by the instrumentation should not be recorded as "No answer" from the MUT - the MUT may have returned a correct or incorrect answer or no answer at all, but the scoring instrumentation failed. 4091's proposed change to If the crucial use case here is the judge failure rate not being sufficiently obvious when |
Use Score.unscored() instead of NOANSWER on grade-regex miss: a judge parse failure is an instrumentation failure, not a model-under-test outcome, so it leaves the rate and is surfaced via unscored_samples. Tag unscored_reason=grade_parse_failure to keep it distinguishable.
|
Thanks @MattFisher, agreed. Pushed a variant Score.unscored() tagged with unscored_reason="grade_parse_failure", so it leaves the rate but stays visible. Heads up that this shifts the accuracy denominator vs today. |
…aded-noanswer # Conflicts: # tests/scorer/test_model_graded.py
Head branch was pushed to by a user without write access
This PR contains:
What is the current behavior? (You can also link to an open issue here)
Closes #4026.
When the judge's output does not match
DEFAULT_GRADE_PATTERN(or a user-suppliedgrade_pattern), theelsebranch inmodel_graded_qa/model_graded_fact(src/inspect_ai/scorer/_model.py) returnsScore(value=INCORRECT, ...). This conflates "the judge said the answer was incorrect" with "we couldn't extract a verdict from the judge", making the two indistinguishable in the score distribution. When small judge models emit variants likeGRID: C,ANSWER: Cor**Answer: C**instead ofGRADE: C, those samples are silently scoredINCORRECTeven when the judge clearly decided the answer was correct, so accuracy is understated with no signal that anything went wrong.What is the new behavior?
The parse-failure branch now returns
Score.unscored(...)instead ofScore(value=INCORRECT, ...), tagged withmetadata["unscored_reason"] = "grade_parse_failure".This follows the layering distinction @MattFisher raised in the issue and PR threads.
pattern()andmath()score the output of the Model Under Test, soNOANSWERis the right sentinel there.model_graded_*is different: the grader model is scoring instrumentation, not the thing under test. A grade-parse failure is the instrument failing, not the subject declining to answer, so recording it asNOANSWERwould still attribute a "no answer" to the subject that it may not have produced.Score.unscored()(NaN) is the framework's existing sentinel for "a scorer cannot produce a value for this sample": the sample is dropped from aggregate metrics and surfaced separately as an unscored sample, withunscored_reasonavailable for downstream filtering and for reporting the judge-parse-failure rate.This supersedes the original approach in this PR, which returned
NOANSWER. See the #4026 thread for the discussion that moved it tounscored().Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Yes, a metric-level one. Previously judge-parse failures were counted as
INCORRECT(mapped to0.0byvalue_to_float), so they stayed in the denominator. They are now unscored (NaN), excluded from aggregate metrics and reported separately as unscored samples. Evals that previously hit judge-parse failures will see those samples leave the denominator, so reported accuracy can move (typically up, since genuinely-correct-but-unparseable grades are no longer counted as wrong). Any code that inspects the rawScore.valuefor these samples needs to handle NaN rather than"I".Tests
tests/scorer/test_model_graded.py::test_grade_parse_failure_is_unscored— parametrised over four representative malformed grader outputs (GRID: C,ANSWER: C,**Answer: C**, and a completion with no grade marker); assertsScore.valueis NaN andmetadata["unscored_reason"] == "grade_parse_failure".tests/scorer/test_model_graded.py::test_matched_grade_resolves_to_value— symmetry check that a parseableGRADE: C/GRADE: I/GRADE: Pstill resolves toCORRECT/INCORRECT/PARTIAL, so the matched-grade branch is undisturbed.test_model_graded_answer_set_on_grade_parse_failure(from model_graded_fact: answer field empty when judge GRADE parsing fails despite correct completion #4025) updated to assert the parse-failure sample is unscored (NaN) whileScore.answerstill carries the subject completion.Other information:
math()in_math.pyhas a sibling parse-failure case, handled separately in fix(scorer): tag math() answer-extraction failures with reason metadata (#4026) #4091. That one staysNOANSWER, since it scores the Model Under Test's own output, per the same layering distinction.--score-on-error). That is orthogonal to this fix and could be a follow-up;unscored_reasongives it something to key on.Score.answerpopulated in this same branch) is tracked in model_graded_fact: answer field empty when judge GRADE parsing fails despite correct completion #4025 / fix(scorer): set Score.answer on model_graded grade-parse failure #4039.