speedtest.py: warn on logs it can't time, list the slowest samples

#170
by powercoding1 - opened

šŸ¤– Opened by an AI agent on behalf of @powercoding1 , per the agent-comment convention in #40.

Follow-up to #119, from @gowrishkumar 's review there (points 1–4).

  • Logs that can't be timed now say so. A log with no stored samples (--no-log-samples, or a run stopped before any sample finished) used to print a table of zeros and a 0.0 s wall clock. It now gets a warnings entry, also printed to stderr: "log has no samples… nothing to time". A run whose status isn't success gets a warning too, and a missing start or end time gives wall_clock_s: null instead of a crash.
  • retries vs failed_calls is documented (point 1). retries is whatever the provider reports in ModelEvent.retries. It was set in my Ollama runs (44), but not with gowrishkumar's provider. A call Inspect retried after an error always counts in failed_calls, so nothing was being lost; the docstring and README now say which is which.
  • slowest_samples (point 4): the five samples with the longest total time. On the 8192-token log from #119, it shows cb-0001 at 860 s, which was the sample caught by the Ollama restart, so it's useful for spotting outliers.
  • The #141 retry limits (point 3): the recommended configuration notes that the task's max_retries 10 / timeout 3600 still apply.

Tests: two new tests run a real two-sample mockllm eval, with and without log_samples. One checks that the empty log warns instead of reporting zeros; the other checks there's no warning and that the slowest list shows both samples. The warning test fails with the check removed (mutation-tested). All inspect_eval tests pass, including the existing 6 in test_speedtest.py.

No solver or scorer code changes.

Stationhouse org

šŸ¤– Task triage: the closest task to this PR is Make the solver more efficient (optimize-solver), confidence 0.95. The PR modifies speedtest.py within the inspect_eval directory to add timing warnings and list slowest samples. The task description explicitly mentions 'adds a reproducible speed test for the Inspect solver' and reports timings. This is a direct implementation of that task.

This is recorded in the platform's backend, not in the title.

Recorded that powercoding1 is working on Make the solver more efficient.

This comment was added automatically by a local model (qwen3.5:4b). If it got this wrong, reply here and a maintainer will fix the record.

Hi šŸ˜€ noticed something with the following change: if no per-sample detailed are missing from the log (could happen when running the eval with --no-log-samples for example), the output of speedtest.py is JSON object with all zeros and a warning For example:

{
  "warnings": ["log has no samples ..."],
  "samples": 0,
  "errors": 0,
  "run_out_tok_per_s": 0.0,
  "model_under_test": {
    "calls": 0,
    "failed_calls": 0,
    "tokens_out": 0
  }
}

This might be problematic for tools consuming the JSON numeric fields alone. What do you think of making these fields null in these scenarios?

{
  "warnings": ["log has no samples ..."],
  "samples": 0,
  "errors": null,
  "run_out_tok_per_s": null,
  "model_under_test": null,
  "by_group": null,
  "judge": null,
  "slowest_samples": null
}

Went through this and ran it on a few real logs. It covers the points I raised in #119: retries versus failed_calls is clear now, the warnings work (a run that stopped keeps its numbers and gets a warning), and the slowest samples list is useful.

I agree with @tomerz14 that the zeros are a problem. I took a --no-log-samples log and gave it a realistic 800 s wall clock, and got run_out_tok_per_s: 0.0, errors: 0 and calls: 0. A script reading only the numbers wouldn’t see the warning, and errors: 0 reads as ā€œno errorsā€.

One addition: the log still has Inspect’s own totals in log.results (planned and completed samples), so samples and errors don’t have to be null, they can be exact. report.py already uses that fallback in its coverage block. null fits what can’t be computed without samples: model_under_test, by_group, judge, run_out_tok_per_s and slowest_samples. wall_clock_s can stay, since the start and end times are in the log.

That’s your call, @powercoding1 . Either way, I’d say approve once the empty case isn’t zeros.

šŸ¤– Posted by an AI agent on behalf of @powercoding1 , per the agent-comment convention in #40.

Thanks @tomerz14 and @gowrishkumar , good catch. Pushed a fix (e3d4cb9):

  • Without stored samples, samples and errors now come from Inspect's own totals, reusing report.py's coverage() as suggested, so they're exact rather than 0.
  • Everything that needs the samples (run_out_tok_per_s, model_under_test, by_group, judge, slowest_samples) is null instead of 0. wall_clock_s stays, since the start and end times are in the log.
  • The test now checks both the exact counts and the nulls, and fails without the change. A normal log's output is unchanged (checked on the 8192-token run from #119).
stationhouse changed pull request status to merged

Sign up or log in to comment