Repository navigation
Add provider smoke harness (openvino/qnn/migraphx/coreml/dnnl EP smokes) - #391
Conversation
…ense gates Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
…trained CPU Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Summary
This PR adds a Python-based provider smoke test harness and TTS benchmarking infrastructure. The smoke harness tests execution provider loading and basic inference, while the TTS benchmark framework measures Kokoro TTS performance.
Critical Issues Found (3)
Three defects were identified that could cause crashes or incorrect behavior:
- Python crash risk in
qnn_backend_path(): Missing ImportError handling will crash whenonnxruntime_qnnis not installed - Unused import: The
ctypesmodule is imported but never used - Python crash risk in
bench_run(): Function will crash withValueErrorwhenrepeatsparameter is 0
The C# code (TtsEvalRunner.cs and related files) is well-structured with proper error handling and validation. The logic correctly validates that repeatRuns >= 1 at the argument parsing level, preventing invalid values from reaching the benchmark execution.
Recommendation
Address the three Python defects before merging. The C# implementation is production-ready.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoAdd execution-provider smoke harness and TTS benchmark command
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3d1f0a71e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Review by Qodo
1.
|
Qodo Fixer🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (2) 🔗 Fix PR: #392 This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR. Prompt for coding agent Process — 2 fixed
|
There was a problem hiding this comment.
All reported issues were addressed across 7 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Provider status detection is inaccurate, valid inputs are mishandled, and unrelated unverified changes from separate PRs are included.
Review effort: Balanced
Findings: 1
Open (10)
Provider list misreports actual node execution · New Model alias lookup rejects valid manifest aliases · New Validate per-job overrides before benchmark execution · New Preserve input tensor dtypes from explicit NPZ feeds · New Comma-delimited provider options are parsed as separate providers · New Remove unrelated design spec from this provider-smoke PR · New Usage advertises unsupported Windows ML provider tokens · New Unrelated tts-bench changes lack .NET validation evidence · New Missing ROCm prerequisite should be labeled absent · New JSON feed format is advertised but unsupported · New
What changed in this PR
Adds a lightweight Python ONNX Runtime provider smoke harness, but also includes unrelated TTS benchmarking and Piper design work from separate PRs.
Changes:
- Adds provider loading, inference, fallback-status, and timing checks.
- Adds the
tts-benchcommand and tests. - Adds the Piper CPU TTS design specification.
| File | Description |
|---|---|
tools/provider-smoke/smoke_execution_providers.py |
Implements provider smoke runs and reporting. |
tools/provider-smoke/README.md |
Documents harness usage and provider prerequisites. |
src/Trackdub.Benchmarks/TtsEvalRunner.cs |
Implements TTS benchmark jobs. |
src/Trackdub.Benchmarks/Program.cs |
Registers tts-bench. |
src/Trackdub.Benchmarks/BenchmarkConsole.cs |
Adds command usage text. |
tests/Trackdub.Benchmarks.Tests/TtsEvalRunnerTests.cs |
Tests TTS benchmark behavior. |
docs/specs/design-piper-cpu-tts-tier.md |
Proposes a benchmark-first Piper fallback tier. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Review submitted: pullrequestreview-5432028722 Independent read-only review of the Overall: The provider-smoke harness's core status contract is unsound — its headline "never fabricated" guarantee doesn't hold — and the stacked
Out of diff: None. |
|
/oc fix |
Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
|
All verification passed. Here's the summary for the PR. Autopilot pass — provider-smoke harnessMerge conflicts: none (PR is Scope: this branch stacks the piper spec (#386) and Fixed (provider-smoke, 35 threads)
Not fixed (dismissed as out of scope, 21 threads)
Blocked on a human: none beyond the |



Summary
tools/provider-smoke/— a dependency-light Python harness that answers "does execution provider X load, take nodes, and run on this host" in seconds, complementing the .NETprovider-matrixscenario (which measures shipped pipeline stages end-to-end)cpu,openvino,qnn(auto-registers the wheel's provider library + HTP backend path),migraphx,dnnl,coremltokens; explicit never-fabricated status semantics:ok/fallback(EP loaded, all nodes on CPU) /fail(with error) /absent(platform prerequisite missing — e.g. CoreML on non-macOS)--feednpz, avoiding misleading smoke results)Verification
cpuok,openvinook), qnn wheel (qnnfallback — clean load, no Hexagon device, matches repo QNN readiness-probe semantics), migraphx wheel (migraphxfallback — missing ROCmlibmigraphx)python -m py_compilecleanInitial evidence summary (GPU/NPU-less Intel sandbox)
openvinoEP works on the CPU plugin but is ~11% slower than plain CPU EP on transformer encoders and rejects Kokoro's dynamic-rank STFT graphqnnEP loads cleanly viaregister_execution_provider_libraryand falls back to CPU without Hexagon; the wheel ships no x86QnnCpubackendmigraphxwheel exists for Linux x86_64 but requires the AMD ROCm stack (absent); CoreML wheel is macOS-onlytools/onnxruntime-dnnl/)