Skip to content

Commit

Permalink
Add test case for unsupported input-variable reuse in multi-output pa…
Browse files Browse the repository at this point in the history
…ttern matcher (#1731)

The multi-output pattern does not support a useful case, where the
pattern's input variables are reused elsewhere.

This PR adds a test-case illustrating the scenario. I was hoping that
some changes in the in-progress PR #1636 by Xavier might fix this. But I
don't think it does. Adding the test-case for now. Need to figure out
how to support this.

---------

Co-authored-by: Justin Chu <[email protected]>
  • Loading branch information
gramalingam and justinchuby authored Jul 16, 2024
1 parent 581e998 commit fb7dea4
Showing 1 changed file with 35 additions and 0 deletions.
35 changes: 35 additions & 0 deletions onnxscript/rewriter/generic_pattern_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,41 @@ def apply_pattern(op, x, **_):
self.assertEqual(len(graph.node), 2)
self.assertEqual(graph.node[0].op_type, "SinCos")

@unittest.skip("Input variable reuse not supported yet")
def test_shared_root_value_extra_use(self):
def match_pattern(op, x):
t1 = op.Sin(x)
t2 = op.Cos(x)
return t1, t2

def apply_pattern(op, x, **_):
return op.SinCos(x, domain="com.microsoft", outputs=2)

rule = pattern.RewriteRule(
match_pattern,
apply_pattern,
matcher=generic_pattern.GenericPatternMatcher,
)
model_proto = onnx.parser.parse_model(
"""
<ir_version: 7, opset_import: [ "" : 17]>
agraph (float[N] y) => (float[N] z)
{
temp1 = Sin(y)
temp2 = Cos(y)
w = Add(temp1, temp2)
z = Mul(w, y)
}
"""
)
onnx.checker.check_model(model_proto)
model = onnx.shape_inference.infer_shapes(model_proto)
ir_model = ir.serde.deserialize_model(model)
rule.apply_to_model(ir_model)
graph = ir_model.graph
self.assertEqual(len(graph), 3)
self.assertEqual(graph.node[0].op_type, "SinCos")

def test_rotary_embedding(self):
# The test work on a model if it has the expected name.
# A dummy model is used if not present (not implemented yet).
Expand Down

0 comments on commit fb7dea4

Please sign in to comment.