Skip to content

feat(eval): add model cache controls - #1270

Open
xieofxie wants to merge 5 commits into
mainfrom
hualxie/eval-cache-flags
Open

feat(eval): add model cache controls#1270
xieofxie wants to merge 5 commits into
mainfrom
hualxie/eval-cache-flags

Conversation

@xieofxie

@xieofxie xieofxie commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add canonical cache and rebuild controls to winml eval with cache reuse enabled by default
  • carry cache policy through eval config and both Hugging Face and ONNX model loading
  • derive ignored-control warnings from the same loader selection used by evaluation and identify config-file values by their actual source
  • serialize resolved cache controls explicitly and cover Click defaults plus CLI-over-config precedence
  • document the distinction between model build caching and GenAI runtime compilation caching

Review notes

Stack

Shared cache primitives landed in #1269.

Hualiang Xie and others added 2 commits August 3, 2026 13:39
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@zhenchaoni zhenchaoni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the cache-control plumbing end to end and ran the affected suites on this branch (tests/unit/commands/test_eval.py, tests/unit/eval/test_eval.py, tests/unit/utils/test_cli.py, tests/unit/commands/test_perf_cli.py — 342 passed).

The core wiring is sound and the precedence outcome is currently correct. My main concerns are:

  1. The GenAI path is classified as "no build runs", but it does build and cache (bundle_dir/_compiled/). Cache controls are silently dropped there and the user is told they were ignored. This one looks like a real bug.
  2. _model_build_skip_reason re-implements _load_model's dispatch with different predicates, so the two can disagree.
  3. ignored_build_flags_warning changed a keyword's name and inverted its meaning in one step — worth a second pair of eyes on the two call sites.
  4. The stated precedence chain has no "CLI default" layer — it works today only because the Click defaults happen to equal the dataclass defaults, and nothing enforces that.

Details inline.

Comment thread src/winml/modelkit/commands/eval.py Outdated
Comment thread src/winml/modelkit/commands/eval.py Outdated
Comment thread src/winml/modelkit/commands/eval.py Outdated
Comment thread src/winml/modelkit/utils/cli.py
Comment thread src/winml/modelkit/utils/cli.py Outdated
Comment thread src/winml/modelkit/commands/eval.py
Comment thread tests/unit/commands/test_eval.py
Comment thread src/winml/modelkit/eval/config.py Outdated
Comment thread src/winml/modelkit/commands/perf.py
Comment thread tests/unit/eval/test_eval.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants