Skip to content

Commit a969ada

Browse files
michaelgtuttleGitHub Enterprise
authored andcommitted
Fix onnxruntime dispatching to incorrect function definition
Signed-off-by: Michael Tuttle <mtuttle@qti.qualcomm.com>
1 parent 555c0df commit a969ada

7 files changed

Lines changed: 106 additions & 40 deletions

File tree

TrainingExtensions/onnx/src/python/aimet_onnx/graph_passes/fusions/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from .fusion import fuse_supergroups
77
from .fusion_registry import AIMET_SUPERGROUP_DOMAIN
8-
from .ir_utils import inline_all_supergroups
8+
from .ir_utils import inline_all_supergroups, is_fused_supergroup
99
from .layernorm import LayerNormFusion
1010
from .matmul_add import MatmulAddFusion
1111
from .rmsnorm import RMSNormFusion

TrainingExtensions/onnx/src/python/aimet_onnx/graph_passes/fusions/fusion.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from collections import defaultdict
77
import onnx_ir
88
from onnxscript.rewriter import pattern
9-
from .fusion_registry import FUSION_PASS_REGISTRY, AIMET_SUPERGROUP_DOMAIN
9+
from .fusion_registry import FUSION_PASS_REGISTRY
1010
from . import ir_utils
1111

1212

@@ -53,6 +53,8 @@ def fuse_supergroups(
5353
_inline_nested_functions(model)
5454
onnx_ir.passes.common.RemoveUnusedNodesPass().call(model)
5555
_rename_supergroup_nodes(model)
56+
# Note: ORT does not correctly dispatch using function.overload, workaround by fusing to domain
57+
ir_utils.fold_overload_into_op_domain(model)
5658

5759
return model
5860

@@ -64,7 +66,7 @@ def _inline_nested_functions(model: onnx_ir.Model):
6466
node.op_identifier()
6567
for func in model.functions.values()
6668
for node in func.graph.all_nodes()
67-
if node.domain == AIMET_SUPERGROUP_DOMAIN
69+
if ir_utils.is_fused_supergroup(node)
6870
)
6971
# Note: To get around name mangling of nested functions by InlinePass, sort functions hierarchically (outermost first)
7072
ir_utils._sort_functions_hierarchically(model) # pylint: disable=protected-access
@@ -77,9 +79,7 @@ def _rename_supergroup_nodes(model: onnx_ir.Model):
7779
"""Rename supergroup nodes to have more meaningful names derived from their function body, if possible."""
7880
node_names = {node.name for node in model.graph.all_nodes()}
7981
supergroup_nodes = [
80-
node
81-
for node in model.graph.all_nodes()
82-
if node.domain == AIMET_SUPERGROUP_DOMAIN
82+
node for node in model.graph.all_nodes() if ir_utils.is_fused_supergroup(node)
8383
]
8484
root_to_nodes = defaultdict(list)
8585

TrainingExtensions/onnx/src/python/aimet_onnx/graph_passes/fusions/ir_utils.py

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""ONNX-ir related utility functions"""
55

66
import onnx_ir
7+
import onnx
78
import numpy as np
89

910
from .fusion_registry import AIMET_SUPERGROUP_DOMAIN
@@ -64,10 +65,7 @@ def _sort_functions_hierarchically(model: onnx_ir.Model) -> None:
6465
sorted_funcs = {}
6566

6667
def node_has_impl(node: onnx_ir.Node) -> bool:
67-
return (
68-
node.domain != AIMET_SUPERGROUP_DOMAIN
69-
or node.op_identifier() in sorted_funcs
70-
)
68+
return not is_fused_supergroup(node) or node.op_identifier() in sorted_funcs
7169

7270
while True:
7371
runnable_functions = {
@@ -92,15 +90,20 @@ def node_has_impl(node: onnx_ir.Node) -> bool:
9290
def inline_all_supergroups(model: onnx_ir.Model) -> None:
9391
"""Inline all aimet supergroup functions, restoring original node and value names."""
9492
supergroup_functions = {
95-
func
96-
for func in model.functions.values()
97-
if func.domain == AIMET_SUPERGROUP_DOMAIN
93+
func for func in model.functions.values() if is_fused_supergroup(func)
9894
}
9995
if not supergroup_functions:
10096
return
10197

10298
_sort_functions_hierarchically(model)
10399
onnx_ir.passes.common.InlinePass(lambda f: f in supergroup_functions).call(model)
100+
supergroup_opsets = [
101+
opset
102+
for opset in model.graph.opset_imports
103+
if opset.startswith(AIMET_SUPERGROUP_DOMAIN)
104+
]
105+
for name in supergroup_opsets:
106+
model.graph.opset_imports.pop(name)
104107

105108

106109
def unique_name(base: str, existing: set[str]) -> str:
@@ -120,3 +123,34 @@ def get_upstream_cast_type(value: onnx_ir.Value) -> int | None:
120123
return None
121124
to_attr = producer.attributes.get("to")
122125
return to_attr.as_int() if to_attr is not None else None
126+
127+
128+
def fold_overload_into_op_domain(model: onnx_ir.Model):
129+
"""
130+
Works around bug in onnxruntime dispatch to overloaded functions by replacing
131+
function.domain with "{function.domain}.{function.overload}" for all supergroup ops
132+
"""
133+
new_functions = {}
134+
for function in model.functions.values():
135+
if function.domain == AIMET_SUPERGROUP_DOMAIN and function.overload:
136+
function.domain = f"{function.domain}.{function.overload}"
137+
new_functions[function.identifier()] = function
138+
139+
for node in model.graph.all_nodes():
140+
if node.domain != AIMET_SUPERGROUP_DOMAIN:
141+
continue
142+
if not node.overload:
143+
continue
144+
node.domain = f"{node.domain}.{node.overload}"
145+
146+
model._functions = new_functions # pylint:disable = protected-access
147+
opsets = set(f.domain for f in model.functions.values() if is_fused_supergroup(f))
148+
for opset in opsets:
149+
model.opset_imports[opset] = 1
150+
151+
152+
def is_fused_supergroup(
153+
node: onnx_ir.Node | onnx_ir.Function | onnx.NodeProto | onnx.FunctionProto,
154+
) -> bool:
155+
"""Return True if ``node`` represents an AIMET supergroup op."""
156+
return node.domain.startswith(AIMET_SUPERGROUP_DOMAIN)

TrainingExtensions/onnx/src/python/aimet_onnx/quantsim.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@
109109
OrtInferenceSession,
110110
)
111111
from aimet_onnx.graph_passes.fusions import (
112-
AIMET_SUPERGROUP_DOMAIN,
113112
inline_all_supergroups,
113+
is_fused_supergroup,
114114
)
115115
from aimet_onnx.batch_norm_fold import _has_unfolded_batchnorms
116116
import aimet_onnx
@@ -786,8 +786,8 @@ def _is_tensor_quantizable(self, name: str) -> bool:
786786
return False
787787

788788
if all(
789-
(consumer.domain, consumer.op_type)
790-
== (AIMET_SUPERGROUP_DOMAIN, "MaskedSoftmax")
789+
consumer.op_type == "MaskedSoftmax"
790+
and is_fused_supergroup(consumer)
791791
and name == consumer.input[2]
792792
for consumer in consumer_nodes
793793
):
@@ -1954,8 +1954,7 @@ def export(
19541954
with self._remove_quantization_nodes():
19551955
ir_model = onnx_ir.from_proto(self.model.model)
19561956
if any(
1957-
node.domain == AIMET_SUPERGROUP_DOMAIN
1958-
for node in ir_model.graph.all_nodes()
1957+
is_fused_supergroup(node) for node in ir_model.graph.all_nodes()
19591958
):
19601959
inline_all_supergroups(ir_model)
19611960

@@ -2565,10 +2564,7 @@ def _to_onnx_qdq(
25652564

25662565
# Note: Must inline supergroups before version conversion, since version_converter does not handle functions
25672566
ir_model = onnx_ir.from_proto(model_copy)
2568-
if any(
2569-
node.domain == AIMET_SUPERGROUP_DOMAIN
2570-
for node in ir_model.graph.all_nodes()
2571-
):
2567+
if any(is_fused_supergroup(node) for node in ir_model.graph.all_nodes()):
25722568
inline_all_supergroups(ir_model)
25732569
model_copy = onnx_ir.to_proto(ir_model)
25742570

TrainingExtensions/onnx/test/python/models/models_for_tests.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5432,3 +5432,37 @@ def matmul_add_with_shared_root_naming():
54325432
)
54335433
onnx.checker.check_model(model)
54345434
return model
5435+
5436+
5437+
def model_with_transposed_and_non_transposed_gemm():
5438+
class Model(torch.nn.Module):
5439+
"""
5440+
W -> Transpose -----v W2 -----v
5441+
x --------------> MatMul -> Add -> MatMul -> Add
5442+
"""
5443+
5444+
def __init__(self):
5445+
super().__init__()
5446+
self.linear = torch.nn.Linear(in_features=10, out_features=5)
5447+
self.weight_2 = torch.nn.Parameter(torch.ones(5, 3))
5448+
self.bias_2 = torch.nn.Parameter(torch.ones(3))
5449+
5450+
def forward(self, x):
5451+
return self.linear(x) @ self.weight_2 + self.bias_2
5452+
5453+
model = Model()
5454+
x = torch.randn(1, 1, 10)
5455+
buffer = io.BytesIO()
5456+
torch.onnx.export(
5457+
model,
5458+
(x,),
5459+
buffer,
5460+
input_names=["input"],
5461+
output_names=["output"],
5462+
opset_version=20,
5463+
optimize=False, # prevent folding transB
5464+
)
5465+
buffer.seek(0)
5466+
model = load_model(buffer)
5467+
onnx.checker.check_model(model)
5468+
return model

TrainingExtensions/onnx/test/python/test_graph_passes/test_fusions.py

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
from aimet_onnx.utils import make_dummy_input, get_node_attribute
1717
from aimet_onnx import QuantizationSimModel
1818

19-
from aimet_onnx.graph_passes.fusions import fuse_supergroups, inline_all_supergroups
20-
from aimet_onnx.graph_passes.fusions.fusion_registry import AIMET_SUPERGROUP_DOMAIN
19+
from aimet_onnx.graph_passes.fusions import (
20+
fuse_supergroups,
21+
inline_all_supergroups,
22+
is_fused_supergroup,
23+
)
2124
from ..models import models_for_tests, test_models
2225
from ..models.test_models import rmsnorm_model
2326

@@ -365,6 +368,10 @@ class TestMatmulAddFusion:
365368
@pytest.mark.parametrize(
366369
"model_factory, expected_matches",
367370
[
371+
(
372+
lambda path: models_for_tests.model_with_transposed_and_non_transposed_gemm(),
373+
2,
374+
),
368375
(lambda path: models_for_tests.matmul_bias_add_model(bias_first=True), 1),
369376
(lambda path: models_for_tests.matmul_bias_add_model(bias_first=False), 1),
370377
(lambda path: models_for_tests.matmul_add_model(), 0), # Not a bias add
@@ -416,7 +423,7 @@ def test_fuses_matmul_add(self, tmp_path, model_factory, expected_matches):
416423
supergroups = [
417424
node
418425
for node in model_proto.graph.node
419-
if node.op_type == "Gemm" and node.domain == "aimet.supergroup"
426+
if node.op_type == "Gemm" and is_fused_supergroup(node)
420427
]
421428
assert len(supergroups) == expected_matches
422429

@@ -445,7 +452,7 @@ def test_transb_attribute_with_transpose(self):
445452
gemm_nodes = [
446453
node
447454
for node in model_proto.graph.node
448-
if node.op_type == "Gemm" and node.domain == "aimet.supergroup"
455+
if node.op_type == "Gemm" and is_fused_supergroup(node)
449456
]
450457
assert len(gemm_nodes) == 1
451458

@@ -468,7 +475,7 @@ def test_transb_attribute_without_transpose(self):
468475
gemm_nodes = [
469476
node
470477
for node in model_proto.graph.node
471-
if node.op_type == "Gemm" and node.domain == "aimet.supergroup"
478+
if node.op_type == "Gemm" and is_fused_supergroup(node)
472479
]
473480
assert len(gemm_nodes) == 1
474481

@@ -790,14 +797,10 @@ def test_fuse_then_inline_restores_original_names(self, tmp_path, model_factory)
790797

791798
# No supergroup functions remain
792799
assert not any(
793-
func.domain == AIMET_SUPERGROUP_DOMAIN
794-
for func in ir_model.functions.values()
800+
is_fused_supergroup(func) for func in ir_model.functions.values()
795801
)
796802

797-
assert not any(
798-
node.domain == AIMET_SUPERGROUP_DOMAIN
799-
for node in ir_model.graph.all_nodes()
800-
)
803+
assert not any(is_fused_supergroup(node) for node in ir_model.graph.all_nodes())
801804

802805
# Original non-Constant names are fully restored
803806
final_node_names = [
@@ -856,9 +859,7 @@ def test_conventional_naming_derives_module_name(
856859
model = fuse_supergroups(model, patterns=ALL_PATTERNS)
857860

858861
supergroup_names = {
859-
node.name
860-
for node in model.graph.all_nodes()
861-
if node.domain == AIMET_SUPERGROUP_DOMAIN
862+
node.name for node in model.graph.all_nodes() if is_fused_supergroup(node)
862863
}
863864
assert supergroup_names == expected_names
864865

@@ -881,7 +882,7 @@ def test_non_conventional_naming_keeps_default_with_unique_names(
881882
fuse_supergroups(model, patterns=ALL_PATTERNS)
882883

883884
for node in model.graph.all_nodes():
884-
if node.domain == AIMET_SUPERGROUP_DOMAIN:
885+
if is_fused_supergroup(node):
885886
assert node.name
886887

887888
all_names = [node.name for node in model.graph.all_nodes()]

TrainingExtensions/onnx/test/python/test_quantsim.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102
identity_tree,
103103
back_to_back_qdq_pairs,
104104
)
105-
from aimet_onnx.graph_passes.fusions import fuse_supergroups, AIMET_SUPERGROUP_DOMAIN
105+
from aimet_onnx.graph_passes.fusions import fuse_supergroups, is_fused_supergroup
106106

107107
CPU_PROVIDERS = ["CPUExecutionProvider"]
108108
CUDA_PROVIDERS = ["CUDAExecutionProvider", "CPUExecutionProvider"]
@@ -155,7 +155,7 @@ def _get_tensor_dtypes(model: onnx.ModelProto):
155155

156156

157157
def _has_aimet_supergroups(model: onnx.ModelProto):
158-
return any(node.domain == AIMET_SUPERGROUP_DOMAIN for node in model.graph.node)
158+
return any(is_fused_supergroup(node) for node in model.graph.node)
159159

160160

161161
def _fuse_all_supergroups(model: onnx.ModelProto):
@@ -7749,6 +7749,7 @@ def forward(self, x, y):
77497749
@pytest.mark.parametrize(
77507750
"model_factory",
77517751
[
7752+
lambda _: models_for_tests.model_with_transposed_and_non_transposed_gemm(),
77527753
models_for_tests.llama_rmsnorm_model,
77537754
models_for_tests.rmsnorm_model,
77547755
partial(models_for_tests.rmsnorm_model, elementwise_affine=False),
@@ -7777,7 +7778,7 @@ def test_e2e_quantsim_with_fused_supergroups(model_factory, tmp_path, providers)
77777778
supergroup_bias = set()
77787779
supergroup_nodes = []
77797780
for node in model.graph.node:
7780-
if node.domain != AIMET_SUPERGROUP_DOMAIN:
7781+
if not is_fused_supergroup(node):
77817782
continue
77827783
supergroup_nodes.append(node)
77837784
if node.op_type in ("Gemm", "LayerNormalization"):

0 commit comments

Comments
 (0)