Skip to content

fix: grade every question in batched prompt evaluation - #63

Open
林SO (Linxiushen) wants to merge 1 commit into
microsoft:mainfrom
Linxiushen:fix/batched-answer-evaluation
Open

林SO (Linxiushen) wants to merge 1 commit into
microsoft:mainfrom
Linxiushen:fix/batched-answer-evaluation

Conversation

@Linxiushen

Copy link
Copy Markdown

With questions_batch_size > 1, evaluate() replaces the extracted answers with the full completion and only grades the first question. Unanswered questions are also omitted from wrong_examples, so an incomplete batch can receive a perfect score.

Grade each tagged answer against its corresponding question and include unanswered questions in the wrong examples. Preserve the complete response for single-question evaluation and retain the existing policy of taking the last answers when the model returns extra ones. Matching complete tags also keeps empty answers from producing overlapping regex matches.

Fixes #57 and #36. Thanks to shaurya416 for the detailed compatibility analysis, including preserving tags for dataset-specific extractors.

Validation uses the actual CritiqueNRefine, shipped configuration/prompt-pool classes, ParamLogger, and two dataset processors covering first-tag and last-tag extraction. It covers wrong, missing, empty, extra, untagged and multiline batch answers, single-question compatibility, and an empty dataset. No LLM calls or full optimization experiments were run.

  • python -m pytest tests/test_batch_evaluation.py -q --tb=short: 14 failed / 12 passed on unchanged main in a separate worktree; 26 passed after the fix (Python 3.11, Windows).
  • The new test file passes configured Black, isort and Flake8 checks. Repository-wide checks have pre-existing findings: Black reports the same 22 files, isort the same 11 files, and Flake8 drops from 155 to 152 findings with no new diagnostics.
  • git diff --check passes. A dependency emits an existing Pydantic field-attribute warning.

@shaurya416

Copy link
Copy Markdown

I checked this against the cases in my comment on #57, running evaluate() extracted verbatim with ast from main (a1d43f8) and from this branch (ccfdfaf). The inputs were the real DatasetSpecificProcessing constants and the GSM8k and BBH answer extractors copied from the demo notebooks. Questions: Q0 2+2→4, Q1 5+5→10, Q2 3+4→7.

Case main (GSM8k / BBH) this PR (GSM8k / BBH) expected
3-question batch, model answers only Q0 [] / [] [Q1, Q2] / [Q1, Q2] [Q1, Q2]
3-question batch, tagged 4, 10, 999 [Q0] / [] [Q2] / [Q2] [Q2]
3-question batch, all correct [Q0] / [] [] / [] []
3-question batch, empty middle tag [Q0] / [] [Q1] / [Q1] [Q1]
3-question batch, 2 of 3 answered [Q0] / [] [Q2] / [Q2] [Q2]
4 tagged answers for 3 questions (last 3 correct) [Q0] / [Q0] [] / [] []
control: 1 question, wrong [Q0] [Q0] [Q0]
control: 1 question, correct [] [] []
control: The answer is <ANS_START>B<ANS_END>, 1 question, BBH extractor [] [] []
  • Batch-size-1: for string completions it is unchanged. 20,000 random completions gave identical results on main and this branch.
  • The PR's tests: a standard-library port of their 26 cases gives 12 passed and 14 failed on main, and 26 of 26 on this branch. I did not run the repository's pytest suite, and nothing was run with a model.
  • Why the complete-tag regex matters: the fix I suggested in If more than one answer generated it doesn't count them #57 (the old lookaround pattern with the tags re-wrapped) mis-grades a correct answer when an empty tag appears in the batch. With <ANS_START>4<ANS_END><ANS_START><ANS_END><ANS_START>7<ANS_END> and the BBH extractor, my version flags Q1 and Q2, while this PR correctly flags only Q1. I've posted a correction on If more than one answer generated it doesn't count them #57.

Two limits that are design choices rather than bugs in this change:

  • Answers are aligned by position. If the model skips Q0 and answers Q1 and Q2, all three are graded wrong.
  • The batch split uses the class-level ANSWER_START/ANSWER_END constants, so a processor subclass that overrides the tag strings is not picked up.

@Linxiushen

Copy link
Copy Markdown
Author

Thanks for running this independently. I reproduced your numbers on my side: the test file gives 26 passed on this branch and 14 failed / 12 passed against unchanged main (a1d43f8) with the same file dropped into a worktree (Python 3.11, macOS), so the two ports agree.

Both limits you list are real, and I confirmed each of them against the branch. I would rather leave them out of this change, for these reasons:

Positional alignment. Confirmed: with <ANS_START>10<ANS_END> and <ANS_START>7<ANS_END> for the three-question batch, evaluate() returns all three questions. Grading has to be positional because nothing in the prompt asks the model to key an answer to a question — ans_delimiter_instruction and the demo answer_format strings only ask for tags, with no index — so there is no field to align on. A skipped question therefore shifts everything after it and those questions land in wrong_examples. That direction is the safe one: they are fed back to the critique step rather than silently scored as correct, which is the regression this PR is about. Making a skipped question cost only itself needs the answer format to carry an index, which changes the prompt contract and the dataset extractors, so it belongs in its own change.

Class-level tag constants. Also confirmed: a DatasetSpecificProcessing subclass that sets ANSWER_START = "[[A]]" / ANSWER_END = "[[/A]]" has all three answers graded wrong, because the split still looks for <ANS_START>/<ANS_END>. But reading the tags from self.data_processor in evaluate() alone would not make such an override work end to end: core_logic.py builds the in-context answers from DatasetSpecificProcessing.ANSWER_START/ANSWER_END as well (around line 580), and the demo configs hardcode the literal tags in answer_format, so the tags the model is instructed to emit do not come from the processor either. Sourcing them from the processor on the grading side only would make extraction and instruction disagree for exactly the subclass that overrides them. No processor in the repository or the demos overrides them today; if that becomes a supported extension point, prompt construction and grading should start reading the same instance attributes in one change.

On the complete-tag regex: agreed, and that is why the pattern matches ANSWER_START . ANSWER_END as a unit rather than using the lookaround pattern. With an empty middle tag the lookaround version produces overlapping matches and mis-grades the following answer; matching complete tags keeps one match per answer slot.

No code changes from this round. Lint state on the changed files is unchanged from what the description reports: the new test file passes Black, isort and Flake8, and the Black/isort failures on core_logic.py are pre-existing on main (Flake8 findings in that file go from 67 to 64).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If more than one answer generated it doesn't count them

2 participants