Skip to content

[Fix] Fix OlympiadBench grading of factored answers - #2593

Open
mkzung wants to merge 1 commit into
open-compass:mainfrom
mkzung:fix/olympiadbench-interval-fallthrough
Open

[Fix] Fix OlympiadBench grading of factored answers#2593
mkzung wants to merge 1 commit into
open-compass:mainfrom
mkzung:fix/olympiadbench-interval-fallthrough

Conversation

@mkzung

@mkzung mkzung commented Aug 12, 2026

Copy link
Copy Markdown

Motivation

MathJudger.is_equal runs four equivalence tests in sequence: interval, numerical,
expression, equation. The last three swallow a failure and fall through to the next
one. The interval test returns False, which ends the comparison.

is_interval classifies a string by its first and last character only, so a factored
product such as (x-1)(x+1) is taken for an interval. interval_equal strips the
outer brackets, leaving the fragment x-1)(x+1, and comparing it raises inside
compare_two_interval: expression_equal returns a sympy Relational when the
difference is still symbolic, and if not ... cannot evaluate it. The exception then
ends the whole comparison, so the expression test that proves these two answers
equal is never reached.

With sympy 1.14.0, OlympiadBenchEvaluator.score reports 0.0 accuracy for three such
prediction/reference pairs:

(x-1)(x+1)  vs (x^2-1)      is_interval=(True, True) interval_equal raised TypeError judge=False
(a+b)(a-b)  vs (a^2-b^2)    is_interval=(True, True) interval_equal raised TypeError judge=False
(x+2)(x+3)  vs (x^2+5x+6)   is_interval=(True, True) interval_equal raised TypeError judge=False
OlympiadBenchEvaluator accuracy: 0.0

Tightening is_interval so that a factored product is not classified as an interval
would keep these answers off the interval path in the first place. This PR does the
smaller thing: a failed interval test no longer ends the comparison.

Modification

  • The interval handler in is_equal falls through to the remaining tests, as the
    numerical, expression and equation handlers already do.
  • tests/datasets/test_olympiadbench.py covers a factored product against its
    expansion and against a wrong expansion, and pins interval comparison for equal,
    unequal and mixed-bracket bounds.

BC-breaking (Optional)

No. An answer that graded correct still grades correct; the change can only add a
later test, never remove one.

A sweep of 26 bracket-delimited pairs through MathJudger.judge moves three verdicts,
all of them false negatives:

'(x-1)(x+1)'  '(x^2-1)'      False -> True
'(a+b)(a-b)'  '(a^2-b^2)'    False -> True
'(x+2)(x+3)'  '(x^2+5x+6)'   False -> True

The other 23 are unchanged, including (1,2) vs (1,3), [0,1] vs (0,1),
(-\infty,0) vs (-\infty,0] and (x-1)(x+1) vs (x^2+1), which stay False, and
(0,\infty) vs (0,+\infty) and (\frac{1}{2},1) vs (0.5,1), which stay True.

Use cases (Optional)

Not applicable.

Tests

  • pytest tests/datasets/test_olympiadbench.py -q — 2 passed. On main,
    test_factored_product_equals_expanded_form fails with
    AssertionError: False is not true and the interval test already passes.
  • pytest tests/datasets tests/openicl tests/utils tests/evaluator --ignore=tests/datasets/test_local_datasets.py -q — 3 failed, 162 passed, 2 skipped.
    The same three test_livecodebench_evaluator cases fail unchanged on main
    (macOS multiprocessing spawn, unrelated to this PR).
  • pre-commit run --files opencompass/datasets/OlympiadBench.py tests/datasets/test_olympiadbench.py — flake8, isort, yapf and codespell pass.

Checklist

Before PR:

  • Pre-commit or other linting tools are used to fix the potential lint issues.
  • Bug fixes are fully covered by unit tests, the case that causes the bug should be added in the unit tests.
  • The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, like docstring or example tutorials.

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with those projects.
  • CLA has been signed and all committers have signed the CLA in this PR.

is_interval only inspects the first and last character, so a factored
product such as (x-1)(x+1) is routed to interval_equal. Stripping the
outer brackets leaves the fragment x-1)(x+1, comparing it raises, and
the handler abandoned the comparison there. The numerical, expression
and equation checks that follow do prove these answers equal, so the
handler falls through to them like the other three.
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.

1 participant