Carry the weight-layout tag through an Identity fold - #27
Closed
enthropy7 wants to merge 1 commit into
Closed
Conversation
The tracker model stopped loading on this branch: `Grouped Conv channel mismatch: IC=3, OC=3, group=16` at `/xif1_0/dw/conv_1/Conv`. Its weight `onnx::Conv_1040` holds a pre-permuted depthwise `[3, 3, 16, 1]`, but no layout table listed it, so the Conv parsed it as `[O, I, KH, KW]` and read the kernel height back as the output-channel count. Two halves of one round trip were missing. `foldable_inputs` refused any node whose operand is a pre-permuted weight. That is right for an operator that interprets the bytes, but `Identity` only copies them, and refusing it is not the conservative choice: the node then survives to run time and hands its consumer a permuted tensor with no tag attached. The model reads this weight twice — once straight, which is what makes the loader permute and register it, and once through an alias — so only the second reader broke. `apply_ir` then deliberately left the loader's layout side tables alone, on the grounds that passes preserve weight names. They do not; a folded alias is a new name. The tables are now rebuilt from the IR's per-value `weight_layouts`, which survives the rename and is the authority. Also gate the three plan tests that assume the pre-permuted layout. They assert a fused plan shape that only forms when the loader permutes, and `cargo test -p yscv-onnx --features gpu --lib` (ci.yml:86) keeps the ONNX-native layout, where the plan legitimately differs — the three were a guaranteed red on that job, not a flake. Tracker back to 8.86 ms/1T on Zen 4, against 8.79 on main. Workspace suite 2373 passed; clippy clean with default and with `gpu`.
Owner
Author
|
Closing — landing directly on main instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #25 — targets
onnx-optimizer-ir, notmain.The tracker model does not load on #25:
Grouped Conv channel mismatch: IC=3, OC=3, group=16at/xif1_0/dw/conv_1/Conv. Its weight holds a pre-permuteddepthwise
[3, 3, 16, 1]that no layout table lists, so the Conv parses it as[O, I, KH, KW]and reads the kernel height back as the output-channel count.mainloads and runs the same model. yolo11n was unaffected, which is why itdid not show up sooner.
Two halves of one round trip were missing:
foldable_inputsrefused any node with a pre-permuted operand. Right for anoperator that interprets the bytes, wrong for
Identity, which only copiesthem — refusing it lets the node survive to run time and hand its consumer a
permuted tensor with no tag. This model reads the weight twice, once straight
(which is what registers it) and once through an alias, so only the second
reader broke.
apply_irdeliberately left the layout side tables alone, on the grounds thatpasses preserve weight names. A folded alias is a new name. They are now
rebuilt from the IR's per-value
weight_layouts, which survives the rename.Also gates the three plan tests that assume the pre-permuted layout. They pin a
fused plan shape that only forms when the loader permutes, and
cargo test -p yscv-onnx --features gpu --lib(ci.yml:86) keeps ONNX-nativeOIHW, where the plan legitimately differs — a guaranteed red on that job rather
than a flake. Reproduced locally at 243 passed / 3 failed, matching CI exactly.
The new regression test fails without the fold change and passes with it.
Tracker back to 8.86 ms/1T on Zen 4 against 8.79 on main, so the IR rework does
not cost anything on this model once it loads. Workspace suite 2373 passed;
clippy clean with default features and with
gpu.