Make e2e eval recipe/precision expansion NPU-only - #1163
Merged
Conversation
Recipes and multi-precision expansion now apply only when --device is npu. On NPU, a model without a recipe falls back to winml config expanded into two jobs (w8a8 + w8a16); an explicit per-model precision overrides this. CPU/GPU (and auto) ignore recipes and build a single winml config fallback. Update _build_jobs to take the device, add EvalJob.fallback_precision, adjust _build_for_job, refresh docs and tests.
The fallback build path invoked `winml build -c <config> --device <d>` without `--precision`. Because `--device` is always passed, winml build re-resolves quant/compile from device+precision and overwrites the quant baked into the config by `winml config`. With precision defaulting to auto, an NPU build fell back to w8a16, so a w8a8 fallback job and a w8a16 fallback job produced the same cached artifact (identical config hash) and perf reported w8a16 for both. Forward the resolved precision to the build call as well, mirroring the existing `winml config` invocation, so distinct-precision fallback jobs build distinct artifacts. Add regression tests asserting --precision is passed to both config and build, distinct precisions stay distinct, and the flag is omitted when precision is None.
DingmaomaoBJTU
requested changes
Jul 22, 2026
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
Collaborator
There was a problem hiding this comment.
CPU/GPU also need recipes for winml eval. Please just ignore quantized version for them and add fallbacks
The e2e eval runner previously ignored authored recipes entirely off-NPU and always built a single 'winml config' fallback, which also dropped the accuracy eval/dataset config the recipes carry. '_build_jobs' now discovers recipe variants on every device and filters out quantized variants (delegating to winml's 'is_quantized_precision') when the target is not NPU. Off-NPU builds the non-quantized recipe variants (e.g. fp16) and falls back to 'winml config' only when no applicable variant exists. NPU behavior (fp16 + quantized variants, w8a8+w8a16 fallback) is unchanged. Update the README, module docstring, and tests to match.
DingmaomaoBJTU
left a comment
DingmaomaoBJTU
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the NPU-only recipe/precision expansion. The core change is sound and the --precision forwarding fix is well-reasoned and well-tested. A few observations below — one edge case worth a look (NPU + VitisAI), plus two low-severity notes.
Address PR review feedback: - Suppress the recipe-less NPU w8a8+w8a16 expansion for skip-quant EPs (VitisAI): _resolve_precision forces the flag off for these EPs, so both jobs would build the same unquantized artifact under distinct precision slugs. Thread ep into _build_jobs and emit a single fallback instead. - EvalJob.precision now falls back to entry.precision so a recipe-less job with an explicit per-model precision records a slug/label matching the built artifact.
DingmaomaoBJTU
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
In the e2e eval harness, recipe-driven builds and multi-precision job expansion were applied on every device. That only makes sense on the NPU: recipes target NPU quantization, and expanding a model into several quantized precisions has no meaning on CPU/GPU. This change scopes that behavior to
--device npuand gives NPU models without a recipe a sensible default matrix.What changed
_build_jobstakes the targetdevice. Recipes are only consulted whendevice == "npu". On CPU/GPU/auto, recipes are ignored and each model builds as a singlewinml configfallback job (prior behavior).precisionstill wins and produces a single job.--precisiontowinml build. The fallback path invokedwinml build -c <config> --device npuwithout--precision. Because--deviceis always passed,winml buildre-resolves quant from device+precision and overwrites the quant baked into the config bywinml config. With precision defaulting to auto, the NPU build reverted to w8a16, so a w8a8 job and a w8a16 job produced the same cached artifact (identical config hash) and perf reported w8a16 for both. The build call now forwards the resolved precision, mirroring thewinml configinvocation.Notes for reviewers
device == "npu"match (auto does not count), consistent with_resolve_precision._run_recipe_build) passes neither--devicenor--precision, so the config's quant stays authoritative there. It was never affected by the overwrite issue.--precisionoverwrite mechanism predates this PR and was latent on main (NPU fallback only ever resolved to w8a16, which matched the build's auto default). Introducing the w8a8 variant is what surfaced it; the fix also closes the latent case where a per-modelfp16entry built on NPU would be silently coerced to w8a16.--liststill prints one line per model and does not preview expanded jobs. That is pre-existing behavior and unchanged here.Docs (
scripts/e2e_eval/README.md) and unit tests are updated to cover the NPU-only expansion, the w8a8 + w8a16 fallback, and the--precisionforwarding.