Skip to content

Commit 095d1e5

Browse files
justinchubyCopilot
andauthored
Fix SkipSimplifiedLayerNormalization fallback output arity (#387)
## Problem The standard-ONNX fallback body for `com.microsoft::SkipSimplifiedLayerNormalization` (`functions/skip_layer_normalization.py`) returned only **two** outputs, `[norm_out, add_out]`, placing the residual sum (`input_skip_bias_sum`) at **index 1**. Per the operator spec the four positional outputs are: | idx | output | |-----|--------| | 0 | `output` (normalized) | | 1 | `mean` (optional, training) | | 2 | `inv_std_var` (optional, training) | | 3 | `input_skip_bias_sum` (residual) | The fusion rule in `rewrite_rules/_skip_norm.py` is already spec-correct — it emits a **4-output** node and reads `outputs[3]` for the residual. But when `InlinePass` expands that 4-output node using the 2-output fallback, `onnx_ir.convenience.replace_all_uses_with` raises: ``` ValueError: The number of values and replacements must match. ``` because the residual at index 3 has no replacement value. Even without that guard, the sum would silently land in the optional `mean` slot at index 1. ### Reachability Fusion is gated on `supports_skip_layer_norm` (`_optimizations.py`), so EPs that would inline this op (`onnx-standard`, `qnn`, `trt-rtx`) don't create the node in a normal single-EP pass — the crash only surfaces on a fuse-then-re-optimize path. It is nonetheless a real spec-conformance bug in the fallback body. ## Fix Emit four outputs with the residual sum at index 3. `mean` and `inv_std_var` are training-only outputs that the simplified (RMS) variant does not compute and that single-output `RMSNormalization` cannot supply, so they are emitted as unused `Constant` placeholders and pruned by `RemoveUnusedNodesPass` immediately after the function is inlined. ## Verification - `InlinePass` on a tiny Qwen2 model (`onnx-standard` EP) now expands all fused ops with **no dangling values**; the placeholder `Constant`s are DCE'd. - Inlined fallback produces **byte-identical logits** to the native op (max abs diff `0.0`). - New `functions/skip_layer_normalization_test.py` (output-arity + inline-residual regression tests). - Existing skip-norm and EP-optimization suites pass; lint clean. Signed-off-by: Justin Chu <11205048+justinchuby@users.noreply.github.com> Co-authored-by: Justin Chu <11205048+justinchuby@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1f04e6b commit 095d1e5

2 files changed

Lines changed: 138 additions & 2 deletions

File tree

src/mobius/functions/skip_layer_normalization.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,20 @@ def skip_simplified_layer_normalization() -> ir.Function:
9494
norm_out = RMSNormalization(add_out, weight, epsilon=<forwarded>)
9595
9696
Inputs: ``[input, skip, weight]``
97-
Outputs: ``[norm_out, add_out]``
97+
Outputs: ``[norm_out, mean, inv_std_var, add_out]``
9898
Attr: ``epsilon`` (float)
99+
100+
The ``com.microsoft::SkipSimplifiedLayerNormalization`` op declares four
101+
positional outputs — ``output`` (0), ``mean`` (1), ``inv_std_var`` (2) and
102+
``input_skip_bias_sum`` (3). The residual sum lives at **index 3**, so the
103+
function body must expose it there for InlinePass to reconnect downstream
104+
consumers correctly (see :mod:`mobius.rewrite_rules._skip_norm`, which reads
105+
``outputs[3]``). ``mean`` and ``inv_std_var`` are training-only outputs that
106+
the simplified (RMS) variant does not compute and that ``RMSNormalization``
107+
(single-output) cannot supply; they are emitted as unused ``Constant``
108+
placeholders purely to keep the output arity aligned. Because the fusion
109+
rule never wires indices 1-2 to any consumer, ``RemoveUnusedNodesPass``
110+
prunes these placeholders right after the function is inlined.
99111
"""
100112

101113
def body(op, v_input, v_skip, v_weight):
@@ -110,9 +122,16 @@ def body(op, v_input, v_skip, v_weight):
110122
)
111123
norm_out = op.RMSNormalization(add_out, v_weight, epsilon=epsilon_attr)
112124

125+
# Optional mean (1) / inv_std_var (2): not produced by RMS. Emit unused
126+
# placeholders so input_skip_bias_sum stays at index 3; pruned by DCE.
127+
mean = op.Constant(value_floats=[0.0])
128+
inv_std_var = op.Constant(value_floats=[0.0])
129+
113130
norm_out.name = "norm_out"
131+
mean.name = "mean"
132+
inv_std_var.name = "inv_std_var"
114133
add_out.name = "add_out"
115-
return norm_out, add_out
134+
return norm_out, mean, inv_std_var, add_out
116135

117136
return build_function(
118137
body,
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
"""Tests for the standard-ONNX SkipLayerNormalization function bodies.
5+
6+
These bodies are inlined by :class:`onnx_ir.passes.common.InlinePass` to
7+
expand the ``com.microsoft`` custom ops for EPs that do not support them.
8+
The key invariant is that each function's output arity and ordering match
9+
the ``com.microsoft`` op spec so InlinePass can reconnect downstream
10+
consumers by position — in particular ``input_skip_bias_sum`` must stay at
11+
output index 3, not collapse into the optional ``mean`` slot at index 1.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import onnx_ir as ir
17+
from onnx_ir.passes.common import InlinePass
18+
19+
from mobius._builder import build_from_module
20+
from mobius._configs import ArchitectureConfig
21+
from mobius._registry import registry
22+
from mobius.functions import register_function_bodies
23+
from mobius.functions.skip_layer_normalization import (
24+
skip_layer_normalization,
25+
skip_simplified_layer_normalization,
26+
)
27+
28+
29+
def _tiny_config() -> ArchitectureConfig:
30+
return ArchitectureConfig(
31+
hidden_size=64,
32+
intermediate_size=128,
33+
num_attention_heads=4,
34+
num_key_value_heads=2,
35+
head_dim=16,
36+
num_hidden_layers=2,
37+
vocab_size=256,
38+
max_position_embeddings=128,
39+
hidden_act="silu",
40+
rms_norm_eps=1e-6,
41+
rope_type="default",
42+
rope_theta=10000.0,
43+
pad_token_id=0,
44+
)
45+
46+
47+
def _count(model: ir.Model, op_type: str) -> int:
48+
return sum(1 for n in model.graph.all_nodes() if n.op_type == op_type)
49+
50+
51+
def _has_dangling_inputs(model: ir.Model) -> bool:
52+
"""Return True if any node consumes a value with no producer/initializer/input."""
53+
known: set[ir.Value] = set(model.graph.inputs)
54+
known.update(model.graph.initializers.values())
55+
for node in model.graph.all_nodes():
56+
known.update(node.outputs)
57+
for node in model.graph.all_nodes():
58+
for inp in node.inputs:
59+
if inp is not None and inp.name and inp not in known:
60+
return True
61+
return False
62+
63+
64+
class TestSkipSimplifiedFunctionSignature:
65+
def test_output_arity_matches_spec(self):
66+
"""SkipSimplifiedLayerNormalization must expose 4 positional outputs.
67+
68+
Spec order: output(0), mean(1), inv_std_var(2), input_skip_bias_sum(3).
69+
"""
70+
fn = skip_simplified_layer_normalization()
71+
assert len(fn.outputs) == 4
72+
# The residual sum must live at index 3 (not index 1).
73+
assert fn.outputs[3].name == "add_out"
74+
assert fn.outputs[0].name == "norm_out"
75+
76+
def test_skip_layer_norm_output_arity(self):
77+
"""The non-simplified variant also exposes 4 outputs with sum at index 3."""
78+
fn = skip_layer_normalization()
79+
assert len(fn.outputs) == 4
80+
assert fn.outputs[3].name == "add_out"
81+
82+
83+
class TestSkipSimplifiedInline:
84+
def _inline_skip_norm(self, model: ir.Model) -> None:
85+
register_function_bodies(model)
86+
87+
def criteria(func: ir.Function) -> bool:
88+
return func.domain == "com.microsoft" and func.name in (
89+
"SkipLayerNormalization",
90+
"SkipSimplifiedLayerNormalization",
91+
)
92+
93+
InlinePass(criteria=criteria)(model)
94+
95+
def test_inline_preserves_residual(self):
96+
"""Inlining the fallback must expand every fused op and keep the graph valid.
97+
98+
Regression test: with the old 2-output body, InlinePass raised
99+
``ValueError`` (output-count mismatch) because the 4-output node's
100+
``input_skip_bias_sum`` (index 3) had no replacement value. The fixed
101+
4-output body reconnects the residual and leaves no dangling inputs.
102+
"""
103+
config = _tiny_config()
104+
model = build_from_module(registry.get("qwen2")(config), config)["model"]
105+
106+
fused = _count(model, "SkipSimplifiedLayerNormalization")
107+
assert fused > 0, "expected the build pipeline to fuse Add+RMSNorm"
108+
109+
self._inline_skip_norm(model)
110+
111+
# All fused ops expanded back to Add + RMSNormalization.
112+
assert _count(model, "SkipSimplifiedLayerNormalization") == 0
113+
# Each expansion restores one residual Add.
114+
assert _count(model, "Add") >= fused
115+
# The optional mean/inv_std placeholders are unused and pruned-safe;
116+
# more importantly the residual reconnected with no dangling inputs.
117+
assert not _has_dangling_inputs(model)

0 commit comments

Comments
 (0)