Skip to content

fix(eval): resolve VitPose image processor metadata - #1310

Draft
ssss141414 wants to merge 2 commits into
microsoft:mainfrom
ssss141414:vitpose-eval-processor-fallback
Draft

ssss141414 wants to merge 2 commits into
microsoft:mainfrom
ssss141414:vitpose-eval-processor-fallback

Conversation

@ssss141414

@ssss141414 ssss141414 commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

This L2 contribution enables nielsr/vitpose-base-simple keypoint-detection evaluation by resolving the VitPose image-processor metadata alias through a class-wide, metadata-gated fallback. It ships evaluator code and regression tests while the existing CPU fp32/fp16 recipes remain byte-for-byte unchanged. Full CPU fp32/fp16 coverage reached L2, and the committed Goal reached L3 PASS through one bounded fp32 CPU COCO functional smoke.

Model metadata

What the model does

nielsr/vitpose-base-simple is a top-down human pose model that accepts an RGB image plus person bounding boxes, transforms each person crop into a 256 x 192 tensor, and emits 17 heatmaps whose peaks represent body-keypoint locations.

  • Evidence: the pinned checkpoint at revision 2d440ccdd65125f8fdf61110d5b9e2d3ded4a964 declares pipeline_tag=keypoint-detection, model_type=vitpose, architectures=[ViTPoseForPoseEstimation], image_size=[256,192], and 17 labels; Transformers v5.14.1 VitPoseImageProcessor uses COCO-format boxes for affine person crops, and ViTPoseForPoseEstimation returns per-keypoint heatmaps. Confidence: verified.

Primary user stories

  1. A user supplies an RGB image and one or more detected-person bounding boxes to obtain 17 body-keypoint coordinates and confidence scores for top-down pose estimation. Evidence: Transformers v5.14.1 VitPoseImageProcessor preprocessing/postprocessing contracts and the WinML keypoint evaluator's image, bbox, keypoints, and area fields. Confidence: verified.
  2. An application supplies person detections and uses the resulting keypoints for downstream human-pose analysis. Evidence: the top-down processor and evaluator data flow requires externally supplied person boxes. Confidence: inferred.

Supported tasks

  • keypoint-detection across checkpoint, Transformers, Optimum ONNX, and WinML surfaces. Evidence: the pinned checkpoint task and architecture; Transformers v5.14.1 ViTPoseForPoseEstimation and VitPoseImageProcessor; Optimum's vitpose/keypoint-detection registration to VitPoseOnnxConfig; and WinML's VitPoseIOConfig, VitPoseForPoseEstimation, and merged CPU fp32/fp16 recipes. Confidence: verified.

Model architecture

ViTPoseForPoseEstimation
├── VitPose backbone (256 x 192 RGB; hidden width 768)
│   ├── Patch embedding (16 x 16 patches)
│   ├── Transformer encoder layer x 12
│   │   ├── Multi-head self-attention (12 heads)
│   │   ├── MLP (768 -> 3072 -> 768, GELU)
│   │   └── LayerNorm + residual paths
│   └── Final stage12 feature sequence
├── Token-to-spatial reshape (16 x 12 feature map)
└── VitPoseSimpleDecoder
    ├── ReLU + 4x bilinear upsampling
    └── 3 x 3 convolution -> 17 heatmaps [B,17,64,48]
  • Source/confidence: pinned checkpoint config and Transformers v5.14.1 ViTPoseForPoseEstimation/VitPoseSimpleDecoder source (verified).

Validation and support evidence

Baseline

The current pinned main is 2b9ec0e9e57bba8003d25eaff85f8c7c9e30d75a with WinML 0.3.0; the prior accepted base was 48bfd0f91478a8e281b205b0449222024729f3ea. Baseline provisioning remains the historical FAIL-UPSTREAM measured at 24cccfb23d6e3c3b93ae9bb94399c326cee8ecea: canonical-lock synchronization failed on TLS transport, offline sync lacked locked torch, and product execution never began. Therefore no fresh current-main baseline config/build/perf/Eval result, build time, node count, latency, or metric is claimed.

No baseline winml config command executed and no baseline recipe was generated. Static installed-source evidence identifies the keypoint-detection task, ViTPoseForPoseEstimation, Optimum's vitpose/keypoint-detection registration, and WinML's override with DummyVisionInputGenerator; these are source-inspection facts, not an executed auto-config selection. No registration was found for vit-pose or vit_pose. A fresh dynamic probe was blocked by an internally incompatible current lock and is not represented as fresh evidence.

This is a PARTIAL-RERUN. Between the prior and current bases, only PR #1312 changed winml sys source, documentation, and tests; dependency impact analysis found no reachability into config, build, perf, Eval, analyze, loader/export, model, quantization, or optimization paths. Quality gates reran at shipped head 1207965854b5eeb3dec055a7cfa1f08db30fa6f9; config/build/perf/parity/Eval/analyze remain reused from original execution SHA b9f924e3b23bd8caf85339faa3853eeb20df2f17 under no-impact attestation and exact range-diff, stable-patch, owned-blob, and two-file diff equivalence. The current runtime patch commit 88bd424bbf0e415fa05c25c3cf9ad369277a06d5 is exact-equivalent to the originally measured runtime patch.

Goal

  • Effort: L2.
  • Committed Goal ceiling: L3.
  • Shipped Outcome: L2.
  • Goal success definition: CPU fp32/fp16 L0-L2 plus one real, one-row COCO 2017 fp32 CPU functional smoke proving operability only.
  • Verdict: L3 PASS. The ceiling was not downgraded or re-issued.

Outcome

Outcome L2 shipped at head 1207965854b5eeb3dec055a7cfa1f08db30fa6f9; the highest Goal verdict is L3 PASS. Coverage is full for both required CPU tuples, with no deferred tuples. The product diff contains only src/winml/modelkit/eval/keypoint_detection_evaluator.py and tests/unit/eval/test_keypoint_detection_evaluator.py; the existing merged fp32/fp16 recipes remain byte-for-byte unchanged, and the production recipe README is untouched.

Fresh exact-head quality evidence passed insert-license, Ruff, and Mypy (436 source files); focused evaluator tests 16 passed; direct winml sys unit tests 11 passed; the direct sys-e2e probe reported 20 skipped because -m e2e is required and therefore is not executed e2e coverage; the commands partition reported 3618 passed/9 skipped; the remaining partition reported 871 passed/2 skipped/1 deselected. All nine exact-head GitHub checks completed successfully.

No new model finding was produced in this rerun. Prior Lane A entries vitpose-017 through vitpose-021 are pushed at commit a8d9a406bca1efaaf4dd4bace8d990334b5edc97 on remote ref refs/heads/ssss141414/pr1310-component-mapping-skill; this run was deduplicated and found no methodology friction. These entries are not claimed to be on main, and no Lane A PR URL is asserted.

Per-EP/device/precision results and Functional smoke Eval

All model-stage rows below are reused from original execution SHA b9f924e3b23bd8caf85339faa3853eeb20df2f17; they were not rerun at the shipped head.

Tier EP / Device Precision Verdict Mean p50 Throughput RAM Δ VRAM Δ
L0 CPUExecutionProvider / cpu fp32 PASS - - - - -
L0 CPUExecutionProvider / cpu fp16 PASS - - - - -
L1 CPUExecutionProvider / cpu fp32 PASS 87.770 ms 85.316 ms 11.39 samples/s +41.93 MB 0.0 MB
L1 CPUExecutionProvider / cpu fp16 PASS 111.774 ms 111.538 ms 8.95 samples/s +44.56 MB 0.0 MB

L0 fp32 structure: IR 8, opset 17, input pixel_values [1,3,256,192], output heatmaps [1,17,64,48], 359 nodes, 202 FLOAT initializers, and 343649280 external bytes. L0 fp16 structure: IR 8, opset 17, the same input/output shapes, 361 nodes, 201 FLOAT16 initializers, and 171824640 external bytes; realized fp16 precision was verified.

Tier Precision Verdict Cosine Max abs Mean abs
L2 fp32 PASS 0.9999999999985615 5.066394805908203e-07 1.2054166553809864e-08
L2 fp16 PASS 0.999999000534796 0.0005256682634353638 1.2196907846373506e-05

Parity used named input pixel_values [1,3,256,192] and output [1,17,64,48].

Functional smoke Eval (L3 PASS): CPUExecutionProvider / cpu / fp32 on the official COCO 2017 person-keypoints validation release (person_keypoints_val2017, validation), reduced to fixed image 29397 and non-crowd annotation 449985. One image was selected and processed with one person, one affine crop, one model invocation, one prediction, 17 keypoints, and one metric accumulation; schema, label semantics, and prediction semantics were verified. COCO keypoint OKS metrics were AP 0.9999999999999998, AP50 0.9999999999999999, AP75 0.9999999999999999, AP_large 0.9999999999999998, AP_medium -1.0, AR 1.0, AR50 1.0, and AR75 1.0.

The evaluator capability removed the former AutoImageProcessor VitPose alias blocker by using the explicit metadata-gated fallback. This smoke was executed at b9f924e3b23bd8caf85339faa3853eeb20df2f17 with runtime-equivalent commit e7c8bd2041ac1aa40329b866362ffa72933f6252; it was not rerun at shipped head 1207965854b5eeb3dec055a7cfa1f08db30fa6f9 and is reused through no-impact and exact-equivalence attestation. It proves end-to-end functional operability only, not representative accuracy or benchmark quality; no fp16 Eval or other-EP accuracy is inferred. PR #1299's historical mAP=0.8 citation lacks the required command/artifact/revision provenance and is not this accepted smoke result.

Delta

No baseline recipe was generated because baseline provisioning failed before product execution, so no baseline-generated-recipe or JSON-pointer comparison exists. The existing merged examples/recipes/nielsr_vitpose-base-simple/cpu/cpu/keypoint-detection_fp32_config.json and keypoint-detection_fp16_config.json files remain byte-for-byte unchanged, and this PR has no recipe delta. Original-execution recipe-free build/Eval evidence at b9f924e3b23bd8caf85339faa3853eeb20df2f17 verifies the code path independently under the stated no-impact and exact-equivalence reuse attestation. The production examples/recipes/README.md remains untouched.

The code delta is WinMLKeypointDetectionEvaluator.prepare_pipeline behavior plus its focused unit tests. No product content changed during the rebase: the two commits, resulting evaluator/test blobs, and exact two-file diff are equivalent; only commit and parent identities changed.

Bug fix explanation:

  1. Symptom and trigger: keypoint Eval setup failed before preprocessing/inference when AutoImageProcessor raised its exact unrecognized-image-processor ValueError for a VitPose-family checkpoint.
  2. Root cause: the shared evaluator used only the generic loader, while the checkpoint metadata identifies a processor family that the generic alias resolution did not recognize.
  3. Changed symbols and mechanism: WinMLKeypointDetectionEvaluator.prepare_pipeline retains AutoImageProcessor as primary, inspects model config only after the known error, and retries through VitPoseImageProcessor when model_type or architectures identifies VitPose; trust_remote_code is forwarded to both paths.
  4. General/data-driven rule: fallback selection is derived case-insensitively from the exact error class/text and model metadata, not from a checkpoint ID.
  5. Compatibility and blast radius: primary success is unchanged; non-VitPose metadata, unrelated ValueErrors, non-ValueError exceptions, missing model IDs, input-size overrides, crop fanout, box conversion, custom keypoint metadata, dataset indexing, prediction flattening, COCO metrics, and MIGraphX layout behavior remain unchanged. The intentional change is that metadata-identified VitPose checkpoints with this alias mismatch can proceed.
  6. Regression evidence: all 16 focused evaluator cases pass, covering primary success without fallback, both VitPose metadata gates, non-VitPose propagation, unrelated-error propagation, missing model ID, and trust_remote_code forwarding. Recipe-free fp32 build and the bounded COCO smoke establish reducibility and evaluator operability; existing recipes remain unchanged.

Analyze summary — component level and op level

Status is ANALYZE-PARTIAL-SUCCESS (exit code 1): a complete seven-record static rule roll-up was retained while unavailable plugin/rule groups remained explicit. This is static rule analysis, not runtime execution.

Component-level summary

Artifact Architecture coverage Mapping Confidence Actionable EP findings
fp32 VitPose backbone: 354 nodes; token-to-spatial reshape: 2; simple heatmap decoder: 3 359 mapped, 0 partial, 0 unmapped mapped Decoder Resize unknown on NvTensorRTRTX, OpenVINO, and QNN; no partial or unsupported operators reported.

Op-level summary

Artifact Graph Dominant ops EP roll-up
fp32 359 operators / 12 types Reshape 122; Gemm 72; Transpose 50; Add 26; LayerNormalization 25 NvTensorRTRTX/OpenVINO/QNN classify 11 types and leave one decoder Resize unknown; the other 358 nodes are covered.

CUDA, MIGraphX, TensorRT, and DML have no shipped rules, so all operators are unknown for those groups. Every static-analysis row has runtime_support=false; no runtime support or compatibility is inferred.

Reproduce commands

$OUT='temp/vitpose-base-simple-validation'
$DATASET='$OUT/coco-one-row-one-person'
winml build -m nielsr/vitpose-base-simple -o $OUT/cpu-fp32-recipe-free --ep cpu --device cpu --no-analyze --no-optimize --no-quant --no-compile --rebuild
winml build -c examples/recipes/nielsr_vitpose-base-simple/cpu/cpu/keypoint-detection_fp32_config.json -m nielsr/vitpose-base-simple -o $OUT/cpu-fp32
winml build -c examples/recipes/nielsr_vitpose-base-simple/cpu/cpu/keypoint-detection_fp16_config.json -m nielsr/vitpose-base-simple -o $OUT/cpu-fp16 --precision fp16
winml perf -m $OUT/cpu-fp32/model.onnx --device cpu --ep cpu --iterations 20 --warmup 5 --memory --output $OUT/perf-fp32.json --overwrite
winml perf -m $OUT/cpu-fp16/model.onnx --device cpu --ep cpu --iterations 20 --warmup 5 --memory --output $OUT/perf-fp16.json --overwrite
winml eval -m $OUT/cpu-fp32-recipe-free/model.onnx --model-id nielsr/vitpose-base-simple --task keypoint-detection --ep cpu --device cpu --dataset $DATASET --samples 1 --no-shuffle --skip-build --output $OUT/eval.json --overwrite
winml analyze --model $OUT/cpu-fp32/model.onnx --ep all --output $OUT/analyze-all.json

@ssss141414 ssss141414 added the model-scale-by-skill Model support PR created or maintained by the adding-model-support skill label Aug 14, 2026
ssss141414 added a commit that referenced this pull request Aug 14, 2026
## Summary

Adds CPU fp32/fp16 recipes for `nielsr/vitpose-base-simple` and fixes
recipe-free VitPose input generation.

Optimum's `VitPoseDummyInputGenerator` inherits a static Vision
generator that reloads preprocessors and indexes the result with `[-1]`.
Config-only resolution has no loaded processor, so that path raises
`IndexError`. A WinML `VitPoseIOConfig` now preserves Optimum's VitPose
I/O declarations and model patcher while replacing only its dummy
generator with the config-driven `DummyVisionInputGenerator`.

The workaround is contained in the existing VitPose model adapter.
Generic export I/O behavior is unchanged.

The generic Perf range-consumption change was split into #1309. The
VitPose evaluator processor fallback was split into #1310.

## Baseline and outcome

Without a recipe, current-main baseline export resolves
`VitPoseOnnxConfig` but fails while its static dummy generator loads an
empty processor list:

```text
Error: Build failed: list index out of range
```

With this change, recipe-free input resolution produces `pixel_values`
`[1, 3, 256, 192]`. The checked-in recipes represent verified CPU
fp32/fp16 coverage.

## Validation

- Final head: `5635a06aee12a2dfe3d2f0a4e7c92dc8ac00f8f1`
- Recipe-free CPU fp32 `winml build`: passed; `pixel_values` `[1, 3,
256, 192]` float32 -> `heatmaps`; final `model.onnx` generated
- Checked-in CPU fp32 recipe `winml build`: exit 0; completed in 73.5s;
final `model.onnx` generated
- Checked-in CPU fp16 recipe `winml build`: exit 0; completed in 82.7s
(Export 38.0s, Optimize 35.3s, FP16 8.8s); final `model.onnx` generated
- `tests/unit/export/test_io.py` +
`tests/unit/models/test_vitpose_mapping.py`: 101 passed
- Ruff check: passed
- Ruff format check: passed
- GitHub checks: 9/9 passed
- Recipe README remains unchanged

## Files

-
`examples/recipes/nielsr_vitpose-base-simple/cpu/cpu/keypoint-detection_fp32_config.json`
-
`examples/recipes/nielsr_vitpose-base-simple/cpu/cpu/keypoint-detection_fp16_config.json`
- `src/winml/modelkit/models/hf/vitpose.py`
- `tests/unit/models/test_vitpose_mapping.py`
@ssss141414
ssss141414 force-pushed the vitpose-eval-processor-fallback branch from 9eb7dd8 to b9f924e Compare August 14, 2026 06:56
@ssss141414

Copy link
Copy Markdown
Contributor Author

REQUEST_CHANGES

Reviewed exact PR head a49e317698c1d2f15220ade22560a5fca37c87ca from a fresh detached worktree. This is a skill-level reviewer opinion posted as a normal PR comment, not a GitHub Review state.

Blocking items

  1. Current-main baseline attestation is missing. origin/main is now 8f6675d14142c5d534810516c9db421d9cde6a89, while the body still identifies 24cccfb23d6e3c3b93ae9bb94399c326cee8ecea as pinned main. The complete intervening diff includes shared src/winml/modelkit/commands/build.py and src/winml/modelkit/session/genai_session.py, so the old L0-L3 evidence cannot be relabeled current without the moved-main process. Owners: Planner, Tester, Explainer. Planner must produce the _meta-107 complete file/dependency impact attestation and candidate-patch equivalence against current main; Tester must rerun every stage the attestation marks impacted; Explainer must update Baseline/Outcome provenance in the body. A content-based NO-IMPACT result is acceptable only if every intervening file is covered by the required frozen dependency manifest and rationale.

  2. The methodology declaration is stale and does not cite the pushed Lane A commit. Independent remote verification now finds a8d9a406bca1efaaf4dd4bace8d990334b5edc97 at upstream ref refs/heads/ssss141414/pr1310-component-mapping-skill. The private fork/repository APIs still return HTTP 404, so no Lane A PR URL should be invented, but the body currently says the edits were only prepared locally and upstream push verification was unavailable, and it omits the exact pushed commit. The reviewer contract requires the model PR to cite the exact pushed Lane A commit/PR when methodology friction was observed. Owners: Explainer and Orchestrator. Correct the declaration to cite the pushed SHA/ref and retain the truthful Draft-PR API blocker; if the governing contract requires a Lane A PR rather than accepting the pushed commit, Orchestrator must keep shipment explicitly BLOCKED.

Independent verification

  • Diff scope: only src/winml/modelkit/eval/keypoint_detection_evaluator.py and tests/unit/eval/test_keypoint_detection_evaluator.py; no README, recipe, skill, workflow, or production-index changes.
  • Engineering review: Auto loading remains primary; fallback requires both the exact unrecognized-processor ValueError text and VitPose model_type/architectures metadata; unrelated and non-VitPose exceptions propagate; missing model_id fails before loading; trust_remote_code is forwarded on both paths. Existing input-size override, crop fan-out, custom layout, dataset_index, box conversion, and metric behavior are unchanged.
  • Candidate fingerprint reproduced: PR parent is b9f924e3b23bd8caf85339faa3853eeb20df2f17; hashing the committed test diff reproduces 96e89a5f3a2adeeb566d7fa80969441b42027f1c.
  • Local exact-head gates in the source-bound locked environment: focused evaluator 16 passed; ruff passed; mypy reported no issues in 435 source files; all five ordinary partitions passed (1526 + 1524 + 714 + 3611 + 870 = 8245). Literal uv run attempted a resync and hit the documented TLS handshake failure, so execution used uv run --no-sync after source binding was proven.
  • Artifact checks: fp32/fp16 structures and realized precision match; component mapping reconciles exactly at 359/359 (354/2/3) with matching operator histograms; exporter tagging is 512/512; all seven static-analysis records have runtime_support=false, so no runtime compatibility is inferred.
  • Fresh fp32 CPU perf passed: p50 92.18 ms, within 8.0% of sealed 85.316 ms; RAM delta 42.2 MB versus 41.93 MB.
  • COCO smoke provenance independently checked: official 2017 validation image 29397, annotation 449985, one RGB row/person/invocation/prediction, and the stored raw COCO OKS metrics match the report. This remains an operability smoke, not representative accuracy.
  • GitHub gate at the reviewed SHA: Draft=true; label model-scale-by-skill; 9/9 checks COMPLETED/SUCCESS; 0 issue comments, 0 inline comments, 0 reviews, 0 review threads, 0 open threads.

After both blocking items are corrected, re-run review from the new exact head; any push invalidates the current check/thread snapshot.

@ssss141414
ssss141414 force-pushed the vitpose-eval-processor-fallback branch from a49e317 to 4e12415 Compare August 18, 2026 04:19
@ssss141414

Copy link
Copy Markdown
Contributor Author

REQUEST_CHANGES

Reviewed exact PR head 4e12415ea2829a61e3d45963cc9514fd8930f7dd on current main 48bfd0f91478a8e281b205b0449222024729f3ea from a fresh detached worktree. This is a skill-level reviewer opinion posted as a normal PR comment, not a GitHub Review state.

Blocking item

  1. The refreshed Tester evidence root is not immutable after sealing. Independent SHA-256 verification of all 57 entries in artifact-hashes.json found two post-seal mismatches: 17_pytest_remaining.log expected 166bc15011607b5f27f944fdbe8dfb3476dbef6e994d0bb41128bf913e1ff846 but is now e57a15653dbb3c2f94207a95a42b2a23e48dd90555d7628dfebe0d4678cb9366; command-log.json expected a6408195a54dc74ab17ebfe1e240bb5b2d40fa7f945d9fac385350adee6d22e8 but is now 98612ba87e4d12d6efafbda68681fa179f24de3eee3ee4b38cf24eb0813692c3. The current ledger was rewritten to 17 records and points at a later remaining-partition execution; the current log still reports 871 passes but no longer matches the sealed acceptance record or the duration rendered in the verdict/body. Owner: Tester. Create a new empty revision-specific quality root, rerun the affected remaining workflow partition at exact head, regenerate the command ledger, quality summary, verdict, artifact hash map, and seal, then verify every sealed entry before handoff. Do not repair or reseal the mutated root in place.

Independent verification

  • Scope and code: exactly two changed files, evaluator plus tests; no recipe, production README, workflow, dependency, or skill changes. Generic AutoImageProcessor remains primary; fallback requires the known unrecognized-processor ValueError plus VitPose metadata; unrelated/non-VitPose failures propagate; trust_remote_code is preserved; no model ID is hardcoded.
  • Rebase equivalence: range-diff maps both commits with =; stable patch IDs are unchanged; old/new combined binary diff hash is 41ff5b6148016d49484d23a69e4521dcc3fa7c6e; both owned blobs are identical; focused old/new diff exits 0.
  • Moved-main audit: 24cccfb...48bfd0f contains exactly 30 files, classified independently as 19 NO-IMPACT, 11 quality-only IMPACT, 0 UNKNOWN. GenAI build/session and Qwen/Unlimited-OCR registration changes are unreachable from ordinary VitPose HF ONNX execution; explicit CPU compatibility remains true; VitPose/evaluator/metric/recipe, lock, dependency, and workflow surfaces are unchanged. Reuse of model measurements at b9f924e3b23bd8caf85339faa3853eeb20df2f17 is valid and is not relabeled as final-head execution.
  • Exact-head local checks in the 249/249 lock-audited source-bound environment: focused evaluator 16 passed in 22.87s; Ruff All checks passed!; mypy Success: no issues found in 436 source files. Tester logs report five partitions totaling 1526 + 1531 + 714 + 3615 + 871 = 8257, but the mutated seal prevents accepting that refreshed package as immutable evidence.
  • Body: live body equals the explainer artifact after newline normalization; hierarchy, current-main/old-evidence provenance, final head/runtime equivalence, exact old model measurements, 359/359 mapping, seven runtime_support=false static records, 8257 quality count, and reproduce commands are accurate. No local evidence paths or artifact hashes appear.
  • Lane A: commit a8d9a406bca1efaaf4dd4bace8d990334b5edc97 is remotely reachable at refs/heads/ssss141414/pr1310-component-mapping-skill; the Draft PR API remains HTTP 404, so no PR URL is required or invented under the accepted pushed-commit path.
  • GitHub snapshot before verdict: Draft=true; label model-scale-by-skill; mergeable=MERGEABLE; 9/9 checks COMPLETED/SUCCESS; 1 prior issue comment; 0 inline comments; 0 reviews; 0 review threads; 0 open threads.

After Tester supplies a new immutable sealed root, re-enter review without changing the PR branch unless a product fix is independently required. Any head or main movement restarts the current-main gate.

@ssss141414

Copy link
Copy Markdown
Contributor Author

APPROVE

Reviewed exact PR head 4e12415ea2829a61e3d45963cc9514fd8930f7dd on current main 48bfd0f91478a8e281b205b0449222024729f3ea. This is an adding-model-support skill-level reviewer opinion only, posted as a normal PR comment. The PR remains Draft, and no GitHub Review state is submitted.

Independent verification

  • Scope and engineering judgment: exactly two changed files: src/winml/modelkit/eval/keypoint_detection_evaluator.py and tests/unit/eval/test_keypoint_detection_evaluator.py. The generic AutoImageProcessor path remains primary. Fallback requires the known ValueError text plus VitPose model_type or architecture metadata, forwards trust_remote_code, contains no model-ID branch, and preserves unrelated/non-VitPose error propagation. The tests cover both metadata gates, strict error gating, primary success, missing model ID, and trust forwarding. No design blocker found.
  • Rebase/runtime identity: merge base is current main; HEAD^ is runtime commit e7c8bd2041ac1aa40329b866362ffa72933f6252. Range-diff maps both original commits with =. Runtime/test stable patch IDs remain 478d2148c53cef8dce64706e5f584006ce64aebe and 0f4629fc141ef9b66715bd644055d4b1a6af64f5; owned blobs are unchanged.
  • Corrective evidence seal: the new tester-4e12415-quality-r2 root supersedes the invalid r1 root and does not claim r1 as sealed acceptance evidence. Independent external two-pass verification found exactly 36 listed artifacts plus the 2 declared exclusions, hash-map SHA-256 e67b00427214a6a726c0c85cf867cb9917e733f883d6459cdc6dd96eca3a06d1, 0 missing files, 0 unexpected files, 0 hash mismatches, and no snapshot change. Every carried raw log/exit file matches both its copied hash and the original expected SHA-256 in r1 artifact-hashes.json; 0 carry mismatches.
  • Quality/source proof: exact head/base/runtime and source binding pass; 249/249 applicable lock requirements match and uv lock --check resolves 256 packages. Corrective fresh runs are focused 16 passed and remaining 871 passed; hash-verified carries cover insert-license, Ruff, Mypy, analyze, models, optim, and commands. Canonical total is 8257 passed. My independent fresh focused run also passed 16/16.
  • Model evidence provenance: config/build/perf/parity/Eval/analyze measurements retain original execution SHA b9f924e3b23bd8caf85339faa3853eeb20df2f17; they were not rerun or relabeled. CPU fp32/fp16 L0-L2 and the bounded one-row fp32 CPU COCO functional smoke remain supported through exact patch equivalence and the moved-main attestation.
  • Moved main: all 30 intervening files are accounted for as 19 no-impact, 11 quality-only, and 0 unknown. Dependency locks and workflow definitions did not change. Spot checks confirm GenAI build/session changes are unreachable from VitPose's ordinary ONNX path and explicit CPUExecutionProvider/cpu compatibility remains behavior-preserving. The required impacted quality gates were refreshed.
  • Lane A/body/shipment: the body has the required hierarchy and current provenance, including pushed Lane A commit a8d9a406bca1efaaf4dd4bace8d990334b5edc97 at refs/heads/ssss141414/pr1310-component-mapping-skill; independent ls-remote matches. Draft-PR API 404 remains stated without inventing a URL. PR is Draft, labeled model-scale-by-skill, and mergeable.
  • Discussion and checks: paginated enumeration found 2 prior issue comments, 0 inline comments, 0 reviews, 0 review threads, and 0 open threads. All 9 exact-head checks are COMPLETED/SUCCESS: Analyze (Python), lint, test (analyze), test (models), test (optim), test (commands), test (remaining), CodeQL, and license/cla.

Blockers

None. The prior moved-main/body and evidence-seal blockers are cleared.

@DingmaomaoBJTU

Copy link
Copy Markdown
Collaborator

REQUEST_CHANGES

Reviewed exact PR head 1207965854b5eeb3dec055a7cfa1f08db30fa6f9 on base/current main 2b9ec0e9e57bba8003d25eaff85f8c7c9e30d75a from a fresh detached worktree. This is an adding-model-support skill-level reviewer opinion posted as a normal conversation comment, not a GitHub Review state.

Blocking item

  1. The Baseline and Delta sections claim an executed auto-config result and baseline-generated recipe that do not exist. Owner: Explainer. The same body correctly says baseline provisioning failed before product execution, no fresh current-main baseline config/build ran, and the dynamic Optimum probe was blocked. The charter records a planned config_command and output path, but no executed baseline config result or generated recipe. Therefore Starting auto-config selected... and relative to the baseline-generated recipe are source-fidelity defects, not acceptable shorthand.

    Replace the second paragraph under ### Baseline with exactly:

    No baseline winml config command executed and no baseline recipe was generated. Static installed-source evidence identifies the keypoint-detection task, ViTPoseForPoseEstimation, Optimum's vitpose/keypoint-detection registration, and WinML's override with DummyVisionInputGenerator; these are source-inspection facts, not an executed auto-config selection. No registration was found for vit-pose or vit_pose. A fresh dynamic probe was blocked by an internally incompatible current lock and is not represented as fresh evidence.

    Replace the first paragraph under ### Delta with exactly:

    No baseline recipe was generated because baseline provisioning failed before product execution, so no baseline-generated-recipe or JSON-pointer comparison exists. The existing merged examples/recipes/nielsr_vitpose-base-simple/cpu/cpu/keypoint-detection_fp32_config.json and keypoint-detection_fp16_config.json files remain byte-for-byte unchanged, and this PR has no recipe delta. Original-execution recipe-free build/Eval evidence at b9f924e3b23bd8caf85339faa3853eeb20df2f17 verifies the code path independently under the stated no-impact and exact-equivalence reuse attestation. The production examples/recipes/README.md remains untouched.

This is body-only and does not require a product-branch push. After the Explainer updates the body and refreshes its shipment artifact, invoke a new independent reviewer iteration.

Independent verification

  • Scope and engineering: exactly two changed paths: src/winml/modelkit/eval/keypoint_detection_evaluator.py and tests/unit/eval/test_keypoint_detection_evaluator.py. AutoImageProcessor remains primary; fallback requires the exact known ValueError text plus case-insensitive VitPose model_type/architectures metadata; trust_remote_code is forwarded; missing model_id, unrelated errors, and non-VitPose errors fail closed; no model ID is hardcoded. No skip/xfail or README change was added.
  • Moved main and patch identity: accepted base 48bfd0f91478a8e281b205b0449222024729f3ea is an ancestor of current main. The complete delta is exactly PR Report memory metadata in winml sys JSON #1312's four winml sys files and has no config/build/perf/Eval/analyze import reachability. Range-diff maps both commits with =; old/new stable patch IDs are identical; both owned blobs are identical; the owned trees match; current two-file diff SHA-256 is 4c4a0722b979e44dd1678c1712109c98993125f0f730cdbe4dd2df980fa5cc3c.
  • Sealed tester root: independent verification matched all 52 artifact-map entries, exactly four declared seal exclusions, no missing/unexpected files, no mismatches, and no mutation. Both stored seal snapshots agree. Failed and superseded provisioning/binding/provenance attempts remain in the 21-record ledger with their accepted repairs.
  • Fresh local exact-head gates: insert-license passed and left the worktree clean; full Ruff passed; full Mypy reported no issues in 436 source files; focused evaluator tests passed 16; the complete workflow commands partition passed 3618 with 9 skipped and one warning. Imports were proven to resolve to the detached review source.
  • Model evidence reuse: config/build/perf/parity/Eval/analyze were originally executed at b9f924e3b23bd8caf85339faa3853eeb20df2f17, with old runtime patch e7c8bd2041ac1aa40329b866362ffa72933f6252 exact-equivalent to current 88bd424bbf0e415fa05c25c3cf9ad369277a06d5. They were not rerun at 12079658. Prior independently accepted evidence plus exact patch/dependency equivalence supports full CPU fp32/fp16 L0-L2 coverage and one fp32 CPU COCO functional smoke, with no deferred tuples. The fp16 artifact is genuinely half-size with FLOAT16 initializers; the one-row official COCO result preserves image/annotation identity, one-person fanout, raw OKS metrics, no mAP=0.8 confusion, and no benchmark claim. Component/op evidence reconciles 359/359 nodes.
  • Knowledge: pushed Lane A commit a8d9a406bca1efaaf4dd4bace8d990334b5edc97 contains vitpose-017 through vitpose-021 and _meta-111; today's learner correctly deduplicated and did not claim new methodology friction.
  • GitHub pre-comment gate: OPEN/DRAFT, label model-scale-by-skill, mergeable, live body equals the explainer artifact after newline normalization, all 9 exact-head checks are COMPLETED/SUCCESS, and fully paginated enumeration found 3 historical issue comments, 0 inline comments, 0 reviews, 0 review threads, and 0 open threads.

Blocker

Explainer must make the two exact source-fidelity replacements above and refresh the shipment artifact. All code, test, evidence-seal, reuse, coverage, knowledge, check, and thread gates otherwise pass.

@DingmaomaoBJTU

Copy link
Copy Markdown
Collaborator

APPROVE

Reviewed exact PR head 1207965854b5eeb3dec055a7cfa1f08db30fa6f9 on base/current main 2b9ec0e9e57bba8003d25eaff85f8c7c9e30d75a from the fresh detached worktree C:/src/workspace/winml-cli-worktrees/pr1310-review-20260819-r2. This is an adding-model-support skill-level opinion posted as a normal conversation comment, not a GitHub Review approval or authorization to mark the PR ready.

Iteration 1 correction

The only iteration-1 blocker is resolved. After newline normalization, the live body exactly equals the corrected explainer-r2 artifact. The r1-to-r2 diff contains exactly two paragraph replacements and no other semantic changes: Baseline now explicitly says no baseline winml config command executed, no baseline recipe was generated, and static source facts are not auto-config execution; Delta now explicitly says no baseline-generated-recipe or JSON-pointer comparison exists. Both exact requested replacement paragraphs occur once, and both rejected assertions are absent.

Independent verification

  • Scope and engineering: exactly two changed paths: src/winml/modelkit/eval/keypoint_detection_evaluator.py and tests/unit/eval/test_keypoint_detection_evaluator.py. AutoImageProcessor remains primary; fallback requires the known ValueError text plus case-insensitive VitPose model_type/architectures metadata; trust_remote_code is forwarded; missing model_id, unrelated errors, and non-VitPose errors fail closed. No model ID branch, skip/xfail, recipe, or production README change was added.
  • Moved main and patch identity: prior base 48bfd0f91478a8e281b205b0449222024729f3ea is an ancestor of current main. The complete moved-main delta is PR Report memory metadata in winml sys JSON #1312's four isolated winml sys files, with no import reachability into model stages. Old/new stable patch IDs are identical (478d2148..., 0f4629fc...), range-diff maps both commits with =, and both owned blobs are identical.
  • Sealed evidence: all immutable input hashes are unchanged. Independent verification matched all 52 tester artifact-map entries, exactly four declared exclusions, 21 ledger records, and zero missing, mismatched, or unexpected files. Reuse of iteration-1 reasoning occurred only after re-reading these immutable handoffs and proving their hashes unchanged.
  • Fresh local exact-head gates: Python 3.11.16 from the retained locked runtime was explicitly bound to the r2 src, and both winml and the evaluator imported from the detached checkout. pre-commit run insert-license --all-files passed and left the worktree clean; full ruff check src/ tests/ passed; full mypy -p winml.modelkit found no issues in 436 source files; focused evaluator tests passed 16; the complete workflow commands partition passed 3618, skipped 9, with one warning. The worktree remains clean.
  • Model evidence and coverage: config/build/perf/parity/Eval/analyze were originally executed at b9f924e3b23bd8caf85339faa3853eeb20df2f17; no model-stage rerun occurred at current head. Exact patch/blob equivalence and moved-main dependency analysis support reuse. Coverage is full for CPU/CPU fp32 and fp16 through L0-L2 with no deferred tuples, plus one bounded fp32 CPU COCO functional smoke. The fp16 artifact has FLOAT16 initializers and approximately half-size external data. The smoke is operability-only and makes no representative-accuracy or benchmark claim.
  • Analysis and knowledge: component mapping reconciles 359/359 nodes with zero unmapped; op analysis reports 359 operators across 12 types and preserves unavailable-rule groups as unknown. Lane A commit a8d9a406bca1efaaf4dd4bace8d990334b5edc97 contains vitpose-017 through vitpose-021 and _meta-111; this rerun correctly deduplicated and declared no new methodology friction.
  • GitHub pre-comment gate: OPEN/DRAFT, label model-scale-by-skill, live body equals explainer-r2 after newline normalization, and all 9 exact-head checks are COMPLETED/SUCCESS. Fully paginated enumeration found 4 issue comments, 0 inline comments, 0 GitHub reviews, 0 GraphQL review threads, and 0 open threads. The prior REQUEST_CHANGES is issue comment 5340329154, not a review thread, and its two requested replacements are resolved.

Blocker

None. Keep the PR in DRAFT; this comment does not create a GitHub Review state.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model-scale-by-skill Model support PR created or maintained by the adding-model-support skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants