Skip to content

naive_dapo.compute_score: tiemout=True typo makes the \pi branch always raise (swallowed by bare except:), so correct numeric answers to \pi questions score 0.0 #2289

Description

@shaurya416

skyrl-agent/skyrl_agent/tasks/verifiers/naive_dapo.py:510-519 (at de663e0, current main):

    if not correct:
        try:
            if "\\pi" in extracted_model_output or "\\pi" in ground_truth:
                equivs = []
                for pi in [math.pi, 3.14]:
                    equivs.append(math_equal(extracted_model_output, ground_truth, tiemout=True, pi=pi))
                    correct = any(equivs)
            else:
                correct = math_equal(extracted_model_output, ground_truth, timeout=True)
        except:
            correct = False

math_equal (prime_math/grader.py:175-182) takes timeout and has no **kwargs. Line 514 therefore raises TypeError: math_equal() got an unexpected keyword argument 'tiemout' on the first iteration of every call that reaches it, and the bare except: at line 518 turns that into correct = False.

As a result, any prediction or gold containing \pi that the first-stage grade_answer does not accept scores 0.0. The [math.pi, 3.14] loop and the pi parameter of math_equal never get to compare anything.

The typo dates from the original skyagent upload (3029bfb, #131), when the file was skyagent/skyagent/tasks/verifiers/naive_dapo.py and the call was at line 509. It was carried unchanged through the rename in f09c55b (#372) and through cae0124 (#713) to line 514 today.

Measured

Setup:

  • The target was not pip-installed. The real naive_dapo.py and prime_math/grader.py from the checkout were loaded through a namespace-package shim that skips skyrl_agent/__init__.py, because that file imports the agent runner.
  • The table was run twice. Once with pylatexenc replaced by a stub (both an identity stub and a \pi-aware stub were tried), and once with the real pylatexenc 2.11. Results were identical. With the real pylatexenc, grade_answer returns False for the four "intended use" rows, so those rows are decided by the stage that contains the typo.
  • compute_score was also extracted verbatim with ast.get_source_segment and exec'd in the same namespace. Results were identical.
  • The "typo fixed" column differs from the original in exactly one line, 514 (tiemout -> timeout).
case prediction gold as written tiemout -> timeout expected
intended use \boxed{6.28} 2\pi 0.0 1.0 1.0
intended use \boxed{9.42477796} 3\pi 0.0 1.0 1.0
intended use \boxed{12.566} 4\pi 0.0 1.0 1.0
intended use \boxed{2\pi} 6.2832 0.0 1.0 1.0
control (must fail) \boxed{7} 2\pi 0.0 0.0 0.0
control (must fail) \boxed{9.5} 3\pi 0.0 0.0 0.0
control (must pass, first stage) \boxed{2\pi} 2\pi 1.0 1.0 1.0
control (must pass, non-\pi branch, line 517) \boxed{50\%} 0.5 1.0 1.0 1.0

A spy on math_equal shows that every call into the \pi branch raises the TypeError above, and that the bare except: swallows it.

Direct calls:

  • As written: math_equal("6.28", r"2\pi", tiemout=True, pi=3.14) -> TypeError: math_equal() got an unexpected keyword argument 'tiemout'.
  • With the correct keyword: math_equal("6.28", r"2\pi", timeout=True, pi=3.14) -> True, and math_equal("9.42477796", r"3\pi", timeout=True, pi=math.pi) -> True.

Consequence

  • A correct answer scores 0.0 whenever the gold or the prediction contains \pi and the two match only by numeric approximation (for example 2\pi vs 6.28, or 3\pi vs 9.42477796). This is the case the [math.pi, 3.14] loop was written to handle, so that loop is effectively dead code.
  • The failure is silent. The bare except: swallows the TypeError without logging, so the only symptom is a lower reward on those rows.
  • The scope is limited to false negatives on \pi rows. For those rows the result is deterministic: every sample is scored wrong, not just some of them.
  • Reachability: the scorer is used by GeneralReactTask.evaluate_result when data_source.startswith("math") (general_react/utils.py:115-121). GeneralReactTask can be selected through the task: config and has a math-specific system prompt (utils.py:44). However, none of the shipped example configs or skyrl-agent data scripts currently produce a math* data_source. The path is hit only when GeneralReactTask is run on a user-supplied math dataset. No bundled recipe triggers it.

Suggested fix

Spell the keyword correctly. The sibling copy of this block already does so (prime_math/__init__.py:407: math_equal(extracted_model_output, ground_truth, timeout=True, pi=pi)), as does line 517 of the same function:

equivs.append(math_equal(extracted_model_output, ground_truth, timeout=True, pi=pi))

Optional follow-ups:

  • Narrow except: to except Exception:, as prime_math/__init__.py:411 does.
  • Log the exception so a future signature mismatch is visible.
  • Add a small regression test that pins the four "intended use" rows above.

The fix covers numeric approximations of k\pi-style golds. It does not cover every \pi form: 1.5708 vs \frac{\pi}{2} still scores 0.0 after the change.

Happy to open the PR.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions