Repository navigation
feat(metric-benchmark): orchestrate configured experiments through stage libraries - #99
mstaats-nvidia wants to merge 6 commits into
Conversation
3e78d83 to
4774061
Compare
4774061 to
748d3ec
Compare
748d3ec to
3fad1da
Compare
mstaats-nvidia
changed the title
3fad1da to
a45faa7
Compare
Defines an experimental benchmark for evaluating how well our coverage metric predicts evaluation effectiveness through mutation testing. The concise [root README](https://github.com/NVIDIA-NeMo/eval-author/blob/docs/metric-benchmark-design/labs/metric-benchmark/README.md) describes the project, its work-in-progress status, and the expectation that case examples will grow, change, or be dropped as we build in plain sight. Prep is external to this repository; runs consume prepared case examples, fixed subset manifests, and a YAML benchmark definition. The [stage descriptions](https://github.com/NVIDIA-NeMo/eval-author/blob/docs/metric-benchmark-design/labs/metric-benchmark/docs/stages.md) focus on four executable stages: run mutants × eval tasks, compute mutant scores, compute coverage, and correlate the score pairs. The first execution script will be a basic local Harbor loop; NeMo Helix support is planned afterward. The [architecture and both SVG figures](https://github.com/NVIDIA-NeMo/eval-author/blob/docs/metric-benchmark-design/labs/metric-benchmark/docs/architecture.md) explain the inputs, component boundaries, shared results directory, and statistical comparison. This is the documentation-only base of the stack. Implementation PRs #94 → #105 → #106 → #95 → #96 → #97 → #99 inherit the overview and design unchanged. Their detailed CLI guide lives separately in `docs/tools.md`. Validation: the five files in this PR are Markdown/SVG only. Local links and anchors, Markdown fences, headers, SVG XML, and whitespace checks pass; the updated figure was rendered and visually inspected. Restacking preserves all source and test files byte-for-byte. No live evaluations or model calls were made.
a45faa7 to
b448502
Compare
b448502 to
db930c6
Compare
2787476 to
bddd9a4
Compare
50de2ad to
dd21892
Compare
892d518 to
a72df12
Compare
a72df12 to
fe7c0d7
Compare
c3d9748 to
d13d271
Compare
aleckhoury
left a comment
aleckhoury
left a comment
There was a problem hiding this comment.
The coordinator is thin and calls the stage libraries directly. Execution receives cases and execution settings, analysis receives cases and analysis settings, and evidence is frozen and rechecked before experiment.json is written. At the stack top: 178 passed, 1 skipped. The smoke example ran end to end with a fake Harbor runner and produced a constant-rank correlation, as expected for synthetic data.
I'd fix the fresh-run recovery issue before merge. The rest of the inline items are smaller.
| try: | ||
| write_json( | ||
| output / "experiment-inputs.json", | ||
| { | ||
| "analysis_sha256": analysis_sha256, | ||
| "benchmark": loaded.definition.model_dump(mode="json"), | ||
| "run_id": run_id, | ||
| "implementation": implementation(), | ||
| }, | ||
| ) | ||
| except ValueError as error: | ||
| raise ValueError( | ||
| f"{error}; benchmark, run ID or implementation differs. " | ||
| "Use a new --output directory and --results to reuse saved executions." | ||
| ) from error |
There was a problem hiding this comment.
Fresh-run recovery issue: experiment-inputs.json is written before execute_benchmark runs. It includes implementation(), which hashes every metric_benchmark/*.py file plus the Harbor and Pydantic versions.
Reproduced:
- A fresh run fails on the second agent (the runner raises).
- The code changes.
- Rerunning with the same
--outputis rejected here.
The suggested fallback, a new --output with --results, then fails with FileNotFoundError, because execution.json is written only after execution completes. The partial Harbor jobs under <output>/execution can then only be recovered with run_evals.py, which neither this error nor run-experiment.md mentions.
Suggest writing experiment-inputs.json after execute_benchmark returns in fresh mode. Execution already guards its own resume via execution-inputs.json.
There was a problem hiding this comment.
Agreed, good catch, could have sworn I got this.
| prohibit new coverage invocations. | ||
|
|
||
| Resume into an existing analysis directory requires the same experiment settings, | ||
| run ID, evidence, and analysis implementation. After changing analysis hypotheses |
There was a problem hiding this comment.
Docs: "analysis implementation" is inaccurate. implementation() hashes the whole metric_benchmark package (including execution.py and signing.py) plus the Harbor and Pydantic versions. Suggest "package implementation".
There was a problem hiding this comment.
Agreed, done
| manifest = ExecutionManifest.model_validate(read_json(resolve_evidence(results_root, "execution.json"))) | ||
| if manifest.run_id != run_id or manifest.cases_sha256 != analysis.inputs.identity: | ||
| raise ValueError("execution manifest does not match cases/run identity") | ||
| if manifest.cases.keys() != analysis.inputs.cases.keys(): | ||
| raise ValueError("results case inventory differs from configured cases") |
There was a problem hiding this comment.
Minor: in --results mode these identity checks run after experiment-inputs.json has already been written.
Reproduced: a mistyped run ID fails with "execution manifest does not match cases/run identity". Rerunning with the correct ID into the same --output then fails with "output already exists with different contents", so one typo makes that directory unusable.
Suggest reading execution.json and checking run_id, cases_sha256 and the case inventory before the write when results_root is given.
There was a problem hiding this comment.
Agreed, done.
| index = resolve_evidence(results_root, manifest.cases[case.case_id]) | ||
| if index.name != "results.json": | ||
| raise ValueError(f"case {case.case_id}: index must be named results.json") | ||
| if index.parent in directories: | ||
| raise ValueError("results directory reused by multiple cases") | ||
| directories.add(index.parent) | ||
| raw = RawResults.model_validate(read_json(index)) | ||
| if raw.run_id != manifest.run_id: | ||
| raise ValueError(f"case {case.case_id}: run_id differs from execution.json") |
There was a problem hiding this comment.
Maintainability: this execution.json → results.json resolution now exists in three places:
- here
validation.validate_experiment_results(validation.py:209-232)scoring.load_score_inputs(scoring.py:151-170), which coverage also uses
The copies have drifted. This copy and validation reject extra or unexpected manifest cases and reused result directories; load_score_inputs checks neither, and the error messages differ. As a result, standalone scoring and coverage accept a manifest that validation and the coordinator reject. One shared helper (e.g. load_case_results(root, cases, case_id)) called from all three places would remove the duplication and the drift.
There was a problem hiding this comment.
Ack. This is intentional, they are different things but I did update deeper in the stack to make this clearer.
| write_json(destination / "selection.json", selection_for_results(case, raw, analysis.settings.scoring)) | ||
| write_json(destination / "coverage-config.json", analysis.settings.coverage) | ||
| write_json(destination / "subsets.json", analysis.load_subsets(case.case_id)) | ||
| write_json(destination / "case-description.json", analysis.load_description(case.case_id)) | ||
| mutants = compute_mutant_scores( | ||
| raw, analysis.load_subsets(case.case_id), selection_for_results(case, raw, analysis.settings.scoring) | ||
| ) | ||
| prepared.append((prepared_case, raw, index.parent, destination, mutants)) | ||
| results = {} | ||
| for prepared_case, raw, native, destination, mutants in prepared: | ||
| case = prepared_case.case | ||
| subsets = analysis.load_subsets(case.case_id) | ||
| selection = selection_for_results(case, raw, analysis.settings.scoring) | ||
| write_json(destination / "mutant-scores.json", mutants) | ||
| coverage = invoke_coverage( | ||
| raw, | ||
| subsets, | ||
| selection, | ||
| analysis.settings.coverage, | ||
| analysis.load_description(case.case_id), |
There was a problem hiding this comment.
Per case, this recomputes the same inputs several times:
selection_for_results(...): 3 times (lines 95, 100, 107)load_subsets: 3 times (lines 97, 100, 106), each re-reading and re-verifying the pinned fileload_description: 2 times (lines 98, 114)
So the objects written under the "freeze" comment aren't the ones passed to compute_mutant_scores and invoke_coverage; they match only because of the end-of-run analysis.identity recheck. Suggest computing selection, subsets and description once in the first loop, carrying them in prepared, and reusing them below.
There was a problem hiding this comment.
Agreed, good catch.
f02f4b3 to
a9f5924
Compare
504c827 to
f81f82a
Compare
b5f13a6 to
8e6f313
Compare
…braries Signed-off-by: mstaats <mstaats@nvidia.com>
Signed-off-by: mstaats <mstaats@nvidia.com>
Signed-off-by: mstaats <mstaats@nvidia.com>
Signed-off-by: mstaats <mstaats@nvidia.com>
Signed-off-by: mstaats <mstaats@nvidia.com>
|
Sweet thanks!! |
Signed-off-by: mstaats <mstaats@nvidia.com>
The coordinator reads an experiment YAML referencing separate cases, execution, and analysis configurations, then calls the stage libraries directly. Preparation stays external. There is no REST service, database, or UI.
Fresh execution runs Harbor, validates native evidence, and then computes mutant scores, coverage, and correlation.
--resultsanalyzes saved evidence without requiring executable task sources or agent bundles; explicit native revalidation remains available throughvalidate_results.py. Evidence and analysis inputs are checked before publishingexperiment.json. Cached replay prohibits new coverage calls.The shared per-case reader centralizes safe index paths, schema parsing, and run-ID checks. Experiment-wide inventory checks remain with their callers. Analysis records are written after execution and manifest checks, allowing interrupted execution to resume after a package change and invalid saved-result arguments to be corrected without reserving the output. Per-case analysis inputs are loaded once and reused.
The command guide is
labs/metric-benchmark/docs/run-experiment.md. The bundled small Docker example uses synthetic coverage and is explicitly labeled as a wiring test.Real end-to-end validation at
7d794bfOn 2026-10-31, one fresh
run_experiment.pyinvocation completed in 497 seconds using prepared Rho baseline + three mutants (action budget, memory amnesia, observation degradation) against three Terminal Bench tasks (regex-log,log-summary-date-ranges,polyglot-c-py). This used real Docker/Harbor execution and model calls, not a fake runner or saved execution replay.This is a plumbing smoke test, not scientific evidence of metric validity or semantic coverage. It uses the prepared case's Python 3.12/dependency-cutoff runtime pins. Exact signed configuration and private evidence are retained outside Git under
pr99-e2e-20261031; no credentials or raw model traces are published. No production-code changes were needed during this run.Automated checks
Full local suite: 3,459 passed, 36 skipped (optional runtimes/integrations). Benchmark-only suite: 188 passed, 1 skipped (the opt-in unit-suite Docker smoke; the real 12-trial run above passed separately). Ruff, formatting, type checks for Python 3.12/3.13, copyright, license, and whitespace checks passed. These results include the recovery regressions added in the latest commit. No claim is made about fresh hosted CI.
#95, #96, #97, and #157 are merged; this final core-stack PR is based on
main.Saved-results validation boundary
Test-only follow-up at
8229e0c: a corrupted native task checksum is rejected by explicit native validation, while saved-results analysis completes without producing a native-validation report or altering that metadata. This pins the documented boundary; existing tests cover analysis without executable sources. No production code or output contracts changed.Pipeline and validation suites: 49 passed. Ruff/format, Python 3.12/3.13 type checks, copyright, and whitespace checks passed. The full-suite and live end-to-end results above were obtained at
7d794bf; production code remains byte-identical. Validation-status metadata and further validation refactors remain deferred.