Skip to content

Fix fp16 quantization name collision - #1417

Merged
Zhenchao Ni (zhenchaoni) merged 2 commits into
mainfrom
private/zhenni/fix_fp16
Sep 14, 2026
Merged

Zhenchao Ni (zhenchaoni) merged 2 commits into
mainfrom
private/zhenni/fix_fp16

Conversation

@zhenchaoni

Copy link
Copy Markdown
Member

Problem

ORT’s FP16 converter generates deterministic I/O Cast names when preserving FP32 public I/O. Existing models, including the Affinity encoder and decoder, may already contain these names. Our collision guard consequently rejects conversion.

Solution

Add a preprocessing step that renames conflicting internal tensors and nodes before calling ORT:

  • Use inferred I/O types to identify the exact names ORT will generate.
  • Allocate deterministic __existing_N names, avoiding existing and newly allocated names.
  • Update tensor references and metadata, including nested captures, while respecting local scope.
  • Preserve public I/O names and types, and leave conversion-neutral nodes unchanged.
  • Retain the existing collision guard as a safety check.

This change only resolves naming conflicts; it does not change precision-conversion policy or prevent ORT from adding Casts during repeated conversion.

@zhenchaoni
Zhenchao Ni (zhenchaoni) requested a review from a team as a code owner September 14, 2026 06:29

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the collision preprocessing and its integration. The standalone FP16 test module passed all 148 tests with ONNX 1.18 and ORT 1.30 (outside the repository-wide conftest/native EP setup). A separate reproduction identified the failure-path mutation below; no product files were changed.

Comment thread src/winml/modelkit/quant/fp16.py

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed the update since 35d04da. The previous failure-path mutation finding is addressed: keep-I/O preprocessing and conversion operate on a copy, and the caller model is updated only after successful validation. The original unloaded-external-weight reproduction now leaves the caller model byte-for-byte unchanged. All 149 standalone FP16 tests passed locally with ONNX 1.18 / ORT 1.30, outside the repository-wide conftest, and diff checking passed. No remaining blocking findings. CI optim/models/CLA checks have passed; other CI jobs are still pending at review time, so this approval does not assert a fully green CI run.

@zhenchaoni
Zhenchao Ni (zhenchaoni) merged commit 3772828 into main Sep 14, 2026
9 checks passed
@zhenchaoni
Zhenchao Ni (zhenchaoni) deleted the private/zhenni/fix_fp16 branch September 14, 2026 08:15
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