diff --git a/src/winml/modelkit/commands/eval.py b/src/winml/modelkit/commands/eval.py index 113ef79a3..111dd1c50 100644 --- a/src/winml/modelkit/commands/eval.py +++ b/src/winml/modelkit/commands/eval.py @@ -162,6 +162,7 @@ "random inputs and report tensor-similarity metrics per output tensor." ), ) +@cli_utils.skip_build_option() @cli_utils.build_config_option() @cli_utils.verbosity_options() @click.pass_context @@ -191,6 +192,7 @@ def eval( show_schema: bool, mode: EvalMode, config_file: Path | None, + skip_build: bool, ) -> None: r"""Evaluate a model for a task. diff --git a/src/winml/modelkit/commands/perf.py b/src/winml/modelkit/commands/perf.py index 38d09e658..e027d96b4 100644 --- a/src/winml/modelkit/commands/perf.py +++ b/src/winml/modelkit/commands/perf.py @@ -79,6 +79,7 @@ class BenchmarkConfig: no_quantize: bool = False rebuild: bool = False ignore_cache: bool = False + skip_build: bool = True allow_unsupported_nodes: bool = False monitor: bool = False ep: EPNameOrAlias | None = None @@ -321,13 +322,25 @@ def run(self) -> BenchmarkResult: return self._collect_results(stats) def _load_model(self) -> None: - """Load model via WinMLAutoModel (handles both HF and ONNX).""" + """Load model via WinMLAutoModel. + + Both HF model IDs and pre-exported .onnx files flow through this + single path so latency numbers stay comparable: HF runs export → + optimize → [quantize] → [compile], and ONNX runs the same pipeline + minus export. + """ from ..config import WinMLBuildConfig from ..models import WinMLAutoModel model_id = self.config.model_id model_path = Path(model_id) - is_onnx = model_path.suffix.lower() == ".onnx" and model_path.exists() + is_onnx = model_path.suffix.lower() == ".onnx" + if is_onnx and not model_path.exists(): + # Surface a clear error for programmatic callers. The CLI guards + # this earlier, but without this check from_pretrained would fall + # through to HF loading and produce a confusing "not a valid JSON + # file" error from AutoConfig. + raise FileNotFoundError(f"ONNX file not found: {model_path}") # Only override config when user explicitly passes --no-quantize override = None @@ -353,6 +366,7 @@ def _load_model(self) -> None: if is_onnx: self._model = WinMLAutoModel.from_onnx( onnx_path=model_path, + skip_build=self.config.skip_build, **common_kwargs, ) else: @@ -957,10 +971,7 @@ def _run_monitored_loop( model_id: str, device: str, ) -> None: - """Run the benchmark iteration loop with live hardware monitoring. - - Shared by both HF-path (PerfBenchmark) and ONNX-path (_run_onnx_benchmark). - """ + """Run the benchmark iteration loop with live hardware monitoring.""" display = LiveMonitorDisplay( total_iterations=total_iterations, warmup=warmup, @@ -990,10 +1001,7 @@ def _run_simple_loop( inputs: dict[str, Any], total_iterations: int, ) -> None: - """Run the benchmark iteration loop with periodic debug logging. - - Shared by both HF-path (PerfBenchmark) and ONNX-path (_run_onnx_benchmark). - """ + """Run the benchmark iteration loop with periodic debug logging.""" for i in range(total_iterations): session.run(inputs) @@ -1001,109 +1009,6 @@ def _run_simple_loop( logger.debug("Progress: %d/%d", i + 1, total_iterations) -# ============================================================================= -# ONNX Direct Benchmark -# ============================================================================= - - -def _run_onnx_benchmark( - onnx_path: Path, - *, - device: str, - iterations: int, - warmup: int, - batch_size: int, - config: BenchmarkConfig, -) -> BenchmarkResult: - """Benchmark an ONNX file directly via WinMLSession (no HF build). - - Creates a WinMLSession, reads io_config for input shapes, - generates random inputs, and runs the standard benchmark loop. - """ - from ..session import WinMLSession - - session = WinMLSession(onnx_path=onnx_path, device=device, ep=config.ep) - - # Generate random inputs from session's I/O config - io_cfg = session.io_config - inputs = generate_random_inputs(io_config=io_cfg, batch_size=batch_size) - - # Compile session early so session.device is resolved for display - session.compile() - - # Print model info before benchmark starts - _print_model_info(io_cfg, req_device=device, act_device=session.device, ep_name=session.ep_name) - - # Run benchmark - total_iterations = warmup + iterations - hw_metrics = None - hw_ctx = None - - # Determine if hardware monitoring is available - if config.monitor: - from ..session.monitor.hw_monitor import HWMonitor - - if HWMonitor.is_available(): - hw_ctx = HWMonitor( - poll_interval_ms=_HW_POLL_INTERVAL_MS, - device=session.device or device, - ep_name=session.ep_name, - ) - else: - Console(stderr=True).print( - "[yellow]Warning:[/yellow] HWMonitor unavailable. " - "Running ONNX benchmark without monitoring." - ) - - if hw_ctx: - with session.perf(warmup=warmup) as stats, hw_ctx as hw: - _run_monitored_loop( - session, - inputs, - stats, - hw, - total_iterations=total_iterations, - warmup=warmup, - model_id=str(onnx_path.name), - device=session.device or device, - ) - hw_metrics = hw.to_dict() - else: - with session.perf(warmup=warmup) as stats: - _run_simple_loop(session, inputs, total_iterations) - - # Collect results - mean_latency_sec = stats.mean_ms / 1000.0 - samples_per_sec = batch_size / mean_latency_sec if mean_latency_sec > 0 else 0 - batches_per_sec = 1.0 / mean_latency_sec if mean_latency_sec > 0 else 0 - samples = stats.samples_ms - std_ms = float(np.std(samples)) if samples else 0.0 - - return BenchmarkResult( - config=config, - input_names=io_cfg["input_names"], - input_shapes=[list(s) if s else [] for s in io_cfg["input_shapes"]], - input_types=[str(t) for t in io_cfg["input_types"]], - output_names=io_cfg["output_names"], - output_shapes=[list(s) if s else [] for s in io_cfg["output_shapes"]], - mean_ms=stats.mean_ms, - min_ms=stats.min_ms, - max_ms=stats.max_ms, - p50_ms=stats.p50_ms, - p90_ms=stats.p90_ms, - p95_ms=stats.p95_ms, - p99_ms=stats.p99_ms, - std_ms=std_ms, - raw_samples_ms=stats.samples_ms, - samples_per_sec=samples_per_sec, - batches_per_sec=batches_per_sec, - actual_device=session.device, - actual_task="n/a (direct ONNX)", - actual_ep=session.ep_name, - hw_monitor=hw_metrics, - ) - - # ============================================================================= # CLI Command # ============================================================================= @@ -1179,6 +1084,7 @@ def _run_onnx_benchmark( default=False, help="Build from scratch in a temp folder (discard after benchmarking)", ) +@cli_utils.skip_build_option() @cli_utils.allow_unsupported_nodes_option() @click.option( "--module", @@ -1222,6 +1128,7 @@ def perf( no_quantize: bool, rebuild: bool, ignore_cache: bool, + skip_build: bool, allow_unsupported_nodes: bool, module_class: str | None, monitor: bool, @@ -1235,8 +1142,10 @@ def perf( Measures latency and throughput using random input data generated from the model's I/O configuration. - Accepts both HuggingFace model IDs and local .onnx files. - HF models go through PerfBenchmark; .onnx files use _run_onnx_benchmark. + Accepts both HuggingFace model IDs and local .onnx files. Both flow + through the same PerfBenchmark pipeline (optimize → [quantize] → [compile] + minus export for ONNX inputs), so latency numbers are directly comparable + between the two inputs. \b Examples: @@ -1356,6 +1265,7 @@ def perf( no_quantize=no_quantize, rebuild=rebuild, ignore_cache=ignore_cache, + skip_build=skip_build, allow_unsupported_nodes=allow_unsupported_nodes, monitor=monitor, ep=ep, @@ -1367,33 +1277,25 @@ def perf( is_onnx = model_path.suffix.lower() == ".onnx" if is_onnx: - # ONNX direct path -- skip HF build, benchmark via WinMLSession + # Validate file existence up front; otherwise WinMLAutoModel would + # fall through to HF loading and surface a confusing + # "not a valid JSON file" error from AutoConfig. + if not model_path.exists(): + raise FileNotFoundError(f"ONNX file not found: {model_path}") if shape_config: console.print( "[yellow]Warning:[/yellow] --shape-config is ignored for " "pre-exported ONNX files (shapes are baked into the model)." ) config.shape_config = None - if not model_path.exists(): - raise FileNotFoundError(f"ONNX file not found: {model_path}") console.print(f"[dim]Benchmarking ONNX:[/dim] {model_path}") - - result = _run_onnx_benchmark( - model_path, - device=config.device, - iterations=iterations, - warmup=warmup, - batch_size=batch_size, - config=config, - ) else: - # HF model path -- full build + benchmark via PerfBenchmark if precision != "auto": console.print(f"[dim]Precision: {precision} (applied during model build)[/dim]") console.print(f"[dim]Loading model:[/dim] {hf_model}") - benchmark = PerfBenchmark(config) - result = benchmark.run() + benchmark = PerfBenchmark(config) + result = benchmark.run() # Display console report display_console_report(result, console) diff --git a/src/winml/modelkit/commands/run.py b/src/winml/modelkit/commands/run.py index 031b175d0..81afd655d 100644 --- a/src/winml/modelkit/commands/run.py +++ b/src/winml/modelkit/commands/run.py @@ -497,6 +497,7 @@ def _print_input_hint(engine: Any) -> None: default=False, help="Auto-connect to a running winml serve instance instead of embedded inference", ) +@cli_utils.skip_build_option() @cli_utils.allow_unsupported_nodes_option() @click.pass_context def run( @@ -515,6 +516,7 @@ def run( port: int, connect_host: str, connect: bool, + skip_build: bool, allow_unsupported_nodes: bool, ) -> None: r"""Run one-shot inference on a model. @@ -633,6 +635,7 @@ def run( task=task, device=device, ep=ep, + skip_build=skip_build, allow_unsupported_nodes=allow_unsupported_nodes, ) except (OSError, ValueError, RuntimeError) as exc: diff --git a/src/winml/modelkit/eval/config.py b/src/winml/modelkit/eval/config.py index ffd06dd75..653111367 100644 --- a/src/winml/modelkit/eval/config.py +++ b/src/winml/modelkit/eval/config.py @@ -119,6 +119,7 @@ class WinMLEvaluationConfig: dataset: DatasetConfig = field(default_factory=DatasetConfig) output_path: Path | None = field(default=None, metadata={"cli_name": "output"}) mode: EvalMode = "onnx" + skip_build: bool = True def to_dict(self) -> dict: """Convert to dictionary for serialization.""" @@ -141,6 +142,7 @@ def to_dict(self) -> dict: result["output_path"] = str(self.output_path) if self.mode != "onnx": result["mode"] = self.mode + result["skip_build"] = self.skip_build return result @classmethod @@ -171,4 +173,5 @@ def from_dict(cls, data: dict) -> WinMLEvaluationConfig: dataset=dataset, output_path=(Path(data["output_path"]) if data.get("output_path") else None), mode=data.get("mode", "onnx"), + skip_build=data.get("skip_build", True), ) diff --git a/src/winml/modelkit/eval/evaluate.py b/src/winml/modelkit/eval/evaluate.py index 4c0bfc1ab..69fda09c8 100644 --- a/src/winml/modelkit/eval/evaluate.py +++ b/src/winml/modelkit/eval/evaluate.py @@ -208,7 +208,7 @@ def _load_model(config: WinMLEvaluationConfig) -> WinMLPreTrainedModel: task=config.task, device=config.device, ep=config.ep, - skip_build=True, + skip_build=config.skip_build, hf_config=hf_config, ) model.config = hf_config diff --git a/src/winml/modelkit/inference/engine.py b/src/winml/modelkit/inference/engine.py index 77650050a..9f4c28f72 100644 --- a/src/winml/modelkit/inference/engine.py +++ b/src/winml/modelkit/inference/engine.py @@ -298,6 +298,7 @@ def load( task: str | None = None, device: str = "auto", ep: EPNameOrAlias | None = None, + skip_build: bool = True, allow_unsupported_nodes: bool = False, ) -> None: """Load model from model_path. @@ -307,14 +308,22 @@ def load( task: Required when model_path is a raw .onnx file. device: "auto" | "cpu" | "gpu" | "npu". ep: Explicit EP short name (e.g. "dml", "qnn"). Overrides device. + skip_build: When True (default), use a raw .onnx file as-is. + When False, run the build pipeline + (optimize/quantize/compile) before inference. Honored only + for raw .onnx paths; ignored for HF model IDs and pre-built + build directories. A build directory always loads its + cached ONNX as-is — to re-run the build pipeline on a + model already in a build dir, point ``model_path`` at the + explicit .onnx file inside it. allow_unsupported_nodes: If True, warn instead of raising when the analyzer reports unsupported nodes during an HF build. Note: has - no effect when loading from a pre-built ONNX file or a cached - build directory (no build/analyze step runs in those paths). + no effect on raw .onnx files or pre-built build directories + (no build/analyze step runs in those paths). """ self._model_path = str(model_path) - self._device = device self._ep = ep + self._device = device self._load_start = time.time() path = Path(model_path) @@ -345,7 +354,7 @@ def load( else: raise elif path.suffix == ".onnx" and path.exists(): - self._load_from_onnx(path, task=task, device=device, ep=ep) + self._load_from_onnx(path, task=task, device=device, ep=ep, skip_build=skip_build) else: self._load_from_hf( str(model_path), @@ -965,15 +974,16 @@ def _load_from_onnx( task: str | None, device: str, ep: EPNameOrAlias | None, + skip_build: bool = True, ) -> None: from ..models.auto import WinMLAutoModel self._task = task self._model_id = None self._model = WinMLAutoModel.from_onnx( - onnx_path, task=task, device=device, ep=ep, skip_build=True + onnx_path, task=task, device=device, ep=ep, skip_build=skip_build ) - logger.info("Loaded from ONNX: %s task=%s", onnx_path, task) + logger.info("Loaded from ONNX: %s task=%s skip_build=%s", onnx_path, task, skip_build) def _load_from_hf( self, diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index 2829119b4..1a08feca8 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -274,6 +274,39 @@ def build_config_option(help: str | None = None) -> Callable[[F], F]: ) +def skip_build_option( + default: bool = True, + optional_message: str | None = None, +) -> Callable[[F], F]: + """Add --skip-build/--no-skip-build toggle for commands that accept ONNX inputs. + + When skip-build is on, the build pipeline (optimize -> [quantize] -> [compile]) + is bypassed and the ONNX file is used as-is. Applies only to ONNX inputs. + + Args: + default: Default value (True = skip build by default, use --no-skip-build + to run the full build pipeline on the ONNX file). + optional_message: Extra command-specific guidance appended to help text. + + Returns: + Decorator function. + """ + help_text = ( + "Skip the build pipeline (optimize/quantize/compile) and use the ONNX " + "file as-is. Use --no-skip-build to run the full build pipeline. " + "Applies only to ONNX inputs." + ) + if optional_message: + help_text = f"{help_text} {optional_message}" + + return click.option( + "--skip-build/--no-skip-build", + default=default, + show_default=True, + help=help_text, + ) + + def trust_remote_code_option(optional_message: str | None = None) -> Callable[[F], F]: """Add shared --trust-remote-code option to a Click command. diff --git a/tests/e2e/test_perf_e2e.py b/tests/e2e/test_perf_e2e.py index de25b3e09..c82a4cf27 100644 --- a/tests/e2e/test_perf_e2e.py +++ b/tests/e2e/test_perf_e2e.py @@ -109,6 +109,7 @@ def _build_perf_args( module: str | None = None, monitor: bool = False, verbose: bool = False, + no_skip_build: bool = False, ) -> list[str]: """Build the argv list passed to the perf CLI. @@ -137,6 +138,8 @@ def _build_perf_args( args.append("--monitor") if verbose: args.append("--verbose") + if no_skip_build: + args.append("--no-skip-build") return args @@ -244,6 +247,23 @@ def test_benchmark_cpu_verbose(self, tmp_path: Path, model_arg: str): assert output_file.exists() assert "Results saved to" in result.output + def test_benchmark_cpu_build(self, tmp_path: Path, model_arg: str): + """Benchmark with --no-skip-build should succeed and show debug output.""" + output_file = tmp_path / "perf_cpu_build.json" + + runner = CliRunner() + result = runner.invoke( + perf, + _build_perf_args( + model_arg=model_arg, output_file=output_file, device="cpu", no_skip_build=True + ), + obj={}, + catch_exceptions=False, + ) + assert result.exit_code == 0, f"perf failed (exit {result.exit_code}):\n{result.output}" + assert output_file.exists() + assert "Results saved to" in result.output + def test_benchmark_cpu_monitor(self, tmp_path: Path, model_arg: str): """Benchmark on CPU with --monitor. @@ -419,9 +439,7 @@ def test_benchmark_ep_device_gpu(self, ep: str, tmp_path: Path, model_arg: str): assert output_file.exists() data = json.loads(output_file.read_text()) # Tiny synthetic fixture: below PDH utilization-publish floor. - _assert_monitor_result( - data, device="gpu", ep=EP_ALIASES[ep], require_utilization=False - ) + _assert_monitor_result(data, device="gpu", ep=EP_ALIASES[ep], require_utilization=False) @pytest.mark.parametrize("ep", NPU_EPS) def test_benchmark_ep_device_npu(self, ep: str, tmp_path: Path, model_arg: str): @@ -448,9 +466,7 @@ def test_benchmark_ep_device_npu(self, ep: str, tmp_path: Path, model_arg: str): assert output_file.exists() data = json.loads(output_file.read_text()) # Tiny synthetic fixture: below PDH utilization-publish floor. - _assert_monitor_result( - data, device="npu", ep=EP_ALIASES[ep], require_utilization=False - ) + _assert_monitor_result(data, device="npu", ep=EP_ALIASES[ep], require_utilization=False) # =========================================================================== diff --git a/tests/unit/commands/test_config_value_priority.py b/tests/unit/commands/test_config_value_priority.py index 2aee9457b..36833db75 100644 --- a/tests/unit/commands/test_config_value_priority.py +++ b/tests/unit/commands/test_config_value_priority.py @@ -279,7 +279,12 @@ def _run_perf( json_dict: dict | None, tmp_path: Path, ) -> dict[str, Any]: - """Drive ``winml perf`` (ONNX-file branch) and capture ``BenchmarkConfig``.""" + """Drive ``winml perf`` and capture ``BenchmarkConfig``. + + Both HF and ONNX inputs now flow through ``PerfBenchmark`` (see PR #596 + ``fix(perf): unify HF and ONNX paths``), so this adapter patches the + class itself and captures the ``BenchmarkConfig`` from its constructor. + """ from winml.modelkit.commands.perf import perf as perf_cmd model = _make_fake_onnx(tmp_path) @@ -287,9 +292,12 @@ def _run_perf( captured: dict[str, Any] = {} - def fake_run(model_path, *, config=None, **_kw): + def fake_benchmark(config): captured["config"] = config - return MagicMock() + instance = MagicMock() + instance.run.return_value = MagicMock() + captured["instance"] = instance + return instance args = ["-m", str(model), *cli_args] if config_path is not None: @@ -297,8 +305,8 @@ def fake_run(model_path, *, config=None, **_kw): with ( patch( - "winml.modelkit.commands.perf._run_onnx_benchmark", - side_effect=fake_run, + "winml.modelkit.commands.perf.PerfBenchmark", + side_effect=fake_benchmark, ), patch("winml.modelkit.commands.perf.display_console_report"), patch("winml.modelkit.commands.perf.write_json_report"), @@ -310,8 +318,16 @@ def fake_run(model_path, *, config=None, **_kw): r = CliRunner().invoke(perf_cmd, args, obj={}, catch_exceptions=False) assert r.exit_code == 0, r.output + # Guard against the perf command short-circuiting before the benchmark + # runs (e.g. an early return after constructing PerfBenchmark): the + # captured BenchmarkConfig would still let the priority assertions pass + # but the command wouldn't be exercising the real flow. + assert captured["instance"].run.call_count == 1, ( + "PerfBenchmark was constructed but .run() was never invoked" + ) + cfg = captured["config"] - return {"ep": cfg.ep} + return {"ep": cfg.ep, "skip_build": cfg.skip_build} def _run_analyze( @@ -529,6 +545,7 @@ def fake_evaluate(cfg): "ep": cfg.ep, "task": cfg.task, "device": cfg.device, + "skip_build": cfg.skip_build, "dataset_samples": cfg.dataset.samples, "dataset_name": cfg.dataset.name, } @@ -704,15 +721,11 @@ def test_t2_beats_t3(self, case: FieldCase, tmp_path: Path) -> None: _check_t2_beats_t3(_run_compile, case, tmp_path) @pytest.mark.parametrize("case", COMPILE_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_empty_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_empty_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_empty_section(_run_compile, case, tmp_path) @pytest.mark.parametrize("case", COMPILE_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_absent_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_absent_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_absent_section(_run_compile, case, tmp_path) @@ -728,17 +741,40 @@ def test_t2_beats_t3(self, case: FieldCase, tmp_path: Path) -> None: _check_t2_beats_t3(_run_perf, case, tmp_path) @pytest.mark.parametrize("case", PERF_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_empty_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_empty_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_empty_section(_run_perf, case, tmp_path) @pytest.mark.parametrize("case", PERF_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_absent_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_absent_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_absent_section(_run_perf, case, tmp_path) + # ------------------------------------------------------------------ + # Targeted tests for the ``--skip-build/--no-skip-build`` toggle. + # Unlike ``ep``, ``skip_build`` has NO JSON config source: perf's merge + # block (perf.py) only reads ``task`` and ``execution_provider`` from + # the ``-c`` file, so ``skip_build`` flows CLI -> BenchmarkConfig + # directly with no Tier-2 path. That, plus the boolean flag not fitting + # the FieldCase ``[flag, value]`` shape, is why it's tested explicitly + # here rather than as a PERF_CASES FieldCase. These guard against a + # param-name mismatch in the ``perf()`` signature and against the CLI + # option default drifting from the BenchmarkConfig field default. + # ------------------------------------------------------------------ + + def test_skip_build_default_is_true(self, tmp_path: Path) -> None: + """No flag -> cfg.skip_build keeps the True default.""" + eff = _run_perf([], None, tmp_path) + assert eff["skip_build"] is True + + def test_no_skip_build_flag_sets_false(self, tmp_path: Path) -> None: + """``--no-skip-build`` -> cfg.skip_build is False.""" + eff = _run_perf(["--no-skip-build"], None, tmp_path) + assert eff["skip_build"] is False + + def test_skip_build_flag_sets_true(self, tmp_path: Path) -> None: + """``--skip-build`` -> cfg.skip_build is True.""" + eff = _run_perf(["--skip-build"], None, tmp_path) + assert eff["skip_build"] is True + class TestAnalyzePriority: """``winml analyze`` priority contract.""" @@ -752,15 +788,11 @@ def test_t2_beats_t3(self, case: FieldCase, tmp_path: Path) -> None: _check_t2_beats_t3(_run_analyze, case, tmp_path) @pytest.mark.parametrize("case", ANALYZE_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_empty_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_empty_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_empty_section(_run_analyze, case, tmp_path) @pytest.mark.parametrize("case", ANALYZE_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_absent_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_absent_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_absent_section(_run_analyze, case, tmp_path) @@ -776,15 +808,11 @@ def test_t2_beats_t3(self, case: FieldCase, tmp_path: Path) -> None: _check_t2_beats_t3(_run_export, case, tmp_path) @pytest.mark.parametrize("case", EXPORT_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_empty_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_empty_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_empty_section(_run_export, case, tmp_path) @pytest.mark.parametrize("case", EXPORT_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_absent_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_absent_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_absent_section(_run_export, case, tmp_path) # ------------------------------------------------------------------ @@ -797,13 +825,9 @@ def test_t3_not_shadowed_by_absent_section( # explicitly here. # ------------------------------------------------------------------ - def test_json_export_enable_hierarchy_tags_false_applies( - self, tmp_path: Path - ) -> None: + def test_json_export_enable_hierarchy_tags_false_applies(self, tmp_path: Path) -> None: """JSON ``{"export": {"enable_hierarchy_tags": false}}`` reaches cfg.""" - eff = _run_export( - [], {"export": {"enable_hierarchy_tags": False}}, tmp_path - ) + eff = _run_export([], {"export": {"enable_hierarchy_tags": False}}, tmp_path) assert eff["enable_hierarchy_tags"] is False def test_empty_export_section_keeps_enable_hierarchy_tags_cli_default( @@ -823,9 +847,7 @@ def test_json_export_dynamo_true_applies(self, tmp_path: Path) -> None: eff = _run_export([], {"export": {"dynamo": True}}, tmp_path) assert eff["dynamo"] is True - def test_empty_export_section_keeps_dynamo_cli_default( - self, tmp_path: Path - ) -> None: + def test_empty_export_section_keeps_dynamo_cli_default(self, tmp_path: Path) -> None: """JSON ``{"export": {}}`` must NOT change ``dynamo``. Guards the same fix as ``enable_hierarchy_tags`` above. @@ -846,15 +868,11 @@ def test_t2_beats_t3(self, case: FieldCase, tmp_path: Path) -> None: _check_t2_beats_t3(_run_quantize, case, tmp_path) @pytest.mark.parametrize("case", QUANTIZE_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_empty_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_empty_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_empty_section(_run_quantize, case, tmp_path) @pytest.mark.parametrize("case", QUANTIZE_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_absent_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_absent_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_absent_section(_run_quantize, case, tmp_path) @@ -877,20 +895,42 @@ def test_t2_beats_t3(self, case: FieldCase, tmp_path: Path) -> None: _check_t2_beats_t3(_run_eval, case, tmp_path) @pytest.mark.parametrize("case", EVAL_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_empty_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_empty_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_empty_section(_run_eval, case, tmp_path) @pytest.mark.parametrize("case", EVAL_CASES, ids=lambda c: c.field) - def test_t3_not_shadowed_by_absent_section( - self, case: FieldCase, tmp_path: Path - ) -> None: + def test_t3_not_shadowed_by_absent_section(self, case: FieldCase, tmp_path: Path) -> None: _check_t3_not_shadowed_by_absent_section(_run_eval, case, tmp_path) - def test_empty_quant_section_does_not_leak_to_dataset_samples( - self, tmp_path: Path - ) -> None: + # ------------------------------------------------------------------ + # Targeted tests for the ``--skip-build/--no-skip-build`` toggle. + # Unlike perf's ``skip_build`` (no JSON source -> CLI-only), eval's + # ``skip_build`` has a full Tier-2 path: ``{"eval": {"skip_build": ...}}`` + # flows through ``merge_config(cfg, eval_data)`` in ``_build_eval_config``, + # and the CLI layer (``collect_cli_overrides``) overrides it. The boolean + # flag doesn't fit the FieldCase ``[flag, value]`` shape, so the full + # CLI > config-file > default contract is verified explicitly here. + # ------------------------------------------------------------------ + + def test_skip_build_default_is_true(self, tmp_path: Path) -> None: + """Tier 3: no flag, no JSON -> cfg.skip_build keeps the True default.""" + assert _run_eval([], None, tmp_path)["skip_build"] is True + + def test_no_skip_build_flag_sets_false(self, tmp_path: Path) -> None: + """Tier 1: ``--no-skip-build`` -> cfg.skip_build is False.""" + assert _run_eval(["--no-skip-build"], None, tmp_path)["skip_build"] is False + + def test_json_skip_build_false_applies(self, tmp_path: Path) -> None: + """Tier 2: config file ``{"eval": {"skip_build": false}}`` must take effect.""" + eff = _run_eval([], {"eval": {"skip_build": False}}, tmp_path) + assert eff["skip_build"] is False + + def test_cli_beats_json_skip_build(self, tmp_path: Path) -> None: + """Tier 1 > Tier 2: explicit CLI ``--skip-build`` must win over JSON False.""" + eff = _run_eval(["--skip-build"], {"eval": {"skip_build": False}}, tmp_path) + assert eff["skip_build"] is True + + def test_empty_quant_section_does_not_leak_to_dataset_samples(self, tmp_path: Path) -> None: """JSON ``{"quant": {}}`` must NOT change ``cfg.dataset.samples``. Pins Bug 2a: today ``WinMLQuantizationConfig.samples`` default ``10`` @@ -903,9 +943,7 @@ def test_empty_quant_section_does_not_leak_to_dataset_samples( f"expected CLI default 100, got {eff['dataset_samples']!r}" ) - def test_explicit_quant_samples_does_not_leak_to_dataset_samples( - self, tmp_path: Path - ) -> None: + def test_explicit_quant_samples_does_not_leak_to_dataset_samples(self, tmp_path: Path) -> None: """JSON ``{"quant": {"samples": 50}}`` must NOT change ``cfg.dataset.samples``. diff --git a/tests/unit/commands/test_perf_cli.py b/tests/unit/commands/test_perf_cli.py index 298630269..542271284 100644 --- a/tests/unit/commands/test_perf_cli.py +++ b/tests/unit/commands/test_perf_cli.py @@ -10,6 +10,7 @@ from __future__ import annotations +import json import re from pathlib import Path from unittest.mock import MagicMock, patch @@ -279,14 +280,22 @@ def test_no_quantize_false_passes_no_override(self) -> None: override = mock_from_pretrained.call_args.kwargs["config"] assert override is None - def test_cli_onnx_goes_through_onnx_benchmark(self, runner: CliRunner, tmp_path: Path) -> None: - """CLI with .onnx file should route through _run_onnx_benchmark.""" + def test_cli_onnx_routes_through_perf_benchmark( + self, runner: CliRunner, tmp_path: Path + ) -> None: + """CLI with .onnx file should route through the same PerfBenchmark as HF. + + Both paths must share the build+benchmark pipeline so latency numbers + from `winml perf -m hf/id` and `winml perf -m ` are + comparable (issue #596). + """ onnx_file = tmp_path / "model.onnx" onnx_file.write_bytes(b"fake onnx") with ( - patch( - "winml.modelkit.commands.perf._run_onnx_benchmark", + patch.object( + PerfBenchmark, + "run", return_value=MagicMock(), ) as mock_run, patch( @@ -305,6 +314,55 @@ def test_cli_onnx_goes_through_onnx_benchmark(self, runner: CliRunner, tmp_path: assert result.exit_code == 0, result.output mock_run.assert_called_once() + def test_cli_onnx_clears_shape_config_with_warning( + self, runner: CliRunner, tmp_path: Path + ) -> None: + """ONNX input with --shape-config: warn + clear shape_config before PerfBenchmark. + + Shapes are baked into a pre-exported ONNX, so --shape-config is silently + ignored; we want to be sure the CLI both surfaces the warning to the + user and actually drops the override before constructing PerfBenchmark. + """ + onnx_file = tmp_path / "model.onnx" + onnx_file.write_bytes(b"fake onnx") + + shape_cfg_file = tmp_path / "shapes.json" + shape_cfg_file.write_text(json.dumps({"input_ids": [1, 128]})) + + captured: dict[str, BenchmarkConfig] = {} + + def capture_config(config: BenchmarkConfig) -> MagicMock: + captured["config"] = config + mock = MagicMock() + mock.run.return_value = MagicMock() + return mock + + with ( + patch( + "winml.modelkit.commands.perf.PerfBenchmark", + side_effect=capture_config, + ), + patch("winml.modelkit.commands.perf.display_console_report"), + patch("winml.modelkit.commands.perf.write_json_report"), + ): + result = runner.invoke( + perf, + [ + "-m", + str(onnx_file), + "--shape-config", + str(shape_cfg_file), + "-o", + str(tmp_path / "out.json"), + ], + obj={}, + ) + + assert result.exit_code == 0, result.output + assert "shape-config is ignored" in result.output + assert "Benchmarking ONNX" in result.output + assert captured["config"].shape_config is None + def test_cli_onnx_not_found_error(self, runner: CliRunner, tmp_path: Path) -> None: """CLI with non-existent .onnx file should raise FileNotFoundError.""" missing = tmp_path / "missing.onnx" diff --git a/tests/unit/commands/test_run.py b/tests/unit/commands/test_run.py index 8a8a2854c..ec0d67a5f 100644 --- a/tests/unit/commands/test_run.py +++ b/tests/unit/commands/test_run.py @@ -322,6 +322,7 @@ def test_embedded_text_inference(self, runner: CliRunner) -> None: task=None, device="auto", ep=None, + skip_build=True, allow_unsupported_nodes=False, ) engine.predict.assert_called_once_with(inputs={"text": "hello world"}) @@ -465,6 +466,7 @@ def test_task_forwarded_to_load(self, runner: CliRunner) -> None: task="image-classification", device="auto", ep=None, + skip_build=True, allow_unsupported_nodes=False, ) @@ -492,6 +494,7 @@ def test_device_and_ep_forwarded(self, runner: CliRunner) -> None: task=None, device="gpu", ep="qnn", + skip_build=True, allow_unsupported_nodes=False, ) diff --git a/tests/unit/commands/test_run_spec.py b/tests/unit/commands/test_run_spec.py index ee70dfd5f..43d37179b 100644 --- a/tests/unit/commands/test_run_spec.py +++ b/tests/unit/commands/test_run_spec.py @@ -98,7 +98,12 @@ def test_model_value_forwarded_to_engine(self, runner: CliRunner) -> None: with patch(_ENGINE_PATH, return_value=engine): runner.invoke(run, ["--model", "microsoft/resnet-50", "--text", "x"]) engine.load.assert_called_once_with( - "microsoft/resnet-50", task=None, device="auto", ep=None, allow_unsupported_nodes=False + "microsoft/resnet-50", + task=None, + device="auto", + ep=None, + skip_build=True, + allow_unsupported_nodes=False, )