Skip to content

Commit bfee593

Browse files
authored
Add back inliner to version converter (#2813)
Basically, version converter only supports inlined model. In terms of torch onnx exporter, we should just remind/warn users that if their models includes local-function. We just don't support it. series of version converter PRs: #2791 (Removed assertion of no local function included) #2799 (Missing opset import) #2806 (Raise for RefAttr) Maybe we should just try revert #2791? cc @gramalingam @justinchuby
1 parent e6f79e1 commit bfee593

File tree

2 files changed

+11
-5
lines changed

2 files changed

+11
-5
lines changed

onnxscript/version_converter/__init__.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ def __init__(self, target_version: int, fallback: bool = False) -> None:
3737
super().__init__()
3838
self.target_version = target_version
3939
self.fallback = fallback
40-
self._convert_pass = _ConvertVersionPass(
40+
# NOTE: The current version converter only supports inlined models.
41+
self._inline_pass = common_passes.InlinePass()
42+
self._convert_pass = _ConvertVersionPassRequiresInline(
4143
target_version=target_version,
4244
fallback=fallback,
4345
)
@@ -51,15 +53,16 @@ def call(self, model: ir.Model) -> ir.passes.PassResult:
5153
# Run the conversion pass outside of Sequential so that errors
5254
# (e.g. VersionConverterError) propagate directly without being
5355
# wrapped in PassError.
54-
result = self._convert_pass(model)
55-
cleanup_result = self._cleanup_passes(result)
56+
inline_result = self._inline_pass(model)
57+
result = self._convert_pass(inline_result.model)
58+
cleanup_result = self._cleanup_passes(result.model)
5659
return ir.passes.PassResult(
5760
cleanup_result.model,
58-
result.modified or cleanup_result.modified,
61+
result.modified or cleanup_result.modified or inline_result.modified,
5962
)
6063

6164

62-
class _ConvertVersionPass(ir.passes.InPlacePass):
65+
class _ConvertVersionPassRequiresInline(ir.passes.InPlacePass):
6366
"""Convert the model to the specified ONNX opset version.
6467
6568
This pass leverages the onnxscript version converter to convert the model. If

onnxscript/version_converter/_version_converter_test.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ def test_version_convert_gridsample_cubic(self):
208208
self.assertEqual(model.graph.node(4).version, 20)
209209
self.assertEqual(model.graph.node(4).attributes["mode"].value, "cubic")
210210

211+
@pytest.mark.xfail(reason="Version converter does not currently support local-function.")
211212
def test_version_convert_function_nodes(self):
212213
"""Test that version converter processes nodes inside model functions."""
213214
model = ir.from_onnx_text(
@@ -253,6 +254,7 @@ def test_version_convert_function_nodes(self):
253254
self.assertEqual(func[3].version, 20)
254255
self.assertEqual(len(func[3].inputs), 3) # DFT 19->20 adds dft_length input
255256

257+
@pytest.mark.xfail(reason="Version converter does not currently support local-function.")
256258
def test_version_convert_function_with_control_flow_subgraph(self):
257259
"""Test that version converter processes subgraphs inside control flow nodes in functions."""
258260
model = ir.from_onnx_text(
@@ -455,6 +457,7 @@ def test_metadata_is_copied_to_multiple_replacement_nodes(self):
455457
f"Node {i} ({node.op_type}) should have metadata copied",
456458
)
457459

460+
@pytest.mark.xfail(reason="Version converter does not currently support local-function.")
458461
def test_version_convert_raises_on_function_node_with_ref_attribute(self):
459462
"""Test that version conversion raises when a function contains a node with a ref attribute."""
460463
# Build a function with a LeakyRelu node that uses a RefAttr for 'alpha'

0 commit comments

Comments
 (0)