microsoft / microsoft/winml-cli

Compiler & Device/EP Subsystem — Deep Dive Analysis (51 findings)

Open
#78 1 comment 0 reactions 1 assignee View on GitHub

@tezheng is already working on this.

Since Mar 31, 2026.

bug dev experience GPU graph-optimizer hardware NPU P1 QDQ static-analyzer triaged
Dominant language
Python
Stars
40
Forks
11
Avg merge
1d 8h
Merged PRs (30d)
50

Description

ModelKit Compiler & Device/EP — Final Issue List

Date: 2026-02-28 | Commit: cee639d on main
Methodology: Original analysis (43 findings) → 3 independent reviews (code reviewer, fact-checker, devil's advocate) → synthesized verdicts
Analysis Grade: B (88.7% factually verified, 1 fabricated code snippet, severity inflation corrected)


Must Fix

# Severity File Issue
1 LOW compiler/cli.py Dead code — imports deleted CalibrationConfig, QDQConfig, CalibrateStage, DetectStage. Crashes on import. Delete the file.
2 HIGH compiler/stages/qformat.py QFormatConvertStage.should_run() returns True but process() only warns — passes unconverted QLinear model to QNN EP which will fail at runtime. Should error or skip.
3 HIGH compiler/configs.py:232-281 to_dict() omits verbose field. from_dict() reads it as False. Roundtrip silently loses verbose mode during CompileStage.

Should Fix

# Severity File Issue
4 MEDIUM commands/compile.py:223-236 _resolve_compile_provider() hardcodes "qnn" fallback when --device auto. Auto-detection is advertised but never works.
5 HIGH winml.py + ep_registry.py Two parallel WinML singletons manage same Windows App SDK lifecycle. Unsafe __del__, print vs logging, duplicated DLL deletion.
6 HIGH config/precision.py vs sysinfo/device.py OpenVINO mapped as "gpu" in precision.py but "npu/gpu/cpu" in device.py — contradictory data.
7 HIGH qairt_session.py:143-145 No binary file validation after QAIRT subprocess compilation. rename() on nonexistent file causes cryptic downstream error.
8 MEDIUM session/session.py:482-493 _detect_best_device() returns "auto" — the same value that triggered the call. Cache filename becomes model_auto_ctx.onnx.
9 HIGH utils/python_env.py:75-80 input() call during QAIRT venv setup hangs indefinitely in CI/CD and automated pipelines.

Nice to Fix

# Severity File Issue
10 MEDIUM All commands Device case sensitivity: static-analyzer expects UPPERCASE (NPU), all other commands expect lowercase (npu).
11 MEDIUM qairt_session.py:158 QAIRT utility path hardcoded to aarch64-windows-msvc. x86_64 Windows can't find the binary.
12 MEDIUM compiler/configs.py:114-230 8 factory methods are identical copy-paste. Deprecation warning block duplicated 8 times.
13 MEDIUM utils/constants.py:7-11 SUPPORTED_EPS lists only 3 of 8 EPs (QNN, OpenVINO, VitisAI). Misleading name for importing consumers.
14 MEDIUM commands/export.py:256-278 --torch-module flag exists but silently ignored with warning. Either implement or remove.
15 MEDIUM qairt_session.py:109-119 QAIRT SDK root only checks path.exists(), not internal structure (missing lib/, bin/).
16 MEDIUM compiler/compiler.py:95-139 Temp directory cleanup in finally can delete compiled output if _finalize_output() fails.
17 MEDIUM commands/compile.py:61-64 Dead --quantize/--no-quantize flag. Defaults True, prints nothing. --no-quantize prints deprecation.
18 MEDIUM compile.py vs perf.py vs static-analyzer.py EP normalization inconsistent: static-analyzer uses normalize_ep_name(), others just .lower().
19 MEDIUM commands/perf.py:373-380 EP monitor registry hardcoded as dict literal {"vitisai": VitisAIMonitor} in command file.
20 MEDIUM qairt_session.py:82-107 QAIRT 5-step pipeline not atomic. Partial artifacts on failure, no rollback.
21 MEDIUM qairt_session.py:188-204 Nested JSON field access without .get() fallback. Multiple potential KeyError from SDK version differences.
22 MEDIUM utils/python_env.py:60-72 Existing venv only checks Python version, not installed packages. Missing qairt/torch causes later ImportError.
23 MEDIUM static_analyzer/analyzer.py:646-650 Hardcoded EP list duplicates constants.py::SUPPORTED_EPS — yet another EP mapping location.
24 MEDIUM compiler/stages/compile.py:72 CompileStage accesses private winml_session._session directly. Breaks encapsulation.

Won't Fix

# Severity File Issue Rationale
25 N/A compiler/cli.py:49 CLI EP choices limited to 4 of 8 Sub-finding of #1 (same dead file)
26 LOW commands/compile.py:48-52 Default device is "npu" not "auto" Intentional for WinML NPU platform
27 LOW qairt_session.py:88 Python 3.10 hardcoded in QAIRT venv Matches requires-python = ">=3.10,<3.11"
28 LOW qairt_session.py:23 torch==2.4.1 exact pin Likely SDK compatibility constraint
29 LOW 11 files, 17 touchpoints EP mappings scattered with no single source of truth Different modules need different projections. Fix contradictions (#6, #13), not structure
30 LOW sysinfo/device.py:46-55 OpenVINO excluded from _DEVICE_EP_MAP inverse "/" not in _device filter is intentional for multi-device EPs
31 LOW winml.py + ep_registry.py _fix_winrt_runtime() deletes msvcp140.dll, duplicated in two files Resolved when consolidating singletons (#5)
32 LOW winml.py:51-54 __del__ calls __exit__ without try/except Resolved when deleting winml.py::WinML (#5)
33 LOW winml.py:118-149 add_ep_for_device() has "NEVER modify" comment, appears dead code Resolved when deleting winml.py (#5)
34 LOW compiler/context.py:30 CompileContext uses untyped dict[str, Any] for config Real debt, low urgency for pre-alpha
35 LOW compiler/utils.py + stages/optimize.py Model loaded 3+ times in single pipeline run Performance issue, not correctness
36 LOW compiler/transforms.py:35-50 Transform registry is global mutable state, not thread-safe Single-threaded CLI tool
37 LOW sysinfo/hardware.py Windows-only WMI code with no platform guard Project targets Windows
38 LOW sysinfo/hardware.py:7-39 Qualcomm PNP ID quirks undocumented Niche, works correctly
39 LOW sysinfo/device.py:109-140 _get_available_eps() cached with lru_cache, no refresh API EPs don't change at runtime
40 LOW session/session.py:371-408 _build_session_options() mutates self._session_options on default path Functionally benign
41 LOW session/monitor/ QNN and OpenVINO EP monitors are empty placeholders Pre-alpha, implement when needed
42 LOW compiler/compiler.py:16-19 DEVICE_COMPILER_MAPPING only maps QNN, all others fall to default Correct today
43 LOW compiler/stages/compile.py:175-202 Validation uses all-ones dummy data Minor risk
44 LOW winml.py:96-99 vs ep_registry.py:127 print(stderr) vs logging inconsistency Resolved with #5
45 LOW winml.py:15-21 __new__ + global singleton pattern, not thread-safe Resolved with #5
46 LOW session.py:114 vs device.py:37 TensorRT EP name mismatch: NvTensorRTRTX vs Tensorrt Document, don't change
47 LOW session/session.py:634-740 is_compatible() creates full ONNX model per node test Performance, not correctness

Design Issues

ID Issue Verdict
D-01 "device" vs "ep" vs "provider" terminology confusion across all modules Document conventions, don't refactor
D-02 Quantization separation incomplete (compiler/cli.py references old classes) Resolved by deleting file (#1)
D-03 Two competing compilation paths (WinMLSession vs CompileStage) with overlapping responsibilities Document ownership, don't refactor
D-04 _DEVICE_TO_PROVIDER hardcodes npu=qnn, gpu=dml — ignores IHV diversity Fix --device auto (#4) or document as platform defaults

Summary

Bucket Count Action
Must Fix 3 Do immediately — broken behavior
Should Fix 6 Next sprint — correctness and robustness
Nice to Fix 15 Opportunistic — when touching those files
Won't Fix 23 Intentional design or acceptable pre-alpha debt
Design Issues 4 Document, don't refactor
Total 51

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.