[GitHub] [tvm] KJlaccHoeUM9l commented on a diff in pull request #13866: [ONNX] Support SequenceEmpty op

["KJlaccHoeUM9l (via GitHub)" <gi...@apache.org>]

KJlaccHoeUM9l commented on code in PR #13866:
URL: https://github.com/apache/tvm/pull/13866#discussion_r1090841600


##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -6148,13 +6148,13 @@ def _impl_v11(cls, inputs, attr, params):
         return _expr.Tuple(inputs)
 
 
-class SequenceLength(OnnxOpConverter):
-    """Operator converter for sequence length op."""
+class SequenceEmpty(OnnxOpConverter):
+    """Operator converter for sequence empty op."""
 
     @classmethod
     def _impl_v11(cls, inputs, attr, params):
-        # Get length of input sequence
-        return _expr.const(len(inputs[0]), dtype="int64")
+        # Construct an empty tuple.
+        return _expr.Tuple([])

Review Comment:
   Looks like `dtype` attribute is not supported ([link](https://github.com/onnx/onnx/blob/main/docs/Operators.md#SequenceEmpty)).



##########
tests/python/frontend/onnx/test_forward.py:
##########
@@ -7821,6 +7821,38 @@ def verify_sequence_ops(tensor_shape, num_tensors, axis=0, position=0, new_axis=
     verify_sequence_ops((3, 3, 3, 3), 4, axis=2, new_axis=1)
 
 
+@tvm.testing.parametrize_targets
+def test_empty_sequence(target, dev):
+    """test_empty_sequence"""
+
+    # Test creating an empty tensor sequence.
+    empty_node = helper.make_node(
+        "SequenceEmpty",
+        inputs=[],
+        outputs=["empty_sequence"],
+    )
+
+    length_node = helper.make_node("SequenceLength", inputs=["empty_sequence"], outputs=["output"])
+
+    graph_outputs = [helper.make_tensor_value_info("output", TensorProto.INT64, [])]
+
+    graph_nodes = [empty_node, length_node]
+
+    graph = helper.make_graph(
+        graph_nodes,
+        "Sequence_test",
+        inputs=[],
+        outputs=graph_outputs,
+    )
+
+    model = helper.make_model(
+        graph,
+        producer_name="Sequence_empty_test",
+    )
+
+    verify_with_ort_with_inputs(model, [], target=target, dev=dev)

Review Comment:
   It looks like `verify_with_ort_with_inputs` passes because it use `float32` as the default value for `dtype` parameter, which is the same as the default value of `dtype` attribute in `SequenceEmpty`.
   However, it looks like using `verify_with_ort_with_inputs` doesn't do anything, because inside this test, iteration over the elements of the output tensor occurs, and in this case it is empty, so iteration does not occur and the test is "always green".
   Probably you can not use this test (iteration over an empty tensor). It is enough to check the data types described in the documentation in the loop.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org