feat(e2e_eval): unify example-tests into recipe-driven run_eval#1009
feat(e2e_eval): unify example-tests into recipe-driven run_eval#1009KayMKM wants to merge 5 commits into
Conversation
Unify the two e2e evaluation scripts (run_eval.py and the example-only run_example_tests.py) into a single recipe-driven runner that builds a model, runs winml perf, and — when perf passes — runs winml eval, writing one unified eval_result.json per (model, task, precision). - Add utils/recipes.py: data-driven discovery of examples/recipes/<slug> configs (fp16/w8a16/w8a8), single and composite (encoder/decoder, image/text-encoder). - run_eval.py expands the registry into one job per recipe precision variant; builds via 'winml build -c' (fallback to 'winml config' when a model has no recipe). Perf-gated accuracy reuses the existing both-mode gating. - Accuracy phase now only runs winml eval and records the full metrics+dataset; delta/verdict grading vs the PyTorch baseline is left to the report site. Inline baseline removed; offline --update-baseline mode maintains the baseline cache. - Remove runner-side report generation (generate_report.py, analyze_results.py); slim reporter.py/accuracy.py (keep compute_delta + thresholds for the site). Drop the in-pipeline generate_report step. - Tests for recipe discovery, job expansion, recipe build (single/composite/vitisai/failure), recipe-vs-fallback eval, and the new accuracy schema.
Recipe builds write export/optimized/model.onnx (+ .data/.bin) into each job's result dir via -o; across many models these fill the disk. With --clean-cache, remove a job's build artifacts once its eval_result.json is written, keeping only the JSON facts the report site reads, so peak disk stays at ~one job.
…an-cache onnxruntime/QNN leak temporary ONNX models into the subprocess working directory (the repo root): UUID-named <uuid>.onnx/.data, QNN EP-context dumps (<uuid>.onnx_<EP>.bin), and sym_shape_infer_temp.onnx from shape-inference preprocessing. These sit outside any path the harness controls via -o/--output, so neither the per-job artifact cleanup nor the HF/winml cache cleanup removed them, and they piled up across jobs until the disk filled — even with --clean-cache (reported on a QNN box where EP-context dumps are large). Add _clean_stray_cwd_artifacts(), matching only UUID-prefixed names or the fixed sym_shape_infer_temp so real files are never touched. Sweep the cwd at startup (clear a prior interrupted run's mess) and after every job under --clean-cache, alongside the existing job-artifact and cache cleanups.
…h cleanup
- --retry-failed only matched the coarse accuracy_status values (PASS/FAIL/SKIPPED/NOT_RUN), but the help text advertised the removed ACCURACY_REGRESSION verdict, so documented retries silently matched nothing. Update the help to list the actually-valid values (perf failure classifications + accuracy FAIL).
- Document that an empty winml-eval metrics map ({}) is intentionally treated as FAIL (not PASS), aligning the comment with the legacy baseline path's is-not-None check.
- Sweep the EP engine-compile timing dump (timing_log.csv) that some execution providers leak into the process cwd, alongside the existing UUID/sym-shape scratch cleanup under --clean-cache, so it cannot accumulate and fill the disk.
- Add tests for _should_skip_existing retry matching and the extended stray-cwd cleanup.
With --eval-type accuracy/both and --continue, a perf-only eval_result.json (accuracy=null) is no longer skipped: the model is rebuilt and winml eval is run, reusing the already-recorded perf instead of re-benchmarking, and the accuracy is merged back into the same result file. This lets a fast --eval-type perf sweep be topped up with accuracy later. - Add _needs_accuracy_backfill(): backfill when accuracy is missing and wanted (always for accuracy mode; only when perf passed for both mode; never for perf mode). _should_skip_existing() returns False for these so plain --continue tops them up. - Extract _build_for_job() shared by the normal and backfill paths. - Preserve the recorded perf verbatim; only replace the accuracy section and extend eval_types_run. - Update --continue help and README; add TestAccuracyBackfill.
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
There was a problem hiding this comment.
Well-structured refactor: recipe-driven eval with clean separation of concerns. All 97 tests pass. The runner now records facts only (metrics + dataset maps) and defers baseline grading to the report site. Recipe discovery is data-driven — drop a new precision config and it's picked up automatically. LGTM.
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
There was a problem hiding this comment.
Nice consolidation of the two eval scripts. The recipe-driven job expansion, perf-gated accuracy, and the --clean-cache disk-hygiene sweeps read cleanly and are well covered (103 new tests pass locally; ruff check is clean). I verified _read_eval_output matches EvalResult.to_dict()'s top-level metrics/dataset keys, and that the removed generate_report.py/analyze_results.py plus the trimmed reporter/accuracy symbols have no remaining references anywhere. A couple of minor, non-blocking notes inline.
| # skip run_model and only (re)build + run accuracy. A build that | ||
| # now fails records a skipped accuracy without touching perf. | ||
| if not build_result["success"]: | ||
| accuracy_result = {"skipped": True, "skip_reason": "build_failed"} |
There was a problem hiding this comment.
When a backfilled (re)build fails here, accuracy is recorded as {skipped: build_failed} while the reused perf stays passed, so the job still exits 0. And because _should_skip_existing skips a passed-perf result whose accuracy is skipped, a later --continue / --retry-failed won't re-attempt it — the only recovery is deleting eval_result.json. This mirrors the accuracy-only path at ~L2813, so it may be intentional, but for a resume-oriented feature a transient build failure being permanently masked (and not counted as a failure) is surprising. Consider leaving accuracy as None on build failure so the next --continue backfills it again.
| winml_key = ds_config.get("winml_metric_key") or ds_config.get("metric") | ||
| if winml_key and metrics and winml_key in metrics: | ||
| num_samples = (dataset or {}).get("samples", ds_config.get("num_samples")) | ||
| metric = {"metric": winml_key, "value": float(metrics[winml_key]), "num_samples": num_samples} |
There was a problem hiding this comment.
Minor: this branch isn't ruff format-clean. ruff format rewraps this line (106 chars) into a multi-line dict, and also touches the _build_jobs signature, the BACKFILL safe_print, and two spots in tests/unit/eval/test_run_eval_script.py. CI's lint job only runs ruff check (which passes), so it won't block — but the repo's pre-commit includes ruff-format, so worth running uvx ruff format scripts/e2e_eval/run_eval.py tests/unit/eval/test_run_eval_script.py before merge.
zhenchaoni
left a comment
zhenchaoni
left a comment
There was a problem hiding this comment.
1. Build artifacts go to the wrong place.
Recipe builds run winml build -c … -o eval_results/<date>/models/<slug>. Since -o and --use-cache are mutually exclusive, artifacts never reach ~/.cache/winml/artifacts. The fallback path still uses --use-cache, so within one run, recipe jobs and fallback jobs store artifacts in two different locations. This contradicts your goal of keeping everything in the winml cache.
2. Build failure silently mislabels precision.
When a recipe build fails, onnx_paths is empty, so perf/eval fall back to -m <hf_id> and rebuild at the default precision — then record the result under the recipe's precision slug (e.g. a failed fp16 build gets tested and stored as fp16 but actually ran w8a16). A failed recipe build should be a hard failure, not a silent fallback.
3 participants
Summary
Unifies the two e2e evaluation scripts (
run_eval.pyand the example-onlyrun_example_tests.py) into a single recipe-driven runner: build a model, runwinml perf, and — when perf passes — runwinml eval, writing one unifiedeval_result.jsonper (model, task, precision). Also adds two--clean-cachedisk-hygiene fixes so long matrix runs don't fill the disk.
Recipe-driven runner
utils/recipes.py: data-driven discovery ofexamples/recipes/<slug>configs (fp16/w8a16/w8a8), single and composite (encoder/decoder,
image/text-encoder).
run_eval.pyexpands the registry into one job per recipe precision variant;builds via
winml build -c, falling back towinml configwhen a model hasno recipe. Perf-gated accuracy reuses the existing both-mode gating.
winml evaland records the full metrics +dataset; delta/verdict grading vs the PyTorch baseline is left to the report
site. Inline baseline removed; offline
--update-baselinemaintains thebaseline cache.
generate_report.py,analyze_results.py); slimreporter.py/accuracy.py(keepcompute_deltagenerate_reportstep.Disk hygiene under
--clean-cacheexport/optimized/model.onnx(+
.data/.bin) once itseval_result.jsonis written, keeping only theJSON the report site reads — peak disk stays at ~one job.
working directory (UUID-named
<uuid>.onnx/.data, QNN EP-context dumps<uuid>.onnx_<EP>.bin, andsym_shape_infer_temp.onnx). Matches onlyUUID-prefixed or the fixed temp name so real files are never touched; runs at
startup and after each job.
Notes
main; the prerequisite build-only EP-matrix work landedseparately in feat(e2e_eval): add --build-only mode with per-EP matrix, export dedup, and Azure Artifacts upload #845, so this branch carries only the 3 commits for this change.