Skip to content

Commit baa659f

Browse files
committed
fix: require explicit device + ep in resolve_device; fix all callers
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.
1 parent b762c59 commit baa659f

7 files changed

Lines changed: 17 additions & 80 deletions

File tree

src/winml/modelkit/analyze/analyzer.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,7 @@ def _build_runtime_debug_details_summary(
140140
level_bucket[node_stable_key] = candidate_entry
141141
continue
142142

143-
if (
144-
existing_entry.case_indices is None
145-
and candidate_entry.case_indices is not None
146-
):
143+
if existing_entry.case_indices is None and candidate_entry.case_indices is not None:
147144
existing_entry.case_indices = candidate_entry.case_indices
148145

149146
if existing_entry.table_path is None and candidate_entry.table_path is not None:
@@ -798,7 +795,7 @@ def analyze_from_proto(
798795
if device is not None and device.lower() == "auto":
799796
from ..sysinfo import resolve_device
800797

801-
resolved, _ = resolve_device("auto")
798+
resolved, _ = resolve_device("auto", ep=None)
802799
device_to_use = resolved.upper()
803800
logger.info("Device 'auto' resolved to: %s", device_to_use)
804801
else:

src/winml/modelkit/commands/build.py

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ def build(
554554
from ..sysinfo import resolve_eps as _resolve_eps
555555

556556
try:
557-
resolved_device, _ = _resolve_device(device=device)
557+
resolved_device, _ = _resolve_device(device=device, ep=None)
558558
except ValueError as e:
559559
raise click.UsageError(str(e)) from e
560560
device = resolved_device
@@ -642,24 +642,6 @@ def _patch_device(cfg: WinMLBuildConfig) -> None:
642642
except ValueError as e:
643643
raise click.UsageError(f"Config validation failed: {e}") from e
644644

645-
# Fail fast for compile builds: compilation physically instantiates the
646-
# EP (EPContext), so the target EP/device must be present on this
647-
# machine. Catch an unavailable combo here -- before export/optimize --
648-
# instead of surfacing deep in the compile stage. A no-compile build
649-
# only produces a portable, analyzed ONNX and may legitimately target a
650-
# device absent on this machine (cross-compile), so it is left to run.
651-
for _cfg in _configs_to_validate:
652-
_compile = _cfg.compile
653-
if _compile is None or _compile.ep_config is None:
654-
continue
655-
from ..sysinfo import resolve_device as _resolve_device_check
656-
657-
try:
658-
_resolve_device_check(device=device or "auto", ep=_compile.ep_config.provider)
659-
except ValueError as e:
660-
raise click.UsageError(str(e)) from e
661-
break
662-
663645
preloaded_hf_config = _validate_loader_tasks_for_model(
664646
model_id=model_id,
665647
configs=_configs_to_validate,
@@ -1031,18 +1013,11 @@ def _on_iteration_start(iteration: int, max_iter: int) -> None:
10311013
_header_shown[0] = False
10321014

10331015
# Resolve "auto" to a concrete device once so that has_rule_data_for_ep
1034-
# doesn't search for non-existent "*_AUTO_*.parquet" files. Only a device
1035-
# name is needed (the EP comes from each analyzer callback below), so
1036-
# don't pass ep or availability-fail here: a no-compile cross-compile
1037-
# build may target a device absent on this machine (compile builds are
1038-
# gated earlier, before export).
1016+
# doesn't search for non-existent "*_AUTO_*.parquet" files.
10391017
from ..analyze.utils.ep_utils import has_rule_data_for_ep
10401018
from ..sysinfo import resolve_device as _resolve_device
10411019

1042-
if device and device.lower() != "auto":
1043-
_resolved_device = device.lower()
1044-
else:
1045-
_resolved_device, _ = _resolve_device(device="auto")
1020+
_resolved_device, _ = _resolve_device(device=device or "auto", ep=ep)
10461021

10471022
def _on_ep_start(ep_name: EPName, operator_counts: dict) -> None:
10481023
nonlocal _current_ep

src/winml/modelkit/commands/eval.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ def _resolve_device(cfg: WinMLEvaluationConfig) -> None:
364364

365365
console = Console(stderr=True)
366366
console.print("[bold]Detecting available devices...[/bold]")
367-
resolved, _ = resolve_device(cfg.device)
367+
resolved, _ = resolve_device(cfg.device, ep=None)
368368
cfg.device = resolved
369369
console.print(f"[dim]Using device:[/dim] {resolved}")
370370

src/winml/modelkit/compiler/stages/compile.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ def _compile_multiple(self, context: CompileContext) -> None:
176176
sess_options = context.shared_session_options
177177
if sess_options is None:
178178
register_execution_providers(ort=True)
179-
resolved_device, _ = resolve_device(context.config.get("device", "auto"))
179+
resolved_device, _ = resolve_device(context.config.get("device", "auto"), ep=None)
180180
ep = normalize_ep_name(ep_config.provider) or resolve_eps(resolved_device)[0]
181181
device_type = DEVICE_TO_DEVICE_TYPE.get(resolved_device.upper())
182182

src/winml/modelkit/sysinfo/device.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ def _get_available_eps() -> frozenset[EPName]:
145145

146146

147147
def resolve_device(
148-
device: str = "auto",
148+
device: str,
149149
*,
150150
ep: EPNameOrAlias | None,
151151
) -> tuple[str, list[str]]:

tests/unit/commands/test_build.py

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,38 +1742,3 @@ def test_ep_forwarded_to_generate_build_config(
17421742
)
17431743
assert result.exit_code == 0, result.output
17441744
assert mock_gen.call_args.kwargs["ep"] == "openvino"
1745-
1746-
def test_compile_build_unavailable_ep_fails_before_build(
1747-
self, tmp_path: Path, mock_run_single_build: MagicMock
1748-
):
1749-
"""A compile build whose EP isn't present fails fast, before _run_single_build.
1750-
1751-
Compilation physically instantiates the EP, so an unavailable EP must be
1752-
caught up front rather than deep in the compile stage.
1753-
"""
1754-
cfg = _make_minimal_config_file(tmp_path, compile_section={"execution_provider": "qnn"})
1755-
with patch(
1756-
"winml.modelkit.sysinfo.resolve_device",
1757-
side_effect=ValueError("Requested EP 'qnn' is not available on this system."),
1758-
):
1759-
result = _invoke([*self._base_args(cfg, tmp_path), "--ep", "qnn", "--compile"])
1760-
assert result.exit_code == 2, result.output
1761-
assert "not available on this system" in result.output
1762-
mock_run_single_build.assert_not_called()
1763-
1764-
def test_no_compile_build_skips_ep_availability_gate(
1765-
self, tmp_path: Path, mock_run_single_build: MagicMock
1766-
):
1767-
"""A no-compile build proceeds even when the EP isn't present (cross-compile).
1768-
1769-
No-compile only produces a portable, analyzed ONNX, so it must not require
1770-
the target EP/device to exist on the build machine.
1771-
"""
1772-
cfg = _make_minimal_config_file(tmp_path, compile_section={"execution_provider": "qnn"})
1773-
with patch(
1774-
"winml.modelkit.sysinfo.resolve_device",
1775-
side_effect=ValueError("Requested EP 'qnn' is not available on this system."),
1776-
):
1777-
result = _invoke([*self._base_args(cfg, tmp_path), "--ep", "qnn", "--no-compile"])
1778-
assert result.exit_code == 0, result.output
1779-
mock_run_single_build.assert_called_once()

tests/unit/sysinfo/test_device.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ def test_no_npu_no_gpu(self) -> None:
8383
def test_returns_empty_when_enumeration_fails(self) -> None:
8484
"""If EP enumeration raises, return empty tuple (no devices visible).
8585
86-
``resolve_device("auto")`` is responsible for the CPU fallback when no
86+
``resolve_device("auto", ep=None)`` is responsible for the CPU fallback when no
8787
devices are reachable; ``_get_available_devices`` only reports what is
8888
actually registered.
8989
"""
@@ -214,7 +214,7 @@ def test_resolve_device_auto_npu_with_ep(self) -> None:
214214
"cpu": ("CPUExecutionProvider",),
215215
}
216216
):
217-
device, available = resolve_device("auto")
217+
device, available = resolve_device("auto", ep=None)
218218

219219
assert device == "npu"
220220
assert available == ["npu", "gpu", "cpu"]
@@ -227,15 +227,15 @@ def test_resolve_device_auto_npu_without_ep(self) -> None:
227227
"cpu": ("CPUExecutionProvider",),
228228
}
229229
):
230-
device, available = resolve_device("auto")
230+
device, available = resolve_device("auto", ep=None)
231231

232232
assert device == "gpu"
233233
assert available == ["gpu", "cpu"]
234234

235235
def test_resolve_device_auto_cpu_fallback(self) -> None:
236236
"""Auto mode: only CPU EP registered -> returns "cpu"."""
237237
with _patch_device_ep_map({"cpu": ("CPUExecutionProvider",)}):
238-
device, available = resolve_device("auto")
238+
device, available = resolve_device("auto", ep=None)
239239

240240
assert device == "cpu"
241241
assert available == ["cpu"]
@@ -248,15 +248,15 @@ def test_resolve_device_explicit_valid(self) -> None:
248248
"cpu": ("CPUExecutionProvider",),
249249
}
250250
):
251-
device, available = resolve_device("gpu")
251+
device, available = resolve_device("gpu", ep=None)
252252

253253
assert device == "gpu"
254254
assert available == ["gpu", "cpu"]
255255

256256
def test_resolve_device_explicit_invalid(self) -> None:
257257
"""Unrecognized device "tpu" -> raises ValueError."""
258258
with pytest.raises(ValueError, match="Unknown device 'tpu'"):
259-
resolve_device("tpu")
259+
resolve_device("tpu", ep=None)
260260

261261
def test_resolve_device_explicit_no_ep_error_names_missing_eps(self) -> None:
262262
"""Error message must name the compatible EPs so users know what to install."""
@@ -268,7 +268,7 @@ def test_resolve_device_explicit_no_ep_error_names_missing_eps(self) -> None:
268268
),
269269
pytest.raises(ValueError) as exc_info,
270270
):
271-
resolve_device("npu")
271+
resolve_device("npu", ep=None)
272272

273273
message = str(exc_info.value)
274274
assert "no compatible EP" in message
@@ -278,7 +278,7 @@ def test_resolve_device_explicit_no_ep_error_names_missing_eps(self) -> None:
278278
def test_resolve_device_case_insensitive(self) -> None:
279279
"""Device argument should be case-insensitive."""
280280
with _patch_device_ep_map({"cpu": ("CPUExecutionProvider",)}):
281-
device, _ = resolve_device("CPU")
281+
device, _ = resolve_device("CPU", ep=None)
282282

283283
assert device == "cpu"
284284

@@ -293,7 +293,7 @@ def test_resolve_device_no_eps_raises(self) -> None:
293293
_patch_device_ep_map({}),
294294
pytest.raises(RuntimeError, match="No execution providers detected"),
295295
):
296-
resolve_device("auto")
296+
resolve_device("auto", ep=None)
297297

298298

299299
class TestResolveDeviceWithEp:

0 commit comments

Comments
 (0)