fix(build): forward --ep; require explicit device + ep in resolve_device#947
Draft
xieofxie wants to merge 3 commits into
Draft
fix(build): forward --ep; require explicit device + ep in resolve_device#947xieofxie wants to merge 3 commits into
xieofxie wants to merge 3 commits into
Conversation
…ilability `winml build` dropped --ep when auto-generating a config — it passed only device/precision to generate_build_config, so the requested EP never reached config generation. With an explicit --device that isn't present, config gen then resolved the device with ep=None and failed immediately with a device-availability error, silently ignoring --ep (e.g. `winml build -m microsoft/resnet-50 --ep openvino --device npu --compile`). - Forward `ep` into generate_build_config so the EP shapes the generated config. - Add an early, compile-gated EP-availability check after config validation: a compile build physically instantiates the EP, so an unavailable EP/device now fails fast (before export) instead of deep in the compile stage. A no-compile build only produces a portable, analyzed ONNX and may target a device absent on the build machine (cross-compile), so it is allowed to proceed. - Stop the optimize-stage device resolution from availability-failing: it only needs a concrete device name for the analyzer's rule-data lookup, so it no longer passes ep or rejects an absent explicit device. Verified end-to-end: `--compile --device npu --ep openvino` (no NPU) now fails before export with the EP respected; `--no-compile` proceeds to export the portable model. Related to the EP-threading family (#931).
resolve_device(device, *, ep) now takes both device and ep as required args, so an omission is a call error instead of a silent ep=None / device="auto". Several callers still relied on the old defaults (latent TypeErrors) — pass ep=None explicitly where no EP filter is intended: - analyzer: auto-device resolution for rule-file selection - build: the ep-is-None auto-EP resolution - eval: device resolution - compile stage: device resolution (EP comes from ep_config) - test_device.py: resolve_device call sites Also keeps the build command forwarding --ep to generate_build_config, and drops the compile-availability gate / optimize-stage tweak from earlier on this branch — requiring explicit device+ep is the cleaner, centralized enforcement.
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
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.
Summary
Two related fixes around EP handling, after
winml build -m ... --ep openvino --device npu --compilesilently ignored--ep.1.
winml builddropped--epThe build command passed only
device/precisiontogenerate_build_config— neverep— so the requested EP never reached config generation. Nowepis forwarded.2.
resolve_devicemadedevice+epboth required, and all callers fixedresolve_device(device, *, ep)previously defaulteddevice="auto"(andephad recently become required, leaving several callers passing neither — latentTypeErrors, plus 8 failingtest_device.pytests). Both are now required, so an omission is a call error instead of a silentep=None/device="auto". This is the root-cause class behind bug #931 / the build drop: implicit EP resolution.Callers that relied on the old defaults now pass
ep=Noneexplicitly where no EP filter is intended:ep is Noneauto-EP resolutionep_config)test_device.pycall sitesNotes
An earlier iteration of this branch added a compile-availability gate + an optimize-stage tweak; those were reverted in favor of the centralized signature enforcement above, which is cleaner and catches the whole class of bug at the boundary.
Verification
winml build --ep openvino --device npu --compilenow consults the EP (fails on absent NPU, as expected) instead of silently ignoring--ep.tests/unit/{commands,sysinfo,compiler,analyze,config}(2500+ passed). 3 new build tests + thetest_device.pycall sites updated.Related: #931 (fixed in #941).