Skip to content

feat(e2e-eval): support pre-exported ONNX models#902

Merged
xieofxie merged 12 commits into
mainfrom
hualxie/run_onnx
Jul 14, 2026
Merged

feat(e2e-eval): support pre-exported ONNX models#902
xieofxie merged 12 commits into
mainfrom
hualxie/run_onnx

Conversation

@xieofxie

@xieofxie xieofxie commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds support for evaluating models that ship a pre-exported ONNX in the e2e_eval harness, including HF refs that point at a specific .onnx file under a repo. This supports models such as the PaddleOCR *_onnx repos, where the native HF export path is blocked by the pinned transformers / optimum-onnx dependency stack.

When a registry entry points at an ONNX file, the harness downloads that file from the HF repo and feeds the local ONNX path to winml config, winml build, and winml perf, bypassing HF-to-ONNX export.

Changes

  • run_eval.py: resolves registry ONNX files before build/perf, preserves ONNX metadata through --hf-model task overrides, and resolves direct-ONNX build artifacts deterministically.
  • Analyzer metadata: removes the analyzer-only opset floor so ONNX models are recorded with their actual opset instead of being rejected before downstream build/optimize handling.
  • Registry data: updates PaddleOCR curated/all entries to use the ONNX file refs and explicit task metadata.
  • build_registry.py: preserves ONNX metadata during registry rebuilds, adds --recheck-downloads to refresh download counts before ordering, preserves existing download counts if a recheck lookup fails, and normalizes nested .onnx HF refs to org/model only for HF API calls while keeping the full registry hf_id.
  • Tests: adds/updates coverage for ONNX eval wiring, task-required cache validation, analyzer opset metadata recording, registry ONNX API normalization, and download rechecks.

Validation

  • uv run pytest tests/unit/eval/test_run_eval_script.py
  • uv run pytest tests/unit/commands/test_build.py -q
  • uv run pytest tests/unit/analyze/models/test_onnx_model.py -q
  • uv run pytest tests/unit/test_build_registry.py -q
  • End-to-end PaddleOCR ONNX perf run on CPU completed successfully for the detector model.

@xieofxie xieofxie requested a review from a team as a code owner June 16, 2026 08:20
Comment thread scripts/e2e_eval/run_eval.py Fixed
Comment thread tests/unit/eval/test_run_eval_script.py Fixed
Comment thread tests/unit/eval/test_run_eval_script.py Fixed
hualxie added 2 commits June 16, 2026 16:28
CodeQL flagged 'onnx' being imported with both 'import onnx' and
'from onnx import ...' in the same module. Use attribute access
(onnx.version_converter / onnx.helper / onnx.TensorProto) throughout
instead of mixing import styles.
The Phase 2 onnx_file passthrough was dead: load_curated_entries
dropped every key except hf_id/task/group/priority, so a rebuild
that re-added or newly-created a curated onnx_file entry would lose
it. Carry onnx_file through when present.
@xieofxie

Copy link
Copy Markdown
Contributor Author

need to update this in favor of #582

@xieofxie xieofxie changed the title feat(e2e-eval): support pre-exported ONNX models via onnx_file feat(e2e-eval): support pre-exported ONNX models Jul 10, 2026
Comment thread src/winml/modelkit/analyze/models/onnx_model.py Outdated

@DingmaomaoBJTU DingmaomaoBJTU left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the ONNX-eval enablement. The code changes themselves are small and mostly reasonable, but the PR description is significantly out of sync with what actually landed — worth reconciling before merge.

Description vs. diff mismatch
The "Changes" section describes functionality that isn't in this diff:

  • run_eval.py is said to "resolve registry ONNX files before build/perf, preserve ONNX metadata through --hf-model, upgrade sub-minimum opsets, and resolve direct-ONNX build artifacts." The actual run_eval.py diff is formatting-only — there is no onnx_file handling anywhere in the file.
  • The registry is said to carry "ONNX file metadata" / onnx_file, but there is no onnx_file key. The real mechanism encodes the ONNX path directly in hf_id (e.g. PaddlePaddle/PP-OCRv6_medium_det_onnx/inference.onnx) and leans on winml's existing hub_onnx resolution of org/repo/path/file.onnx refs. That's a cleaner design, but the description (and the earlier load_curated_entries passthrough commit) describe the reverted approach.

It looks like the "revert and update" commit changed the approach. Please update the description so reviewers know the ONNX support rides on winml's existing nested-onnx resolution plus the registry/opset/cache tweaks here — as written it's misleading about where the behavior lives and what's actually tested.

models_all.json churn (FYI)
The ~1.6k-line diff there is almost entirely downloads/order refresh from --recheck-downloads (440/443 entries changed downloads, 315 reordered). Only the two PaddleOCR entries actually change identity (v5 → v6 _onnx). Verified: no entries added/dropped and no duplicate keys.

Inline notes below.

Comment thread scripts/e2e_eval/build_registry.py Outdated
Comment thread scripts/e2e_eval/build_registry.py
Comment thread src/winml/modelkit/analyze/models/onnx_model.py Outdated
Comment thread src/winml/modelkit/analyze/models/onnx_model.py Outdated
Comment thread src/winml/modelkit/commands/build.py Outdated
Comment thread scripts/e2e_eval/testsets/models_curated.json Outdated

@DingmaomaoBJTU DingmaomaoBJTU left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The follow-up commit (7527b4c) cleanly addresses all three review threads:

  • onnx_model.py — the analyzer opset floor is removed entirely, so the model records its opset without gating (a reasonable separation of concerns: analyze records, build enforces). Stale "upgrades sub-minimum opsets" wording dropped from the PR body.
  • build.py — reverted to match main exactly, restoring the fail-fast on missing loader.task. Safe for the ONNX eval because run_eval.py passes --task from the registry entry, and the PaddleOCR entries now carry explicit task metadata.
  • models_curated.json — explicit task: image-to-text added, keeping curated source and the built registry consistent across rebuilds. The v5-server → v6-medium swap is confirmed intentional.

Verified locally against the PR head: test_onnx_model.py + test_build.py = 127 passed, and ruff check is clean on the changed files.

Minor non-blocking note: removing the opset floor outright (vs. lowering it) means genuinely ancient opsets now surface only later in optimize rather than at analysis time — acceptable given the layering.

@xieofxie xieofxie merged commit 2e37dc8 into main Jul 14, 2026
9 checks passed
@xieofxie xieofxie deleted the hualxie/run_onnx branch July 14, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants