Add two-ONNX comparison to winml eval --mode compare by xieofxie · Pull Request #1139 · microsoft/winml-cli

xieofxie · 2026-07-20T03:31:35Z

Summary

Extends winml eval --mode compare so the reference can be a second ONNX file instead of only the HuggingFace PyTorch model derived from --model-id.

A new --reference <path> option points at a reference .onnx. When set, both the candidate (-m) and the reference run as raw ORT sessions (WinMLSession) on identical inputs — random by default, or the tensors from --input-data — and the existing tensor-similarity metrics (cosine, SQNR, PSNR, MSE, max-abs-diff) are computed over their aligned outputs. Because both are raw ONNX graphs, no HF config/task is loaded, so --model-id and --task are not required in this path. Both sessions honor --device / --ep.

Usage

winml eval -m candidate.onnx --mode compare --reference reference.onnx

Changes

eval/config.py: add reference_path field (cli_name="reference") with to_dict / from_dict support.
commands/eval.py: add --reference option; _resolve_reference() normalizes hub refs and validates .onnx existence; allow_missing_model_id lets a bare ONNX candidate skip the --model-id requirement; guard so --reference only works with --mode compare; print the reference path.
eval/evaluate.py: skip task resolution and load no torch model for the two-ONNX path; guard None model_id/task and print the reference in the config summary.
eval/tensor_similarity_evaluator.py: add _ONNXSessionModel wrapper (exposes onnx_path, io_config, __call__ returning torch tensors); build both raw sessions when reference_path is set, reusing the existing similarity metrics.
docs/commands/eval.md: document --reference, clarify --mode, add a two-ONNX example.
Tests: cover config round-trip, _resolve_reference validation, the compare-mode guard, bare-ONNX candidate handling, the two-ONNX evaluate flow, and the _ONNXSessionModel wrapper.

Scope

Phase 1 supports an ONNX-file reference only. HF-local-folder and composite-model references are deferred. The branch is rebuilt on current main, so the diff contains only the --reference feature (the earlier --input-data work landed separately in #1221).

## Summary Adds `--input-data` to `winml eval --mode compare`, letting you compare an ONNX candidate against its HuggingFace reference on **real input tensors** from a `.npz` archive instead of randomly generated ones — mirroring `winml perf`'s `--input-data`. The archive keys must match the candidate model's input names and are validated/dtype-cast against them. The **leading axis of each array is the sample axis**, so an archive shaped `(N, ...)` runs `N` samples and the similarity table reflects a real distribution (mean/std/min/max); all inputs must share the same `N`. The flag is only valid with `--mode compare`. ```bash winml eval --mode compare -m cand.onnx --model-id microsoft/resnet-50 --input-data inputs.npz ``` ## Changes - **`datasets/input_data.py`** (new): shared `load_input_data` (extracted from `perf`) plus a multi-sample `InputDataDataset` wrapper (leading axis = sample axis, each sample sliced to a batch of 1). - **`commands/perf.py`**: `load_input_data` now delegates to the shared module via a thin lazy import (same public symbol, unchanged behavior). - **`eval/config.py`**: new `input_data` field with `to_dict`/`from_dict` support. - **`commands/eval.py`**: `--input-data` option, compare-mode guard (`UsageError` when `mode != compare`, checked before model resolution), docstring example, and the report header now shows `Input data` / the real sample count. - **`eval/tensor_similarity_evaluator.py`**: `prepare_data` builds `InputDataDataset` when `input_data` is set and propagates `len(dataset)` into the effective config so the report/JSON reflect the real `N`. - **`eval/evaluate.py`**: `print_config` prints the input-data path in the config summary. - **docs**: `docs/commands/eval.md` flag row + example. - **tests**: new `tests/unit/datasets/test_input_data.py`; input-data cases added to the eval command/config/evaluator test suites; `test_perf_cli.py` logger target retargeted to the shared module. ## Notes This cherry-picks **only** the `--input-data` parts of #1139, decoupled from that PR's two-ONNX `--reference` feature (not included here). The guard runs before model resolution so it fires cleanly even without `--model-id`. --------- Co-authored-by: Hualiang Xie <hualxie@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Introduce a --reference option so `winml eval --mode compare` can diff two ONNX files directly instead of only comparing an ONNX candidate against its HF PyTorch reference. Both models run as raw ORT sessions on identical inputs (random by default, or --input-data tensors), so --model-id and --task are not required and no torch/transformers reference is loaded. - config: add reference_path field (to_dict/from_dict) - cli: --reference option, _resolve_reference validation/normalization, allow a bare ONNX candidate without --model-id, require --mode compare - evaluate: skip task resolution and return no model for the two-ONNX path; print the reference in the config summary and eval report - evaluator: _ONNXSessionModel wrapper; build both raw sessions when reference_path is set, reusing the existing similarity metrics - docs + unit tests Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

DingmaomaoBJTU

Review Comments

⚠️ Unrelated e2e test removals (~200 lines)

This commit removes TestProcessExitCleanup, TestInspectWarningNoise, and test_benchmark_qnn_process_exit_releases_native_session plus subprocess helpers from 3 e2e files. These are unrelated to the --reference feature — were they moved elsewhere or intentionally dropped? Should be a separate commit if intentional.

⚠️ Potential bug: torch→numpy in `_ONNXSessionModel.call`

_inference_model passes torch tensors via model(**inputs). _ONNXSessionModel.__call__ forwards them directly to self._session.run(inputs). Does WinMLSession.run() handle torch tensor inputs, or does it expect numpy arrays? If the latter, a .numpy() conversion is needed inside __call__ before calling self._session.run().

Minor: No `--input-data` + `--reference` combined test

The code supports combining --input-data with --reference (real tensors fed to two-ONNX compare), but no unit or e2e test covers this interaction path.

👍 Overall

Feature is well-designed — clean _ONNXSessionModel wrapper, proper validation in _resolve_reference, good docs/tests coverage. The concerns above are the main items.

Add a dedicated fake ORT session (exposing input_types) plus two tests that exercise the previously-uncovered interaction of --input-data with --reference in --mode compare: - prepare_data builds an InputDataDataset over the candidate _ONNXSessionModel's io_config (not a RandomDataset), proving the two options compose. - compute() runs the two real samples end-to-end through both raw ORT sessions (torch tensors in, numpy out), confirming _ONNXSessionModel forwarding torch tensors to WinMLSession.run is safe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

xieofxie · 2026-07-29T02:43:21Z

Thanks for the thorough review @DingmaomaoBJTU. Went through all three:

① "Unrelated e2e test removals" — not an actual removal; the branch was just behind main.

The branch was rebuilt on main at deadd822 (#1221), and main moved 2 commits ahead since (#1239, #1240), which is where TestProcessExitCleanup, TestInspectWarningNoise, test_benchmark_qnn_process_exit_releases_native_session, ep_path.py, test_ensure_provider_ready.py, etc. landed. A two-dot git diff main HEAD on an out-of-date branch renders those additions-on-main as deletions-on-branch, but nothing was actually removed here. I've merged current main into the branch, so those files/tests are all present and the diff now shows only the 8 --reference files.

② torch → numpy in _ONNXSessionModel.__call__ — not a bug.

WinMLSession.run() already accepts torch tensors; it converts them to numpy in _prepare_inputs before running:

# session.py
if hasattr(value, "numpy"):  # torch.Tensor
    arr = value.cpu().numpy()

Its docstring even states inputs may be "torch.Tensor or numpy arrays". So forwarding the tensors directly is correct and no .numpy() is needed in __call__. The new test_compute_runs_real_inputs_through_both_sessions exercises this end-to-end (torch tensors in, numpy out) as a guard.

③ No --input-data + --reference combined test — fixed.

Added TestONNXReferenceWithInputData with two cases:

test_prepare_data_uses_input_data_over_candidate_session — confirms prepare_data builds an InputDataDataset over the candidate _ONNXSessionModel's io_config (not a RandomDataset) when both options are set, proving they compose.
test_compute_runs_real_inputs_through_both_sessions — runs the compare loop end-to-end on real .npz samples through both raw ORT sessions.

All eval/command unit tests pass and ruff is clean.

DingmaomaoBJTU

Overall the --reference feature is well-designed. A few inline comments below.

Address PR review follow-ups: - Add an inline comment in _ONNXSessionModel.__call__ noting that inputs may be torch tensors and WinMLSession.run/_prepare_inputs handles the torch->numpy conversion, so no explicit .numpy() is needed. - Render a composite {role: path} model_path readably in the eval report header (joined paths) and detail lines (one "ONNX (role): path" per sub-model) instead of leaking a raw Python dict repr. - Cover both with TestDisplayEvalReportHeader. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

DingmaomaoBJTU

Second-pass review with a few additional findings.

The overlap/mismatch diagnostics in TensorSimilarityEvaluator.compute() hardcoded "HF reference", which is misleading in the two-ONNX --reference path where both sides are raw ONNX. Compute a reference_label based on config.reference_path and use candidate/reference wording in both the no-overlap ValueError and the mismatch warning. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

DingmaomaoBJTU

Third-pass review — backward compatibility, error handling, and flag interactions deep-dive.

Verified OK (no issues):

Backward compat: from_dict() uses .get("reference_path") → old config files without the field load fine (defaults to None)
to_dict() only emits reference_path when non-None → minimal serialization, no noise for existing configs
Error recovery: WinMLSession failures in _ONNXSessionModel.__init__ bubble up cleanly through evaluate()'s exception handler
--skip-build interaction: two-ONNX path bypasses entire build pipeline, no conflict
--ep/--device: both models use same EP/device by design (fair comparison isolating quantization/optimization diffs)
_resolve_reference mutation: safe — config is replace()-copied before evaluate()
allow_missing_model_id with non-ONNX: gated by _resolve_reference's .onnx suffix check, so HF model IDs are blocked upstream
Click option ordering: decorators are independent, no ordering dependency

DingmaomaoBJTU

LGTM — approving after three review passes.

Non-blocking follow-ups (can be addressed in a later PR):

compute() error/warning messages still say "ONNX and HF reference" in two-ONNX mode — cosmetic, doesn't affect functionality
Consider adding a test for display_eval_report when model_id is None (two-ONNX path)

All other areas verified: backward compat, config serialization, error recovery, flag interactions, torch/numpy handling — all clean.

DingmaomaoBJTU

LGTM

xieofxie requested a review from a team as a code owner July 20, 2026 03:31

xieofxie marked this pull request as draft July 20, 2026 06:40

xieofxie mentioned this pull request Jul 27, 2026

feat: Add --input-data to winml eval --mode compare #1221

Merged

xieofxie force-pushed the hualxie/compare_onnx branch from fe807aa to de504ec Compare July 28, 2026 08:11

xieofxie marked this pull request as ready for review July 28, 2026 08:54

xieofxie force-pushed the hualxie/compare_onnx branch from de504ec to abe903d Compare July 28, 2026 09:04

DingmaomaoBJTU reviewed Jul 29, 2026

View reviewed changes

Hualiang Xie and others added 2 commits July 29, 2026 10:33

Merge remote-tracking branch 'origin/main' into hualxie/compare_onnx

17a4aa6

DingmaomaoBJTU reviewed Jul 29, 2026

View reviewed changes

Comment thread tests/e2e/test_inspect_e2e.py

Comment thread src/winml/modelkit/eval/tensor_similarity_evaluator.py

Comment thread src/winml/modelkit/eval/tensor_similarity_evaluator.py

Comment thread src/winml/modelkit/commands/eval.py

DingmaomaoBJTU reviewed Jul 29, 2026

View reviewed changes

Comment thread src/winml/modelkit/eval/tensor_similarity_evaluator.py

Comment thread src/winml/modelkit/eval/tensor_similarity_evaluator.py

Comment thread tests/unit/commands/test_eval.py

DingmaomaoBJTU reviewed Jul 29, 2026

View reviewed changes

DingmaomaoBJTU approved these changes Jul 29, 2026

View reviewed changes

Merge branch 'main' into hualxie/compare_onnx

dc42601

DingmaomaoBJTU approved these changes Jul 29, 2026

View reviewed changes

xieofxie enabled auto-merge (squash) July 29, 2026 07:22

xieofxie merged commit 7eedb8f into main Jul 29, 2026
9 checks passed

xieofxie deleted the hualxie/compare_onnx branch July 29, 2026 07:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add two-ONNX comparison to winml eval --mode compare - #1139

Add two-ONNX comparison to winml eval --mode compare#1139
xieofxie merged 6 commits into
mainfrom
hualxie/compare_onnx

xieofxie commented Jul 20, 2026 •

edited

Loading

Uh oh!

DingmaomaoBJTU left a comment

Uh oh!

xieofxie commented Jul 29, 2026

Uh oh!

DingmaomaoBJTU left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DingmaomaoBJTU left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DingmaomaoBJTU left a comment

Uh oh!

DingmaomaoBJTU left a comment

Uh oh!

DingmaomaoBJTU left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

xieofxie commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Usage

Changes

Scope

Uh oh!

DingmaomaoBJTU left a comment

Choose a reason for hiding this comment

Review Comments

⚠️ Unrelated e2e test removals (~200 lines)

⚠️ Potential bug: torch→numpy in _ONNXSessionModel.__call__

Minor: No --input-data + --reference combined test

👍 Overall

Uh oh!

xieofxie commented Jul 29, 2026

Uh oh!

DingmaomaoBJTU left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DingmaomaoBJTU left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DingmaomaoBJTU left a comment

Choose a reason for hiding this comment

Uh oh!

DingmaomaoBJTU left a comment

Choose a reason for hiding this comment

Uh oh!

DingmaomaoBJTU left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

xieofxie commented Jul 20, 2026 •

edited

Loading

⚠️ Potential bug: torch→numpy in `_ONNXSessionModel.call`

Minor: No `--input-data` + `--reference` combined test