Skip to content

fix(perf): bind concrete CGC input shapes before compilation - #1433

Open
Qiong Wu (qiowu) (DingmaomaoBJTU) wants to merge 3 commits into
mainfrom
codex/perf-cgc-concrete-shapes
Open

Qiong Wu (qiowu) (DingmaomaoBJTU) wants to merge 3 commits into
mainfrom
codex/perf-cgc-concrete-shapes

Conversation

@DingmaomaoBJTU

@DingmaomaoBJTU Qiong Wu (qiowu) (DingmaomaoBJTU) commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Local ONNX perf compilation can run before concrete input dimensions reach CGC. Resolve shapes from input metadata or CLI defaults and forward them through Runtime compiler options or ORT named-dimension overrides. Provider-discovery changes are excluded.

Windows 11 ARM64 / Adreno X2-90, x64 Python 3.11, WindowsML 2.7.30.dev0 + onnxruntime-windowsml 1.30.0.202609102321. CLI official main 3dd87d3 or PR 1433 at 43d2562. All fixture code is independently authored; no private model, weights, paths or input files are needed.

Minimal synthetic reproduction

Activate the prepared CLI environment with onnx/numpy installed. Save this as make_repro.py:

import numpy as np
import onnx
from onnx import helper as h, TensorProto as T, numpy_helper as nh

def tensor(name, dtype, shape):
    return h.make_tensor_value_info(name, dtype, shape)

kind = 'dynamic_add'
initializers = []
opset = 18
nodes = [h.make_node('Add', ['a', 'b'], ['y'])]
inputs = [tensor('a', T.FLOAT, ['batch', 3]), tensor('b', T.FLOAT, ['batch', 3])]
outputs = [tensor('y', T.FLOAT, ['batch', 3])]
model = h.make_model(h.make_graph(nodes, kind, inputs, outputs, initializers), opset_imports=[h.make_opsetid('', opset)], ir_version=10)
onnx.checker.check_model(model)
onnx.save(model, kind + '.onnx')
python ./make_repro.py
winml perf -m ./dynamic_add.onnx --runtime winml-runtime --backend cgc --device gpu --iterations 1 --warmup 0 --no-memory --no-monitor
$LASTEXITCODE

Also run WinMLCG:

$env:WINMLCLI_EP_PATH = (Resolve-Path .venv/Lib/site-packages/windowsml/lib).Path
winml perf -m ./dynamic_add.onnx --runtime winml-ort --ep winmlcg --device gpu --iterations 1 --warmup 0 --no-memory --no-monitor

Verified behavior

The 117-byte dynamic Add graph uses CLI default batch=1. No shape file or NPZ is required. Base 3dd87d3: A/E both fail compilation. PR 43d2562: A/E both complete using the identical wheels/model. ONNX ReferenceEvaluator validates the elementary computation. This is a shape-handoff test, not full-model support or accuracy certification.

Review fixes preserve anonymous input axes for compiler handling, accept matching static zero extents, and read NPY v1/v2/v3 headers without loading array payloads. Named-dimension overrides still require positive int64 values. The Runtime session import is now used explicitly to address the CodeQL unused-import finding.

Validation of the review fixes: 197 targeted perf/session/CGC-export tests, repository-wide Ruff, and full-package mypy (464 source files) passed. Regression tests reproduced all three reported failures before the fixes and pass afterward. Native GPU compilation was not rerun for this revision.

Only generated data is included. Private inventory references and full-model details have been removed.

Comment thread src/winml/modelkit/commands/perf.py Fixed
@zhenchaoni

Copy link
Copy Markdown
Member

Reviewed head 43d25621b77db8f7b314f30cea1997f1a0435f59 against base 3dd87d3806b9b7aa3bc756367e5479ebf06f7b79.

The core fix works, but the new pre-compilation validation introduces three reproducible compatibility regressions. I recommend addressing the two P2 findings before merging. I did not find an additional confirmed P0/P1 issue in the reviewed paths.

1. [P2] Do not reject an otherwise executable graph just because an unused input has an anonymous axis

runtime_session.py, lines 278-282

Minimal ONNX graph (opset 18, IR version 10, float32):

tensor_0: [2, 3]
tensor_1: [2, 3]
tensor_2: [?, 3]     # graph input, but no node consumes it; ? has no dim_param
tensor_3 = Add(tensor_0, tensor_1)

Supply an NPZ with all three input keys, each containing a [2, 3] array:

winml perf -m .\model.onnx --runtime winml-ort --ep winmlcg --device gpu --input-data .\inputs.npz --iterations 1 --warmup 0 --no-memory --no-monitor

Base: exits 0. PR: exits 1 before session creation:

Input 'tensor_2' axis 0 has no symbolic dimension name; the compiler's named-dimension API cannot bind it.

I understand that this PR intentionally does not add anonymous-axis binding. However, this example does not require that capability: the actual computation already has concrete shapes. Not being able to bind an axis by name is not sufficient evidence that the graph cannot execute. The new blanket rejection narrows the set of previously working models.

Please preserve named overrides without unconditionally rejecting every anonymous source axis, or otherwise distinguish axes that actually require specialization for the lowered computation. This should not silently invent names or guess extents.

2. [P2] The positive-extent restriction must not reject a matching static zero dimension

runtime_session.py, lines 271-277

Use the same graph/command, but declare the unused tensor_2 as ["extent", 0] and supply an array of shape [2, 0].

Base: exits 0. PR: exits 1:

Input 'tensor_2' axis 1 must be a positive int64.

The static zero axis exactly matches the ONNX declaration. Only extent=2 needs to be sent as a symbolic override. The compiler API's positive-value requirement for an override should not be applied indiscriminately to every static axis of every source input. Validate fixed axes separately, retaining legal zero extents when they match the schema.

This reproduction establishes a regression for the unused-input case; it does not claim that every CGC operator supports empty tensors.

3. [P3] Preserve the existing loader's support for NPY v3 entries inside NPZ

perf.py, lines 934-941

Use the same graph with unused tensor_2: ["extent", 3], all three arrays shaped [2, 3], and write the archive's .npy entries with NumPy's np.lib.format.write_array(..., version=(3, 0), allow_pickle=False).

Base: exits 0; the existing np.load(..., allow_pickle=False) loader accepts the file. PR: exits 1:

Cannot read concrete --input-data shapes: Unsupported NPY header version (3, 0)

These are ordinary float32 arrays, not object/pickled arrays. Please support v3 headers without allocating the payload, keeping the header reader's accepted formats aligned with the existing loader. The same named-axis fixture with v1 entries succeeds on both revisions.

Verification and scope

  • Environment: Windows x64 / Python 3.11, AMD Radeon Pro WX 3200, windowsml-2.7.52a0 and onnxruntime_windowsml-1.30.0.202609102321, in an isolated venv. Both revisions used the same wheels. Reproductions use generated models/data, not private model assets.
  • Each regression was confirmed both through PerfBenchmark and through an actual CLI subprocess launched by pytest. Four compatibility fixtures (three regressions plus the named-axis/v1 control) give 8/8 passing checks on base, versus 2 passing / 6 failing checks on the PR.
  • The intended fix is also confirmed: eight dynamic Add cases covering NPZ, symbolic shape-config, per-input shape-config, and batch-size across Runtime CGC and ORT WinMLCG fail at compilation on base and succeed on the PR, with outputs checked against ONNX ReferenceEvaluator. Static controls pass on both revisions. A separate WinMLCG check succeeds with CPU EP fallback explicitly disabled.
  • Additional direct CLI checks pass for default batch resolution and for NPZ shapes taking precedence over conflicting shape-config/batch-size settings on both CGC paths. The latest targeted repository run passes all 153 tests covering shape/session behavior, CGC export/freeze options, and ONNX model routing; the earlier adjacent perf/composite/memory/session-options run passed 207 tests.
  • I found no direct double-freeze conflict with the exporter's existing batch_size=1 default: standalone CGIR export uses that exporter, whereas online ONNX CGC perf bypasses it. Pre-exported MLIR does not enter this PR's new shape-binding path. Those paths can still intentionally represent different fixed shapes.

The first two findings are about preserving previously executable inputs, not a request to implement general dynamic-shape support. Internal data-dependent shapes and full-model/operator coverage remain outside this validation.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants