feat(cli): variance-aware scoring for noisy benchmarks (closes #4) - #108
Srinivasan8888 wants to merge 2 commits into
Conversation
Noisy benchmarks (LLM-judged, sampled, network-dependent) were scored from a single run, so one lucky run could set the new best and bias every later comparison. Add opt-in repeat-and-aggregate scoring. - core.aggregate_trial_scores(scores, method, metric): pure median/mean/worst aggregator (worst is direction-aware). Median default resists lucky outliers. - cli.run_extra_benchmark_trials(): re-runs only the scored benchmark N times (fresh runs only; crash-recovery/attach yields one score and is not re-executed), clearing result.json between trials so each writer claim is clean. Strict: a non-zero exit or timeout still fails the attempt. - Wire into the scored run path: the aggregate becomes the top-level score, so compare_scores/best_committed_score/dashboard/frontier are unchanged; per-trial scores + aggregate stats are stored in the benchmark record. - Config: `evo config set bench-repeat N` (default 1 = today's single-run behavior, zero overhead) and `score-aggregation median|mean|worst`; shown in `evo config show`, readable via `evo config get`. Tested: unit tests for the aggregator and the trials loop (fake executor), plus config set/get validation. Locally verified end-to-end against a real LocalExecutor with a noisy benchmark and via the real CLI.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| trial_scores = [score] + extra_scores | ||
| score, agg_stats = aggregate_trial_scores( | ||
| trial_scores, aggregation, metric | ||
| ) | ||
| benchmark_record["trials"] = [ | ||
| {"score": s, "returncode": 0} for s in trial_scores | ||
| ] | ||
| benchmark_record["aggregate"] = {**agg_stats, "value": score} |
There was a problem hiding this comment.
🟡 Aggregated score lost on remote resume
In remote mode with bench_repeat > 1, each extra trial overwrites result.json, so it ends holding the last trial's raw score, not the aggregate. If the attempt crashes after aggregation and resumes, benchmark_completed is true and load_result reads that raw score while re-aggregation is skipped, so the node is scored by one noisy trial. See the resume read at load_result call and the benchmark_completed gate at cli.py:3281-3285.
Prompt for agents
In cmd_run's variance-aware scoring branch (plugins/evo/src/evo/cli.py around lines 3435-3477), after computing the aggregate score the on-disk result.json still contains the last trial's raw score. On a remote resume of an attempt that crashed after this point, benchmark_completed becomes true (computed at ~cli.py:3281-3285) and load_result(result_path, ...) at ~cli.py:3431 re-reads result.json, yielding the last noisy trial's score rather than the aggregate, and fresh_primary_run is false so no re-aggregation happens. Consider persisting the aggregate back to result_path after aggregation (e.g. write {**parsed, 'score': aggregate} atomically) so recovery reads the aggregate, or otherwise make the recovery path aware of the stored aggregate score so the variance-aware result survives a resume.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed and fixed in 38ba9ff. Traced it: on a remote resume the benchmark_completed gate re-reads result.json via load_result without re-aggregating, and each extra trial's _fetch_remote_artifacts had overwritten result.json with the last raw trial. Added persist_aggregate_result(result_path, parsed, aggregate) which writes {**parsed, "score": aggregate} atomically right after aggregation, so recovery (and every downstream result.json reader) sees the aggregate, consistent with the committed node score. Covered by a new test and re-verified end-to-end (result.json held 0.48 → now 0.5 median).
| if bench is not None and (bench.exit_code or 0) != 0: | ||
| raise RuntimeError(f"benchmark_exit_{bench.exit_code}") |
There was a problem hiding this comment.
🔍 Trial loop does not distinguish remote infra failure
The primary run inspects _remote_infra_error_for_log and raises remote_infra_failure on a non-zero remote exit (cli.py:3403-3405), but the trial loop always raises benchmark_exit_{code}. A remote infra failure during an extra trial is then classified as a benchmark failure, altering telemetry failure_type. The attempt fails either way; relevant to the untested remote path.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 38ba9ff. The trial loop now mirrors the primary run: on a non-zero remote exit it fetches artifacts and checks _remote_infra_error_for_log, raising remote_infra_failure:<err> when the journal marks failed_infra, otherwise benchmark_exit_<code> — so telemetry failure_type stays accurate. Added tests for both branches (infra journal → remote_infra_failure; plain non-zero → benchmark_exit).
… failures
Address two review findings on the extra-trial path:
- Persist the aggregate back to result.json after aggregating. Each remote
extra trial overwrites result.json with its own raw score, so a crash-recovery
resume (benchmark_completed gate) re-read the last noisy trial instead of the
aggregate. persist_aggregate_result() writes {**parsed, score: aggregate}
atomically so recovery and every result.json reader match the committed score.
- Mirror the primary run's remote infra-failure classification in the trial
loop: a non-zero remote exit now checks _remote_infra_error_for_log and raises
remote_infra_failure instead of always benchmark_exit, keeping telemetry
failure_type accurate.
Tests: persist overwrites last-trial score with aggregate (fields preserved);
trial loop raises remote_infra_failure on an infra journal, benchmark_exit
otherwise. Re-verified end-to-end that result.json holds the aggregate.
1 participant
What & why
Closes #4. Noisy benchmarks (LLM-judged, sampled, network-dependent) are scored from a single run today, so one lucky run can land as the new best score and bias every comparison after it. This adds opt-in repeat-and-aggregate scoring; deterministic benchmarks are untouched and pay nothing.
The direction (opt-in config vs. auto-detection) follows the thread: @alokwhitewolf noted auto noise-detection is coming later at discover time, so this ships the config primitive that such detection would simply set. The
score-aggregationname matches @anirudh5harma's suggestion.Design
Only the benchmark evaluation repeats — the agent attempt runs once (the code under test is fixed; the measurement is what's noisy), and the benchmark is the last phase, so repeating just it is cheap.
core.aggregate_trial_scores(scores, method, metric)— pure aggregator returning(aggregate, stats). Methods:median(default, resists a single lucky/unlucky run),mean,worst(direction-aware: min formetric=max, max formetric=min).cli.run_extra_benchmark_trials(...)— re-runs the scored benchmarkbench_repeat - 1extra times for fresh runs only (a crash-recovery/attach yields one score and is not re-executed), clearingresult.jsonbetween trials so each writer'sO_EXCLclaim starts clean. Strict: a non-zero exit or timeout still fails the attempt, so a flaky benchmark never masks a real crash.score, socompare_scores/best_committed_score/ dashboard / frontier strategies are unchanged — they just consume a sturdier number. Per-trial scores + aggregate stats are stored in the benchmark record (only whenbench_repeat > 1).Config surface
Behavior compatibility
bench_repeat = 1(the default) takes the existing single-run path byte-for-byte — no record-shape change, no extra work for deterministic benchmarks.Testing
aggregate_trial_scores(median/mean/worst, direction-awareness, n==1, stdev, validation) and for the trials loop (fake executor: N runs, result cleared between trials, strict on exit/timeout).LocalExecutorwith a noisy benchmark: trials[0.5, 0.99, 0.48]→ median 0.5 (lucky 0.99 spike neutralized) vs mean 0.657. Full unit suite passes except pre-existing Rust hook-binary failures unrelated to this change (reproduce onmain).Note for reviewers
The remote-sandbox multi-trial path mirrors the existing
_fetch_remote_artifactspattern but I could only exercise the local executor locally; a check on a remote backend would be appreciated.Out of scope
Automatic noise detection (planned at discover time) and single-concurrent-benchmark locking (#4 thread, separate concern).