From 840e539d86f4c3d8338e3a65fadf6a85cb31951a Mon Sep 17 00:00:00 2001 From: Hualiang Xie Date: Mon, 3 Aug 2026 13:39:03 +0800 Subject: [PATCH 1/5] refactor(cli): centralize cache controls Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/winml/modelkit/commands/build.py | 15 ++----- src/winml/modelkit/utils/cli.py | 52 ++++++++++++++++++++++ tests/unit/utils/test_cli.py | 64 ++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 11 deletions(-) diff --git a/src/winml/modelkit/commands/build.py b/src/winml/modelkit/commands/build.py index 625e22318..3c243c72f 100644 --- a/src/winml/modelkit/commands/build.py +++ b/src/winml/modelkit/commands/build.py @@ -732,17 +732,10 @@ def _maybe_build_genai_bundle( default=None, help="Output directory for all build artifacts", ) -@click.option( - "--use-cache/--no-use-cache", - default=False, - show_default=True, - help="Use WinML CLI global cache (~/.cache/winml/). Mutually exclusive with -o.", -) -@click.option( - "--rebuild/--no-rebuild", - default=False, - show_default=True, - help="Overwrite existing artifacts and rebuild", +@cli_utils.cache_options( + use_cache_default=False, + use_cache_help="Use WinML CLI global cache (~/.cache/winml/). Mutually exclusive with -o.", + rebuild_help="Overwrite existing artifacts and rebuild", ) @cli_utils.quant_option() @cli_utils.compile_option( diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index 46da14c8a..a8db527e6 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -767,6 +767,58 @@ def skip_build_option( ) +def cache_options( + *, + use_cache_default: bool = True, + use_cache_help: str = "Use the persistent model build cache", + rebuild_help: str = "Force rebuild even if cached artifacts exist", +) -> Callable[[F], F]: + """Add the shared cache-control toggles to a Click command. + + The decorated function receives ``use_cache`` and ``rebuild`` parameters. + Commands that auto-build models should translate them with + :func:`cache_extra_kwargs`. ``build`` uses the same option contract with a + command-specific ``use_cache_default=False`` because cache selection is also + its artifact-destination choice. + + Args: + use_cache_default: Whether persistent caching is enabled by default. + use_cache_help: Command-specific help for the cache toggle. + rebuild_help: Command-specific help for the rebuild toggle. + + Returns: + Decorator function. + """ + + def decorator(func: F) -> F: + func = click.option( + "--rebuild/--no-rebuild", + default=False, + show_default=True, + help=rebuild_help, + )(func) + return click.option( + "--use-cache/--no-use-cache", + default=use_cache_default, + show_default=True, + help=use_cache_help, + )(func) + + return decorator + + +def cache_extra_kwargs(*, use_cache: bool, rebuild: bool) -> dict[str, bool]: + """Translate shared cache controls into ``WinMLAutoModel`` keyword arguments. + + Disabling the persistent cache selects a temporary build directory in the + model-loading API, so it must always produce a fresh build. + """ + return { + "use_cache": use_cache, + "force_rebuild": rebuild or not use_cache, + } + + 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/unit/utils/test_cli.py b/tests/unit/utils/test_cli.py index 2c0f55fee..7f737456d 100644 --- a/tests/unit/utils/test_cli.py +++ b/tests/unit/utils/test_cli.py @@ -17,6 +17,8 @@ from winml.modelkit.utils.cli import ( analyze_option, build_pipeline_extra_kwargs, + cache_extra_kwargs, + cache_options, guard_output, ignored_build_flags_warning, load_export_overrides, @@ -317,6 +319,68 @@ def test_combined_optimize_and_analyze(self) -> None: assert result == {"skip_optimize": True, "hack_max_optim_iterations": 0} +class TestCacheOptions: + """Tests for the shared cache_options() decorator.""" + + @staticmethod + def _make_cmd(*, use_cache_default: bool) -> click.Command: + @click.command() + @cache_options( + use_cache_default=use_cache_default, + use_cache_help="Cache help", + rebuild_help="Rebuild help", + ) + def cmd(use_cache: bool, rebuild: bool) -> None: + click.echo(f"{use_cache=},{rebuild=}") + + return cmd + + @pytest.mark.parametrize("use_cache_default", [False, True]) + def test_configurable_cache_default(self, use_cache_default: bool) -> None: + result = CliRunner().invoke(self._make_cmd(use_cache_default=use_cache_default)) + assert result.exit_code == 0 + assert result.output.strip() == f"use_cache={use_cache_default},rebuild=False" + + @pytest.mark.parametrize( + ("flag", "expected"), + [ + ("--use-cache", "use_cache=True,rebuild=False"), + ("--no-use-cache", "use_cache=False,rebuild=False"), + ("--rebuild", "use_cache=True,rebuild=True"), + ("--no-rebuild", "use_cache=True,rebuild=False"), + ], + ) + def test_flag_pairs(self, flag: str, expected: str) -> None: + result = CliRunner().invoke(self._make_cmd(use_cache_default=True), [flag]) + assert result.exit_code == 0 + assert result.output.strip() == expected + + def test_help_uses_command_specific_text(self) -> None: + result = CliRunner().invoke(self._make_cmd(use_cache_default=True), ["--help"]) + assert result.exit_code == 0 + assert "Cache help" in result.output + assert "Rebuild help" in result.output + + +class TestCacheExtraKwargs: + """Tests for the shared cache_extra_kwargs() translator.""" + + @pytest.mark.parametrize( + ("use_cache", "rebuild", "force_rebuild"), + [ + (True, False, False), + (True, True, True), + (False, False, True), + (False, True, True), + ], + ) + def test_mapping(self, use_cache: bool, rebuild: bool, force_rebuild: bool) -> None: + assert cache_extra_kwargs(use_cache=use_cache, rebuild=rebuild) == { + "use_cache": use_cache, + "force_rebuild": force_rebuild, + } + + class TestIgnoredBuildFlagsWarning: """Tests for the shared ignored_build_flags_warning() helper.""" From ec4006e110144423fd068763d986f0f1714b13ea Mon Sep 17 00:00:00 2001 From: Hualiang Xie Date: Mon, 3 Aug 2026 14:02:53 +0800 Subject: [PATCH 2/5] feat(eval): add model cache controls --- docs/commands/eval.md | 11 ++ src/winml/modelkit/commands/eval.py | 43 +++++- src/winml/modelkit/commands/perf.py | 4 +- src/winml/modelkit/eval/config.py | 8 + src/winml/modelkit/eval/evaluate.py | 10 +- src/winml/modelkit/utils/cli.py | 56 +++++-- tests/unit/commands/test_eval.py | 219 ++++++++++++++++++++++++++++ tests/unit/eval/test_eval.py | 29 +++- tests/unit/utils/test_cli.py | 62 +++++++- 9 files changed, 411 insertions(+), 31 deletions(-) diff --git a/docs/commands/eval.md b/docs/commands/eval.md index 1d39df88f..8abd95fff 100644 --- a/docs/commands/eval.md +++ b/docs/commands/eval.md @@ -141,6 +141,17 @@ Evaluate a composite model from pre-exported ONNX files. Some tasks (e.g., `imag $ winml eval -m encoder=encoder.onnx -m decoder=decoder.onnx --model-id microsoft/trocr-base-printed ``` +## Model build cache + +Evaluation reuses persistent model build artifacts by default. Pass +`--no-use-cache` for a fresh build in a temporary directory, or `--rebuild` for +a fresh build that replaces the persistent cache entry. + +For a pre-built ONNX input, cache controls apply only when +`--no-skip-build` is set. Cache controls are ignored when no model build runs, +including two-ONNX comparisons and pre-built GenAI bundles; the CLI warns when +an explicit cache control has no effect. + ## Common pitfalls - **ONNX file without `--model-id` fails.** When `-m` is a `.onnx` path, `--model-id` is mandatory. Without it the command cannot resolve the preprocessor or label vocabulary and will exit with a usage error. diff --git a/src/winml/modelkit/commands/eval.py b/src/winml/modelkit/commands/eval.py index 68d0c3e1c..dde844110 100644 --- a/src/winml/modelkit/commands/eval.py +++ b/src/winml/modelkit/commands/eval.py @@ -209,6 +209,7 @@ "--model-id / --task are not required in this mode." ), ) +@cli_utils.cache_options() @cli_utils.skip_build_option() @cli_utils.format_option() @cli_utils.build_config_option() @@ -253,6 +254,8 @@ def eval( input_data: str | None, reference: str | None, config_file: Path | None, + use_cache: bool, + rebuild: bool, skip_build: bool, ) -> None: r"""Evaluate a model for a task. @@ -334,20 +337,33 @@ def eval( "comes from the leading axis of the provided tensors." ) - # The build-pipeline flags only take effect when eval rebuilds the model. - # With a pre-built ONNX path and skip_build (the default), they are no-ops - # forwarded to from_onnx, so warn the user that they were ignored — mirrors - # the --precision warning above. Shared with perf via utils/cli.py. + build_skip_reason = _model_build_skip_reason(cfg) + + # Build-pipeline flags only take effect when eval builds model artifacts. + # Warn when evaluator-owned or pre-built paths bypass the build entirely. build_flags_warning = cli_utils.ignored_build_flags_warning( - skip_build_onnx=cfg.model_path is not None and cfg.skip_build, + build_runs=build_skip_reason is None, quant=cfg.quant, optimize=cfg.optimize, analyze=cfg.analyze, max_optim_iterations=cfg.max_optim_iterations, + reason=build_skip_reason, + rebuild_hint=("--no-skip-build" if build_skip_reason == "pre-built ONNX inputs" else None), ) if build_flags_warning: logger.warning(build_flags_warning) + cache_flags_warning = cli_utils.ignored_cache_flags_warning( + build_runs=build_skip_reason is None, + use_cache=cfg.use_cache, + rebuild=cfg.rebuild, + use_cache_was_set=cli_utils.is_cli_provided(ctx, "use_cache"), + rebuild_was_set=cli_utils.is_cli_provided(ctx, "rebuild"), + reason=build_skip_reason, + ) + if cache_flags_warning: + logger.warning(cache_flags_warning) + logger.debug("Effective eval config: %s", cfg.to_dict()) json_mode = output_format == "json" @@ -435,6 +451,23 @@ def _build_eval_config( return cfg +def _model_build_skip_reason(cfg: WinMLEvaluationConfig) -> str | None: + """Describe why eval will not build model artifacts, if applicable.""" + if cfg.reference_path is not None: + return "two-ONNX comparisons" + if cfg.model_path is None: + return None + if isinstance(cfg.model_path, str): + model_path = Path(cfg.model_path).expanduser() + if model_path.is_dir() and (model_path / "genai_config.json").is_file(): + return "pre-built GenAI bundles" + if isinstance(cfg.model_path, dict) and cfg.task == "mask-generation": + return "evaluator-managed composite inputs" + if cfg.skip_build: + return "pre-built ONNX inputs" + return None + + def _resolve_model( cfg: WinMLEvaluationConfig, model: tuple[str, ...], diff --git a/src/winml/modelkit/commands/perf.py b/src/winml/modelkit/commands/perf.py index b23a13842..aae0ce7a7 100644 --- a/src/winml/modelkit/commands/perf.py +++ b/src/winml/modelkit/commands/perf.py @@ -3126,11 +3126,13 @@ def perf( # build is skipped (the default). Warn so the silent no-op is visible # — shared detection with eval via utils/cli.py. build_flags_warning = cli_utils.ignored_build_flags_warning( - skip_build_onnx=skip_build, + build_runs=not skip_build, quant=quant, optimize=optimize, analyze=analyze, max_optim_iterations=max_optim_iterations, + reason="pre-built ONNX inputs", + rebuild_hint="--no-skip-build", ) if build_flags_warning: console.print(f"[yellow]Warning:[/yellow] {build_flags_warning}") diff --git a/src/winml/modelkit/eval/config.py b/src/winml/modelkit/eval/config.py index 03944010a..3d6e70e39 100644 --- a/src/winml/modelkit/eval/config.py +++ b/src/winml/modelkit/eval/config.py @@ -172,6 +172,8 @@ class WinMLEvaluationConfig: output_path: Path | None = field(default=None, metadata={"cli_name": "output"}) mode: EvalMode = "onnx" skip_build: bool = True + use_cache: bool = True + rebuild: bool = False _auto_device_selected: bool = field(default=False, repr=False, compare=False, kw_only=True) def to_dict(self) -> dict: @@ -214,6 +216,10 @@ def to_dict(self) -> dict: if self.mode != "onnx": result["mode"] = self.mode result["skip_build"] = self.skip_build + if not self.use_cache: + result["use_cache"] = self.use_cache + if self.rebuild: + result["rebuild"] = self.rebuild return result @classmethod @@ -253,4 +259,6 @@ def from_dict(cls, data: dict) -> WinMLEvaluationConfig: 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), + use_cache=data.get("use_cache", True), + rebuild=data.get("rebuild", False), ) diff --git a/src/winml/modelkit/eval/evaluate.py b/src/winml/modelkit/eval/evaluate.py index 635d034b8..0ab023e9c 100644 --- a/src/winml/modelkit/eval/evaluate.py +++ b/src/winml/modelkit/eval/evaluate.py @@ -264,7 +264,6 @@ def _load_model( if config.mode == "compare" and config.reference_path is not None: return None - quant_override: Any = None if not config.quant: from ..config import WinMLBuildConfig @@ -277,6 +276,10 @@ def _load_model( analyze=config.analyze, max_optim_iterations=config.max_optim_iterations, ) + cache_kwargs = cli_utils.cache_extra_kwargs( + use_cache=config.use_cache, + rebuild=config.rebuild, + ) if config.model_id is None: raise ValueError("model_id is required.") @@ -312,6 +315,7 @@ def _load_model( skip_build=config.skip_build, config=quant_override, hf_config=hf_config, + **cache_kwargs, **pipeline_kwargs, ) model.config = hf_config @@ -341,6 +345,7 @@ def _load_model( allow_unsupported_nodes=config.allow_unsupported_nodes, config=build_override, shape_config=config.shape_config, + **cache_kwargs, **pipeline_kwargs, ) except RuntimeException as error: @@ -386,8 +391,7 @@ def _load_genai_causal_lm(config: WinMLEvaluationConfig) -> WinMLGenaiCausalLM: bundle_path = config.model_path if not bundle_path or isinstance(bundle_path, dict): raise ValueError( - "text-generation evaluation requires a genai bundle *directory* via " - "-m ." + "text-generation evaluation requires a genai bundle *directory* via -m ." ) bundle_dir = Path(bundle_path).expanduser() diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index a8db527e6..d1aebafd8 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -1039,33 +1039,31 @@ def build_pipeline_extra_kwargs( def ignored_build_flags_warning( *, - skip_build_onnx: bool, + build_runs: bool, quant: bool = True, optimize: bool = True, analyze: bool = True, max_optim_iterations: int | None = None, + reason: str | None = None, + rebuild_hint: str | None = None, ) -> str | None: - """Build a warning for build-pipeline flags that are no-ops on a pre-built ONNX. + """Build a warning for build-pipeline flags when no model build runs. - Commands that accept a pre-built ``.onnx`` input (``eval``, ``perf``) forward - ``--no-quant``/``--no-optimize``/``--no-analyze``/``--max-optim-iterations`` to - ``from_onnx``, but with ``skip_build`` (the default) no build runs, so those - toggles silently take no effect. This returns a message naming the flags the - user actually set (or ``None`` when nothing was set or a build will run), so - callers can surface it through their own logger/console — mirroring the - ``--precision``-ignored warning. + Returns a message naming the controls the user changed, or ``None`` when + nothing was changed or a build will run. Args: - skip_build_onnx: True when the input is a pre-built ONNX *and* the build - is skipped (the precondition under which the flags are no-ops). + build_runs: Whether the selected command path builds model artifacts. quant/optimize/analyze: Enabled-semantics toggles (False = user passed the ``--no-*`` form). max_optim_iterations: Explicit value, or ``None`` when left at default. + reason: Description of the path that bypasses the build. + rebuild_hint: Optional flag that enables a build for this path. Returns: Warning message, or ``None`` if no ignored flags apply. """ - if not skip_build_onnx: + if build_runs: return None ignored = [ flag @@ -1079,10 +1077,36 @@ def ignored_build_flags_warning( ] if not ignored: return None - return ( - f"{', '.join(ignored)} ignored for pre-built ONNX inputs " - "(no build runs; pass --no-skip-build to rebuild)." - ) + hint = f"; pass {rebuild_hint} to rebuild" if rebuild_hint else "" + return f"{', '.join(ignored)} ignored for {reason or 'this input'} (no build runs{hint})." + + +def ignored_cache_flags_warning( + *, + build_runs: bool, + use_cache: bool = True, + rebuild: bool = False, + use_cache_was_set: bool = False, + rebuild_was_set: bool = False, + reason: str | None = None, +) -> str | None: + """Build a warning for explicit cache controls when no model build runs.""" + if build_runs: + return None + ignored = [ + flag + for flag, was_set in ( + ( + "--use-cache" if use_cache else "--no-use-cache", + use_cache_was_set or not use_cache, + ), + ("--rebuild" if rebuild else "--no-rebuild", rebuild_was_set or rebuild), + ) + if was_set + ] + if not ignored: + return None + return f"{', '.join(ignored)} ignored for {reason or 'this input'} (no build runs)." def allow_unsupported_nodes_option(optional_message: str | None = None) -> Callable[[F], F]: diff --git a/tests/unit/commands/test_eval.py b/tests/unit/commands/test_eval.py index c68b43fd4..58ca09b93 100644 --- a/tests/unit/commands/test_eval.py +++ b/tests/unit/commands/test_eval.py @@ -446,6 +446,15 @@ def test_help_mentions_reference(self, runner: CliRunner): assert result.exit_code == 0, result.output assert "--reference" in result.output + def test_help_mentions_cache_controls(self, runner: CliRunner): + from winml.modelkit.commands.eval import eval as eval_cmd + + result = runner.invoke(eval_cmd, ["--help"]) + + assert result.exit_code == 0, result.output + assert "--use-cache / --no-use-cache" in result.output + assert "--rebuild / --no-rebuild" in result.output + class TestResolveReference: def test_none_is_noop(self): @@ -614,6 +623,75 @@ def to_dict(self): # config > dataclass defaults (task default is None) assert cfg.task == "image-classification" + @pytest.mark.parametrize( + ("cache_args", "use_cache", "rebuild"), + [ + ([], True, False), + (["--no-use-cache"], False, False), + (["--rebuild"], True, True), + ], + ) + def test_cache_controls_propagate_to_config( + self, + runner: CliRunner, + cache_args: list[str], + use_cache: bool, + rebuild: bool, + ): + from winml.modelkit.commands.eval import eval as eval_cmd + + captured_cfg = {} + + def _fake_evaluate(cfg): + captured_cfg["cfg"] = cfg + return object() + + with ( + patch("winml.modelkit.eval.evaluate", side_effect=_fake_evaluate), + patch("winml.modelkit.commands.eval._resolve_device", return_value=None), + patch("winml.modelkit.commands.eval._write_and_display", return_value=None), + ): + result = runner.invoke( + eval_cmd, + ["-m", "microsoft/resnet-50", *cache_args], + obj={"debug": False}, + ) + + assert result.exit_code == 0, result.output + cfg = captured_cfg["cfg"] + assert cfg.use_cache is use_cache + assert cfg.rebuild is rebuild + + def test_config_file_cache_controls_override_defaults(self, runner: CliRunner, tmp_path): + from winml.modelkit.commands.eval import eval as eval_cmd + + config_path = tmp_path / "eval_config.json" + config_path.write_text( + json.dumps({"eval": {"use_cache": False, "rebuild": True}}), + encoding="utf-8", + ) + captured_cfg = {} + + def _fake_evaluate(cfg): + captured_cfg["cfg"] = cfg + return object() + + with ( + patch("winml.modelkit.eval.evaluate", side_effect=_fake_evaluate), + patch("winml.modelkit.commands.eval._resolve_device", return_value=None), + patch("winml.modelkit.commands.eval._write_and_display", return_value=None), + ): + result = runner.invoke( + eval_cmd, + ["--config", str(config_path), "-m", "microsoft/resnet-50"], + obj={"debug": False}, + ) + + assert result.exit_code == 0, result.output + cfg = captured_cfg["cfg"] + assert cfg.use_cache is False + assert cfg.rebuild is True + def test_cli_default_device_propagates_when_not_explicitly_passed( self, runner: CliRunner, @@ -1152,6 +1230,147 @@ def test_no_warning_when_flags_left_default( ) +class TestIgnoredCacheFlags: + @staticmethod + def _run(runner: CliRunner, args: list[str]): + from winml.modelkit.commands.eval import eval as eval_cmd + + with ( + patch("winml.modelkit.eval.evaluate", return_value=object()), + patch("winml.modelkit.commands.eval._resolve_device", return_value=None), + patch("winml.modelkit.commands.eval._write_and_display", return_value=None), + ): + return runner.invoke(eval_cmd, args, obj={"debug": False}) + + @pytest.mark.parametrize("cache_flag", ["--use-cache", "--no-use-cache", "--rebuild"]) + def test_prebuilt_onnx_warns_for_explicit_cache_control( + self, + cache_flag: str, + runner: CliRunner, + onnx_file, + caplog, + ): + import logging as _logging + + with caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"): + result = self._run( + runner, + ["-m", str(onnx_file), "--model-id", "some/model", cache_flag], + ) + + assert result.exit_code == 0, result.output + assert any( + f"{cache_flag} ignored for pre-built ONNX inputs" in record.getMessage() + for record in caplog.records + ) + + def test_defaults_do_not_warn(self, runner: CliRunner, onnx_file, caplog): + import logging as _logging + + with caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"): + result = self._run( + runner, + ["-m", str(onnx_file), "--model-id", "some/model"], + ) + + assert result.exit_code == 0, result.output + assert not any( + "--use-cache ignored" in record.getMessage() + or "--no-use-cache ignored" in record.getMessage() + or "--rebuild ignored" in record.getMessage() + for record in caplog.records + ) + + def test_two_onnx_comparison_warns_even_with_build_enabled( + self, + runner: CliRunner, + onnx_file, + caplog, + ): + import logging as _logging + + with caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"): + result = self._run( + runner, + [ + "-m", + str(onnx_file), + "--mode", + "compare", + "--reference", + str(onnx_file), + "--no-skip-build", + "--rebuild", + "--no-optimize", + ], + ) + + assert result.exit_code == 0, result.output + assert any( + "--rebuild ignored for two-ONNX comparisons" in record.getMessage() + for record in caplog.records + ) + assert any( + "--no-optimize ignored for two-ONNX comparisons" in record.getMessage() + for record in caplog.records + ) + + def test_genai_bundle_warns_even_with_build_enabled(self, runner: CliRunner, tmp_path, caplog): + import logging as _logging + + bundle = tmp_path / "genai-model" + bundle.mkdir() + (bundle / "genai_config.json").write_text("{}", encoding="utf-8") + + with caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"): + result = self._run( + runner, + [ + "-m", + str(bundle), + "--task", + "text-generation", + "--no-skip-build", + "--no-use-cache", + ], + ) + + assert result.exit_code == 0, result.output + assert any( + "--no-use-cache ignored for pre-built GenAI bundles" in record.getMessage() + for record in caplog.records + ) + + def test_evaluator_managed_composite_warns_even_with_build_enabled( + self, + runner: CliRunner, + onnx_file, + caplog, + ): + import logging as _logging + + with caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"): + result = self._run( + runner, + [ + "-m", + f"encoder={onnx_file}", + "--model-id", + "some/model", + "--task", + "mask-generation", + "--no-skip-build", + "--rebuild", + ], + ) + + assert result.exit_code == 0, result.output + assert any( + "--rebuild ignored for evaluator-managed composite inputs" in record.getMessage() + for record in caplog.records + ) + + # --------------------------------------------------------------------------- # --format json # --------------------------------------------------------------------------- diff --git a/tests/unit/eval/test_eval.py b/tests/unit/eval/test_eval.py index dbf93b72e..80dd3f6c9 100644 --- a/tests/unit/eval/test_eval.py +++ b/tests/unit/eval/test_eval.py @@ -28,7 +28,7 @@ def test_relies_on_model_framework_inference(self) -> None: evaluator.model = MagicMock() sentinel = MagicMock() - with patch("transformers.pipeline", return_value=sentinel) as mock_pipeline: + with patch("transformers.pipelines.pipeline", return_value=sentinel) as mock_pipeline: assert evaluator.prepare_pipeline() is sentinel assert "framework" not in mock_pipeline.call_args.kwargs @@ -111,6 +111,26 @@ def test_config_roundtrip_preserves_input_data(self): restored = WinMLEvaluationConfig.from_dict(config.to_dict()) assert restored.input_data == "inputs.npz" + def test_config_roundtrip_preserves_cache_controls(self): + config = WinMLEvaluationConfig( + model_id="test/model", + use_cache=False, + rebuild=True, + ) + serialized = config.to_dict() + restored = WinMLEvaluationConfig.from_dict(serialized) + + assert serialized["use_cache"] is False + assert serialized["rebuild"] is True + assert restored.use_cache is False + assert restored.rebuild is True + + def test_default_cache_controls_are_omitted(self): + serialized = WinMLEvaluationConfig(model_id="test/model").to_dict() + + assert "use_cache" not in serialized + assert "rebuild" not in serialized + def test_reference_path_default_is_none(self): """reference_path defaults to None and is omitted from to_dict.""" config = WinMLEvaluationConfig(model_id="test/model") @@ -1505,6 +1525,8 @@ def test_load_model_from_pretrained(self): # No --shape-config / export overrides -> both default to None. assert call_args.kwargs["shape_config"] is None assert call_args.kwargs["config"] is None + assert call_args.kwargs["use_cache"] is True + assert call_args.kwargs["force_rebuild"] is False assert result is mock_model def test_auto_target_retries_cpu_after_ort_runtime_failure(self, caplog): @@ -1589,6 +1611,7 @@ def test_load_model_forwards_build_flags(self): quant=False, optimize=False, max_optim_iterations=5, + use_cache=False, ) with patch.dict( @@ -1604,6 +1627,8 @@ def test_load_model_forwards_build_flags(self): # --no-optimize -> skip_optimize; --max-optim-iterations 5 forwarded. assert kwargs["skip_optimize"] is True assert kwargs["hack_max_optim_iterations"] == 5 + assert kwargs["use_cache"] is False + assert kwargs["force_rebuild"] is True def test_load_model_from_onnx(self): """When model_path is set, calls from_onnx and attaches config.""" @@ -1639,4 +1664,6 @@ def test_load_model_from_onnx(self): result = eval_mod._load_model(config) mock_auto.from_onnx.assert_called_once() + assert mock_auto.from_onnx.call_args.kwargs["use_cache"] is True + assert mock_auto.from_onnx.call_args.kwargs["force_rebuild"] is False assert result.config is mock_hf_config diff --git a/tests/unit/utils/test_cli.py b/tests/unit/utils/test_cli.py index 7f737456d..7b9da8bf1 100644 --- a/tests/unit/utils/test_cli.py +++ b/tests/unit/utils/test_cli.py @@ -21,6 +21,7 @@ cache_options, guard_output, ignored_build_flags_warning, + ignored_cache_flags_warning, load_export_overrides, load_input_tensor_specs, max_optim_iterations_option, @@ -388,7 +389,7 @@ def test_returns_none_when_build_runs(self) -> None: """No warning when a build will run, even with flags set.""" assert ( ignored_build_flags_warning( - skip_build_onnx=False, + build_runs=True, quant=False, optimize=False, analyze=False, @@ -399,15 +400,17 @@ def test_returns_none_when_build_runs(self) -> None: def test_returns_none_when_no_flags_set(self) -> None: """No warning when all flags are at their defaults.""" - assert ignored_build_flags_warning(skip_build_onnx=True) is None + assert ignored_build_flags_warning(build_runs=False) is None def test_names_each_set_flag(self) -> None: msg = ignored_build_flags_warning( - skip_build_onnx=True, + build_runs=False, quant=False, optimize=False, analyze=False, max_optim_iterations=5, + reason="pre-built ONNX inputs", + rebuild_hint="--no-skip-build", ) assert msg is not None for flag in ("--no-quant", "--no-optimize", "--no-analyze", "--max-optim-iterations"): @@ -417,7 +420,7 @@ def test_names_each_set_flag(self) -> None: def test_only_includes_set_flags(self) -> None: """Unset flags are not named.""" - msg = ignored_build_flags_warning(skip_build_onnx=True, quant=False) + msg = ignored_build_flags_warning(build_runs=False, quant=False) assert msg is not None assert "--no-quant" in msg assert "--no-optimize" not in msg @@ -426,11 +429,60 @@ def test_only_includes_set_flags(self) -> None: def test_max_optim_zero_counts_as_set(self) -> None: """An explicit 0 is a user-set value (only None is the default).""" - msg = ignored_build_flags_warning(skip_build_onnx=True, max_optim_iterations=0) + msg = ignored_build_flags_warning(build_runs=False, max_optim_iterations=0) assert msg is not None assert "--max-optim-iterations" in msg +class TestIgnoredCacheFlagsWarning: + def test_returns_none_when_build_runs(self) -> None: + assert ( + ignored_cache_flags_warning( + build_runs=True, + use_cache=False, + use_cache_was_set=True, + reason="pre-built ONNX inputs", + ) + is None + ) + + def test_returns_none_when_cache_controls_are_defaults(self) -> None: + assert ( + ignored_cache_flags_warning( + build_runs=False, + reason="pre-built ONNX inputs", + ) + is None + ) + + def test_names_explicit_cache_controls(self) -> None: + msg = ignored_cache_flags_warning( + build_runs=False, + use_cache=False, + rebuild=True, + use_cache_was_set=True, + rebuild_was_set=True, + reason="pre-built ONNX inputs", + ) + + assert msg is not None + assert "--no-use-cache" in msg + assert "--rebuild" in msg + assert "pre-built ONNX inputs" in msg + + def test_nondefault_config_values_count_as_explicit(self) -> None: + msg = ignored_cache_flags_warning( + build_runs=False, + use_cache=False, + rebuild=True, + reason="pre-built ONNX inputs", + ) + + assert msg is not None + assert "--no-use-cache" in msg + assert "--rebuild" in msg + + class TestOverwriteOption: """Tests for the shared overwrite_option() decorator.""" From a7f6583650ceff335ae1cd4271070bc1b3ff6b96 Mon Sep 17 00:00:00 2001 From: Hualiang Xie Date: Mon, 3 Aug 2026 14:37:36 +0800 Subject: [PATCH 3/5] fix(eval): type cache loader kwargs --- src/winml/modelkit/utils/cli.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index d1aebafd8..7d9586ca7 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -9,7 +9,7 @@ import json import os from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal, TypeAlias, TypeVar +from typing import TYPE_CHECKING, Any, Literal, TypeAlias, TypedDict, TypeVar import click from rich.console import Console @@ -33,6 +33,11 @@ OutputFormat: TypeAlias = Literal["text", "json", "table", "compact"] +class _CacheExtraKwargs(TypedDict): + use_cache: bool + force_rebuild: bool + + class ModelLoadError(click.ClickException): """Exit code 3: model could not be loaded onto the device/EP. @@ -807,7 +812,7 @@ def decorator(func: F) -> F: return decorator -def cache_extra_kwargs(*, use_cache: bool, rebuild: bool) -> dict[str, bool]: +def cache_extra_kwargs(*, use_cache: bool, rebuild: bool) -> _CacheExtraKwargs: """Translate shared cache controls into ``WinMLAutoModel`` keyword arguments. Disabling the persistent cache selects a temporary build directory in the From aed7a0a7d69f301a5052a7b73abeeb692e69031c Mon Sep 17 00:00:00 2001 From: Hualiang Xie Date: Tue, 4 Aug 2026 14:07:57 +0800 Subject: [PATCH 4/5] fix(eval): address cache review feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/commands/eval.md | 5 +- src/winml/modelkit/commands/eval.py | 106 +++++++++++-------- src/winml/modelkit/eval/config.py | 6 +- src/winml/modelkit/eval/evaluate.py | 60 +++++++---- src/winml/modelkit/utils/cli.py | 22 ++-- tests/unit/commands/test_eval.py | 151 +++++++++++++++++++++++++++- tests/unit/eval/test_eval.py | 8 +- tests/unit/utils/test_cli.py | 9 +- 8 files changed, 280 insertions(+), 87 deletions(-) diff --git a/docs/commands/eval.md b/docs/commands/eval.md index 8abd95fff..ccdd31253 100644 --- a/docs/commands/eval.md +++ b/docs/commands/eval.md @@ -149,8 +149,9 @@ a fresh build that replaces the persistent cache entry. For a pre-built ONNX input, cache controls apply only when `--no-skip-build` is set. Cache controls are ignored when no model build runs, -including two-ONNX comparisons and pre-built GenAI bundles; the CLI warns when -an explicit cache control has no effect. +including two-ONNX comparisons; the CLI warns when an explicit cache control +has no effect. GenAI's runtime `_compiled/` artifacts are a separate cache and +are not currently governed by these model build cache controls. ## Common pitfalls diff --git a/src/winml/modelkit/commands/eval.py b/src/winml/modelkit/commands/eval.py index dde844110..11183894f 100644 --- a/src/winml/modelkit/commands/eval.py +++ b/src/winml/modelkit/commands/eval.py @@ -299,7 +299,7 @@ def eval( from ..eval import evaluate # ── 1. Build config: defaults ← config file ← CLI ── - cfg = _build_eval_config(ctx, config_file, column, label_mapping_path) + cfg, config_fields = _build_eval_config(ctx, config_file, column, label_mapping_path) if cfg.input_data is not None and cfg.mode != "compare": raise click.UsageError("--input-data is only valid with --mode compare.") @@ -337,39 +337,12 @@ def eval( "comes from the leading axis of the provided tensors." ) - build_skip_reason = _model_build_skip_reason(cfg) - - # Build-pipeline flags only take effect when eval builds model artifacts. - # Warn when evaluator-owned or pre-built paths bypass the build entirely. - build_flags_warning = cli_utils.ignored_build_flags_warning( - build_runs=build_skip_reason is None, - quant=cfg.quant, - optimize=cfg.optimize, - analyze=cfg.analyze, - max_optim_iterations=cfg.max_optim_iterations, - reason=build_skip_reason, - rebuild_hint=("--no-skip-build" if build_skip_reason == "pre-built ONNX inputs" else None), - ) - if build_flags_warning: - logger.warning(build_flags_warning) - - cache_flags_warning = cli_utils.ignored_cache_flags_warning( - build_runs=build_skip_reason is None, - use_cache=cfg.use_cache, - rebuild=cfg.rebuild, - use_cache_was_set=cli_utils.is_cli_provided(ctx, "use_cache"), - rebuild_was_set=cli_utils.is_cli_provided(ctx, "rebuild"), - reason=build_skip_reason, - ) - if cache_flags_warning: - logger.warning(cache_flags_warning) - - logger.debug("Effective eval config: %s", cfg.to_dict()) - json_mode = output_format == "json" # ── 3. Evaluate ── try: + _warn_ignored_model_build_controls(ctx, cfg, config_fields) + logger.debug("Effective eval config: %s", cfg.to_dict()) result = evaluate(cfg) _write_and_display(result, cfg.output_path, json_mode=json_mode) except Exception as e: @@ -383,12 +356,15 @@ def _build_eval_config( config_file: Path | None, column: tuple[str, ...], label_mapping_path: Path | None, -) -> WinMLEvaluationConfig: +) -> tuple[WinMLEvaluationConfig, set[str]]: """Build a WinMLEvaluationConfig with precedence: defaults ← config file ← CLI. Reads raw JSON for config-file values so only explicitly-present keys are applied (avoids overriding with dataclass defaults). Uses ``collect_cli_overrides`` for automatic CLI-to-field mapping. + + Returns the resolved config and the field names explicitly present in the + config file. """ from ..eval import DatasetConfig, WinMLEvaluationConfig from ..utils.config_utils import merge_config @@ -402,6 +378,7 @@ def _build_eval_config( eval_kwargs = cli_utils.collect_cli_overrides(ctx, WinMLEvaluationConfig) dataset_kwargs = cli_utils.collect_cli_overrides(ctx, DatasetConfig) cfg = WinMLEvaluationConfig(dataset=DatasetConfig(**dataset_kwargs), **eval_kwargs) + config_fields: set[str] = set() # ── Config file layer (only explicitly-present keys) ── if config_file is not None: @@ -420,6 +397,7 @@ def _build_eval_config( # Eval section overrides loader/compile fallbacks eval_data = raw.get("eval") if eval_data: + config_fields.update(eval_data) cfg = merge_config(cfg, eval_data) # ── CLI layer (highest priority, auto-mapped via metadata) ── @@ -448,26 +426,72 @@ def _build_eval_config( if overrides: cfg = merge_config(cfg, overrides) - return cfg + return cfg, config_fields def _model_build_skip_reason(cfg: WinMLEvaluationConfig) -> str | None: """Describe why eval will not build model artifacts, if applicable.""" - if cfg.reference_path is not None: + from ..eval.evaluate import _ModelLoaderKind, _select_model_loader + + loader = _select_model_loader(cfg) + if loader is _ModelLoaderKind.DIRECT_ONNX_COMPARE: return "two-ONNX comparisons" - if cfg.model_path is None: - return None - if isinstance(cfg.model_path, str): - model_path = Path(cfg.model_path).expanduser() - if model_path.is_dir() and (model_path / "genai_config.json").is_file(): - return "pre-built GenAI bundles" - if isinstance(cfg.model_path, dict) and cfg.task == "mask-generation": + if loader is _ModelLoaderKind.EVALUATOR_MANAGED: return "evaluator-managed composite inputs" - if cfg.skip_build: + if loader is _ModelLoaderKind.ONNX and cfg.skip_build: return "pre-built ONNX inputs" return None +def _warn_ignored_model_build_controls( + ctx: click.Context, + cfg: WinMLEvaluationConfig, + config_fields: set[str], +) -> None: + """Warn when explicit model-build controls cannot affect the selected loader.""" + _resolve_model_loader_task(cfg) + build_skip_reason = _model_build_skip_reason(cfg) + + build_flags_warning = cli_utils.ignored_build_flags_warning( + build_runs=build_skip_reason is None, + quant=cfg.quant, + optimize=cfg.optimize, + analyze=cfg.analyze, + max_optim_iterations=cfg.max_optim_iterations, + reason=build_skip_reason, + rebuild_hint=("--no-skip-build" if build_skip_reason == "pre-built ONNX inputs" else None), + ) + if build_flags_warning: + logger.warning(build_flags_warning) + + cache_flags_warning = cli_utils.ignored_cache_flags_warning( + build_runs=build_skip_reason is None, + use_cache=cfg.use_cache, + rebuild=cfg.rebuild, + use_cache_was_set=cli_utils.is_cli_provided(ctx, "use_cache"), + rebuild_was_set=cli_utils.is_cli_provided(ctx, "rebuild"), + use_cache_source=("--config" if "use_cache" in config_fields else None), + rebuild_source=("--config" if "rebuild" in config_fields else None), + reason=build_skip_reason, + ) + if cache_flags_warning: + logger.warning(cache_flags_warning) + + +def _resolve_model_loader_task(cfg: WinMLEvaluationConfig) -> None: + """Resolve an omitted task when it can change the selected model loader.""" + if cfg.task is not None or cfg.reference_path is not None: + return + if not isinstance(cfg.model_path, dict) and not ( + isinstance(cfg.model_path, str) and Path(cfg.model_path).is_dir() + ): + return + + from ..eval.evaluate import _infer_task + + cfg.task = _infer_task(cfg) + + def _resolve_model( cfg: WinMLEvaluationConfig, model: tuple[str, ...], diff --git a/src/winml/modelkit/eval/config.py b/src/winml/modelkit/eval/config.py index 3d6e70e39..d8b4e8d66 100644 --- a/src/winml/modelkit/eval/config.py +++ b/src/winml/modelkit/eval/config.py @@ -216,10 +216,8 @@ def to_dict(self) -> dict: if self.mode != "onnx": result["mode"] = self.mode result["skip_build"] = self.skip_build - if not self.use_cache: - result["use_cache"] = self.use_cache - if self.rebuild: - result["rebuild"] = self.rebuild + result["use_cache"] = self.use_cache + result["rebuild"] = self.rebuild return result @classmethod diff --git a/src/winml/modelkit/eval/evaluate.py b/src/winml/modelkit/eval/evaluate.py index 0ab023e9c..3b11238fa 100644 --- a/src/winml/modelkit/eval/evaluate.py +++ b/src/winml/modelkit/eval/evaluate.py @@ -11,6 +11,7 @@ import logging from copy import deepcopy from dataclasses import dataclass, field, replace +from enum import Enum, auto from typing import TYPE_CHECKING, Any, cast from rich.console import Console @@ -28,6 +29,28 @@ logger = logging.getLogger(__name__) + +class _ModelLoaderKind(Enum): + GENAI = auto() + DIRECT_ONNX_COMPARE = auto() + EVALUATOR_MANAGED = auto() + ONNX = auto() + PRETRAINED = auto() + + +def _select_model_loader(config: WinMLEvaluationConfig) -> _ModelLoaderKind: + """Select the model-loading path shared by loading and CLI diagnostics.""" + if config.task == "text-generation": + return _ModelLoaderKind.GENAI + if config.mode == "compare" and config.reference_path is not None: + return _ModelLoaderKind.DIRECT_ONNX_COMPARE + if isinstance(config.model_path, dict) and config.task == "mask-generation": + return _ModelLoaderKind.EVALUATOR_MANAGED + if config.model_path is not None: + return _ModelLoaderKind.ONNX + return _ModelLoaderKind.PRETRAINED + + # Map task -> "module_path:ClassName"; modules are imported lazily by # get_evaluator_class() to improve command latency. # Keep the key/value-per-line layout: collapsing each entry onto one line (the @@ -256,12 +279,13 @@ def _load_model( from ..session import EPDeviceTarget, WinMLEPRegistry, resolve_device from ..utils import cli as cli_utils - if config.task == "text-generation": + loader = _select_model_loader(config) + if loader is _ModelLoaderKind.GENAI: return _load_genai_causal_lm(config) # Two-ONNX compare: the evaluator builds both raw ORT sessions directly from # config.model_path / config.reference_path — no WinMLAutoModel / HF config. - if config.mode == "compare" and config.reference_path is not None: + if loader is _ModelLoaderKind.DIRECT_ONNX_COMPARE: return None quant_override: Any = None @@ -284,7 +308,7 @@ def _load_model( if config.model_id is None: raise ValueError("model_id is required.") - if isinstance(config.model_path, dict) and config.task == "mask-generation": + if loader is _ModelLoaderKind.EVALUATOR_MANAGED: # Evaluator-driven session loading; skip WinMLAutoModel entirely. return None @@ -298,7 +322,7 @@ def _load_model( from onnxruntime.capi.onnxruntime_pybind11_state import RuntimeException try: - if config.model_path is not None: + if loader is _ModelLoaderKind.ONNX: # Pre-built ONNX: precision is already baked into the model and is # ignored here (mirrors winml perf's ONNX path). from transformers import AutoConfig @@ -424,19 +448,7 @@ def _resolve_task(config: WinMLEvaluationConfig) -> str: console = Console() console.print("[bold]Resolving task...[/bold]") - if config.task is not None: - task = config.task - else: - if config.model_id is None: - raise ValueError("Cannot infer task without model_id. Provide --task.") - - from transformers import AutoConfig - - from ..loader import load_hf_config - from ..loader.resolution import resolve_task - - hf_config = load_hf_config(AutoConfig, config.model_id) - task = resolve_task(hf_config).task + task = config.task if config.task is not None else _infer_task(config) console.print(f"[dim]Use[/dim] {task} [dim]to evaluate[/dim]") @@ -446,6 +458,20 @@ def _resolve_task(config: WinMLEvaluationConfig) -> str: return task +def _infer_task(config: WinMLEvaluationConfig) -> str: + """Infer the evaluation task from the model's Hugging Face config.""" + if config.model_id is None: + raise ValueError("Cannot infer task without model_id. Provide --task.") + + from transformers import AutoConfig + + from ..loader import load_hf_config + from ..loader.resolution import resolve_task + + hf_config = load_hf_config(AutoConfig, config.model_id) + return resolve_task(hf_config).task + + def evaluate(config: WinMLEvaluationConfig) -> EvalResult: """Run model evaluation. diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index 7d9586ca7..ad816d437 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -1093,22 +1093,22 @@ def ignored_cache_flags_warning( rebuild: bool = False, use_cache_was_set: bool = False, rebuild_was_set: bool = False, + use_cache_source: str | None = None, + rebuild_source: str | None = None, reason: str | None = None, ) -> str | None: """Build a warning for explicit cache controls when no model build runs.""" if build_runs: return None - ignored = [ - flag - for flag, was_set in ( - ( - "--use-cache" if use_cache else "--no-use-cache", - use_cache_was_set or not use_cache, - ), - ("--rebuild" if rebuild else "--no-rebuild", rebuild_was_set or rebuild), - ) - if was_set - ] + ignored: list[str] = [] + if use_cache_was_set: + ignored.append("--use-cache" if use_cache else "--no-use-cache") + elif use_cache_source is not None: + ignored.append(f"use_cache={str(use_cache).lower()} from {use_cache_source}") + if rebuild_was_set: + ignored.append("--rebuild" if rebuild else "--no-rebuild") + elif rebuild_source is not None: + ignored.append(f"rebuild={str(rebuild).lower()} from {rebuild_source}") if not ignored: return None return f"{', '.join(ignored)} ignored for {reason or 'this input'} (no build runs)." diff --git a/tests/unit/commands/test_eval.py b/tests/unit/commands/test_eval.py index 58ca09b93..d64f35f54 100644 --- a/tests/unit/commands/test_eval.py +++ b/tests/unit/commands/test_eval.py @@ -692,6 +692,52 @@ def _fake_evaluate(cfg): assert cfg.use_cache is False assert cfg.rebuild is True + def test_cli_cache_controls_override_config_file(self, runner: CliRunner, tmp_path): + from winml.modelkit.commands.eval import eval as eval_cmd + + config_path = tmp_path / "eval_config.json" + config_path.write_text( + json.dumps({"eval": {"use_cache": False, "rebuild": True}}), + encoding="utf-8", + ) + captured_cfg = {} + + def _fake_evaluate(cfg): + captured_cfg["cfg"] = cfg + return object() + + with ( + patch("winml.modelkit.eval.evaluate", side_effect=_fake_evaluate), + patch("winml.modelkit.commands.eval._resolve_device", return_value=None), + patch("winml.modelkit.commands.eval._write_and_display", return_value=None), + ): + result = runner.invoke( + eval_cmd, + [ + "--config", + str(config_path), + "-m", + "microsoft/resnet-50", + "--use-cache", + "--no-rebuild", + ], + obj={"debug": False}, + ) + + assert result.exit_code == 0, result.output + cfg = captured_cfg["cfg"] + assert cfg.use_cache is True + assert cfg.rebuild is False + + @pytest.mark.parametrize("field_name", ["use_cache", "rebuild"]) + def test_cache_click_default_matches_config_default(self, field_name: str): + from winml.modelkit.commands.eval import eval as eval_cmd + from winml.modelkit.eval import WinMLEvaluationConfig + + parameter = next(param for param in eval_cmd.params if param.name == field_name) + + assert parameter.default == getattr(WinMLEvaluationConfig(), field_name) + def test_cli_default_device_propagates_when_not_explicitly_passed( self, runner: CliRunner, @@ -1281,6 +1327,40 @@ def test_defaults_do_not_warn(self, runner: CliRunner, onnx_file, caplog): for record in caplog.records ) + def test_config_values_name_their_source( + self, + runner: CliRunner, + onnx_file, + tmp_path, + caplog, + ): + import logging as _logging + + config_path = tmp_path / "eval_config.json" + config_path.write_text( + json.dumps({"eval": {"use_cache": False, "rebuild": True}}), + encoding="utf-8", + ) + + with caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"): + result = self._run( + runner, + [ + "--config", + str(config_path), + "-m", + str(onnx_file), + "--model-id", + "some/model", + ], + ) + + assert result.exit_code == 0, result.output + messages = [record.getMessage() for record in caplog.records] + assert any("use_cache=false from --config" in message for message in messages) + assert any("rebuild=true from --config" in message for message in messages) + assert not any("--no-use-cache ignored" in message for message in messages) + def test_two_onnx_comparison_warns_even_with_build_enabled( self, runner: CliRunner, @@ -1315,7 +1395,7 @@ def test_two_onnx_comparison_warns_even_with_build_enabled( for record in caplog.records ) - def test_genai_bundle_warns_even_with_build_enabled(self, runner: CliRunner, tmp_path, caplog): + def test_genai_bundle_does_not_claim_no_build_runs(self, runner: CliRunner, tmp_path, caplog): import logging as _logging bundle = tmp_path / "genai-model" @@ -1336,10 +1416,39 @@ def test_genai_bundle_warns_even_with_build_enabled(self, runner: CliRunner, tmp ) assert result.exit_code == 0, result.output - assert any( - "--no-use-cache ignored for pre-built GenAI bundles" in record.getMessage() - for record in caplog.records - ) + assert not any("--no-use-cache ignored" in record.getMessage() for record in caplog.records) + + def test_inferred_genai_task_does_not_claim_no_build_runs( + self, + runner: CliRunner, + tmp_path, + caplog, + ): + import importlib + import logging as _logging + + eval_module = importlib.import_module("winml.modelkit.eval.evaluate") + bundle = tmp_path / "genai-model" + bundle.mkdir() + (bundle / "genai_config.json").write_text("{}", encoding="utf-8") + + with ( + patch.object(eval_module, "_infer_task", return_value="text-generation"), + caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"), + ): + result = self._run( + runner, + [ + "-m", + str(bundle), + "--model-id", + "some/model", + "--no-use-cache", + ], + ) + + assert result.exit_code == 0, result.output + assert not any("--no-use-cache ignored" in record.getMessage() for record in caplog.records) def test_evaluator_managed_composite_warns_even_with_build_enabled( self, @@ -1370,6 +1479,38 @@ def test_evaluator_managed_composite_warns_even_with_build_enabled( for record in caplog.records ) + def test_inferred_evaluator_managed_task_warns( + self, + runner: CliRunner, + onnx_file, + caplog, + ): + import importlib + import logging as _logging + + eval_module = importlib.import_module("winml.modelkit.eval.evaluate") + with ( + patch.object(eval_module, "_infer_task", return_value="mask-generation"), + caplog.at_level(_logging.WARNING, logger="winml.modelkit.commands.eval"), + ): + result = self._run( + runner, + [ + "-m", + f"encoder={onnx_file}", + "--model-id", + "some/model", + "--no-skip-build", + "--rebuild", + ], + ) + + assert result.exit_code == 0, result.output + assert any( + "--rebuild ignored for evaluator-managed composite inputs" in record.getMessage() + for record in caplog.records + ) + # --------------------------------------------------------------------------- # --format json diff --git a/tests/unit/eval/test_eval.py b/tests/unit/eval/test_eval.py index 80dd3f6c9..fb5babe8c 100644 --- a/tests/unit/eval/test_eval.py +++ b/tests/unit/eval/test_eval.py @@ -28,7 +28,7 @@ def test_relies_on_model_framework_inference(self) -> None: evaluator.model = MagicMock() sentinel = MagicMock() - with patch("transformers.pipelines.pipeline", return_value=sentinel) as mock_pipeline: + with patch("transformers.pipeline", return_value=sentinel) as mock_pipeline: assert evaluator.prepare_pipeline() is sentinel assert "framework" not in mock_pipeline.call_args.kwargs @@ -125,11 +125,11 @@ def test_config_roundtrip_preserves_cache_controls(self): assert restored.use_cache is False assert restored.rebuild is True - def test_default_cache_controls_are_omitted(self): + def test_default_cache_controls_are_serialized(self): serialized = WinMLEvaluationConfig(model_id="test/model").to_dict() - assert "use_cache" not in serialized - assert "rebuild" not in serialized + assert serialized["use_cache"] is True + assert serialized["rebuild"] is False def test_reference_path_default_is_none(self): """reference_path defaults to None and is omitted from to_dict.""" diff --git a/tests/unit/utils/test_cli.py b/tests/unit/utils/test_cli.py index 7b9da8bf1..2750424f6 100644 --- a/tests/unit/utils/test_cli.py +++ b/tests/unit/utils/test_cli.py @@ -470,17 +470,20 @@ def test_names_explicit_cache_controls(self) -> None: assert "--rebuild" in msg assert "pre-built ONNX inputs" in msg - def test_nondefault_config_values_count_as_explicit(self) -> None: + def test_config_values_name_their_source(self) -> None: msg = ignored_cache_flags_warning( build_runs=False, use_cache=False, rebuild=True, + use_cache_source="--config", + rebuild_source="--config", reason="pre-built ONNX inputs", ) assert msg is not None - assert "--no-use-cache" in msg - assert "--rebuild" in msg + assert "use_cache=false from --config" in msg + assert "rebuild=true from --config" in msg + assert "--no-use-cache" not in msg class TestOverwriteOption: From b95676f59e9e584bb073dcf405cc539308d41a35 Mon Sep 17 00:00:00 2001 From: Hualiang Xie Date: Wed, 5 Aug 2026 11:18:49 +0800 Subject: [PATCH 5/5] fix(eval): warn for ignored GenAI controls Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/commands/eval.md | 4 ++- src/winml/modelkit/commands/eval.py | 42 +++++++++++++++++++++-------- src/winml/modelkit/utils/cli.py | 9 +++++-- tests/unit/commands/test_eval.py | 30 ++++++++++++++++++--- tests/unit/utils/test_cli.py | 11 ++++++++ 5 files changed, 78 insertions(+), 18 deletions(-) diff --git a/docs/commands/eval.md b/docs/commands/eval.md index ccdd31253..7e5c80ab9 100644 --- a/docs/commands/eval.md +++ b/docs/commands/eval.md @@ -151,7 +151,9 @@ For a pre-built ONNX input, cache controls apply only when `--no-skip-build` is set. Cache controls are ignored when no model build runs, including two-ONNX comparisons; the CLI warns when an explicit cache control has no effect. GenAI's runtime `_compiled/` artifacts are a separate cache and -are not currently governed by these model build cache controls. +are not currently governed by these model build cache controls. Explicit build +or cache controls on a GenAI bundle produce a warning that distinguishes the +model-build pipeline from the runtime compilation cache. ## Common pitfalls diff --git a/src/winml/modelkit/commands/eval.py b/src/winml/modelkit/commands/eval.py index 11183894f..7c06dd798 100644 --- a/src/winml/modelkit/commands/eval.py +++ b/src/winml/modelkit/commands/eval.py @@ -9,6 +9,7 @@ import json import logging +from dataclasses import dataclass from pathlib import Path from typing import TYPE_CHECKING @@ -429,17 +430,32 @@ def _build_eval_config( return cfg, config_fields -def _model_build_skip_reason(cfg: WinMLEvaluationConfig) -> str | None: - """Describe why eval will not build model artifacts, if applicable.""" +@dataclass(frozen=True) +class _ModelBuildBypass: + reason: str + build_explanation: str = "no build runs" + cache_explanation: str = "no build runs" + + +def _model_build_bypass(cfg: WinMLEvaluationConfig) -> _ModelBuildBypass | None: + """Describe a selected loader that bypasses the model-build pipeline.""" from ..eval.evaluate import _ModelLoaderKind, _select_model_loader loader = _select_model_loader(cfg) + if loader is _ModelLoaderKind.GENAI: + return _ModelBuildBypass( + reason="GenAI bundles", + build_explanation="no model build pipeline runs", + cache_explanation=( + "model build cache controls do not govern the GenAI runtime _compiled/ cache" + ), + ) if loader is _ModelLoaderKind.DIRECT_ONNX_COMPARE: - return "two-ONNX comparisons" + return _ModelBuildBypass("two-ONNX comparisons") if loader is _ModelLoaderKind.EVALUATOR_MANAGED: - return "evaluator-managed composite inputs" + return _ModelBuildBypass("evaluator-managed composite inputs") if loader is _ModelLoaderKind.ONNX and cfg.skip_build: - return "pre-built ONNX inputs" + return _ModelBuildBypass("pre-built ONNX inputs") return None @@ -450,29 +466,33 @@ def _warn_ignored_model_build_controls( ) -> None: """Warn when explicit model-build controls cannot affect the selected loader.""" _resolve_model_loader_task(cfg) - build_skip_reason = _model_build_skip_reason(cfg) + bypass = _model_build_bypass(cfg) + build_runs = bypass is None + reason = bypass.reason if bypass is not None else None build_flags_warning = cli_utils.ignored_build_flags_warning( - build_runs=build_skip_reason is None, + build_runs=build_runs, quant=cfg.quant, optimize=cfg.optimize, analyze=cfg.analyze, max_optim_iterations=cfg.max_optim_iterations, - reason=build_skip_reason, - rebuild_hint=("--no-skip-build" if build_skip_reason == "pre-built ONNX inputs" else None), + reason=reason, + rebuild_hint=("--no-skip-build" if reason == "pre-built ONNX inputs" else None), + explanation=bypass.build_explanation if bypass is not None else None, ) if build_flags_warning: logger.warning(build_flags_warning) cache_flags_warning = cli_utils.ignored_cache_flags_warning( - build_runs=build_skip_reason is None, + build_runs=build_runs, use_cache=cfg.use_cache, rebuild=cfg.rebuild, use_cache_was_set=cli_utils.is_cli_provided(ctx, "use_cache"), rebuild_was_set=cli_utils.is_cli_provided(ctx, "rebuild"), use_cache_source=("--config" if "use_cache" in config_fields else None), rebuild_source=("--config" if "rebuild" in config_fields else None), - reason=build_skip_reason, + reason=reason, + explanation=bypass.cache_explanation if bypass is not None else None, ) if cache_flags_warning: logger.warning(cache_flags_warning) diff --git a/src/winml/modelkit/utils/cli.py b/src/winml/modelkit/utils/cli.py index ad816d437..7fcfbb939 100644 --- a/src/winml/modelkit/utils/cli.py +++ b/src/winml/modelkit/utils/cli.py @@ -1051,6 +1051,7 @@ def ignored_build_flags_warning( max_optim_iterations: int | None = None, reason: str | None = None, rebuild_hint: str | None = None, + explanation: str | None = None, ) -> str | None: """Build a warning for build-pipeline flags when no model build runs. @@ -1064,6 +1065,7 @@ def ignored_build_flags_warning( max_optim_iterations: Explicit value, or ``None`` when left at default. reason: Description of the path that bypasses the build. rebuild_hint: Optional flag that enables a build for this path. + explanation: Optional explanation of why the controls have no effect. Returns: Warning message, or ``None`` if no ignored flags apply. @@ -1083,7 +1085,8 @@ def ignored_build_flags_warning( if not ignored: return None hint = f"; pass {rebuild_hint} to rebuild" if rebuild_hint else "" - return f"{', '.join(ignored)} ignored for {reason or 'this input'} (no build runs{hint})." + detail = explanation or "no build runs" + return f"{', '.join(ignored)} ignored for {reason or 'this input'} ({detail}{hint})." def ignored_cache_flags_warning( @@ -1096,6 +1099,7 @@ def ignored_cache_flags_warning( use_cache_source: str | None = None, rebuild_source: str | None = None, reason: str | None = None, + explanation: str | None = None, ) -> str | None: """Build a warning for explicit cache controls when no model build runs.""" if build_runs: @@ -1111,7 +1115,8 @@ def ignored_cache_flags_warning( ignored.append(f"rebuild={str(rebuild).lower()} from {rebuild_source}") if not ignored: return None - return f"{', '.join(ignored)} ignored for {reason or 'this input'} (no build runs)." + detail = explanation or "no build runs" + return f"{', '.join(ignored)} ignored for {reason or 'this input'} ({detail})." def allow_unsupported_nodes_option(optional_message: str | None = None) -> Callable[[F], F]: diff --git a/tests/unit/commands/test_eval.py b/tests/unit/commands/test_eval.py index d64f35f54..509594389 100644 --- a/tests/unit/commands/test_eval.py +++ b/tests/unit/commands/test_eval.py @@ -1395,7 +1395,12 @@ def test_two_onnx_comparison_warns_even_with_build_enabled( for record in caplog.records ) - def test_genai_bundle_does_not_claim_no_build_runs(self, runner: CliRunner, tmp_path, caplog): + def test_genai_bundle_warns_with_runtime_cache_wording( + self, + runner: CliRunner, + tmp_path, + caplog, + ): import logging as _logging bundle = tmp_path / "genai-model" @@ -1412,13 +1417,26 @@ def test_genai_bundle_does_not_claim_no_build_runs(self, runner: CliRunner, tmp_ "text-generation", "--no-skip-build", "--no-use-cache", + "--rebuild", + "--no-quant", + "--no-optimize", ], ) assert result.exit_code == 0, result.output - assert not any("--no-use-cache ignored" in record.getMessage() for record in caplog.records) + messages = [record.getMessage() for record in caplog.records] + assert any( + "--no-use-cache, --rebuild ignored for GenAI bundles" in message + and "do not govern the GenAI runtime _compiled/ cache" in message + for message in messages + ) + assert any( + "--no-quant, --no-optimize ignored for GenAI bundles" in message + and "no model build pipeline runs" in message + for message in messages + ) - def test_inferred_genai_task_does_not_claim_no_build_runs( + def test_inferred_genai_task_warns_with_runtime_cache_wording( self, runner: CliRunner, tmp_path, @@ -1448,7 +1466,11 @@ def test_inferred_genai_task_does_not_claim_no_build_runs( ) assert result.exit_code == 0, result.output - assert not any("--no-use-cache ignored" in record.getMessage() for record in caplog.records) + assert any( + "--no-use-cache ignored for GenAI bundles" in record.getMessage() + and "do not govern the GenAI runtime _compiled/ cache" in record.getMessage() + for record in caplog.records + ) def test_evaluator_managed_composite_warns_even_with_build_enabled( self, diff --git a/tests/unit/utils/test_cli.py b/tests/unit/utils/test_cli.py index 2750424f6..51b9c5732 100644 --- a/tests/unit/utils/test_cli.py +++ b/tests/unit/utils/test_cli.py @@ -485,6 +485,17 @@ def test_config_values_name_their_source(self) -> None: assert "rebuild=true from --config" in msg assert "--no-use-cache" not in msg + def test_uses_custom_explanation(self) -> None: + msg = ignored_cache_flags_warning( + build_runs=False, + rebuild=True, + rebuild_was_set=True, + reason="GenAI bundles", + explanation="does not control the runtime cache", + ) + + assert msg == "--rebuild ignored for GenAI bundles (does not control the runtime cache)." + class TestOverwriteOption: """Tests for the shared overwrite_option() decorator."""