You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/06/15 15:06:18 UTC
[GitHub] [tvm] ashutosh-arm opened a new pull request, #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
ashutosh-arm opened a new pull request, #11732:
URL: https://github.com/apache/tvm/pull/11732
This commit is to fix issues in CMSIS-NN passes
that surface when the same operand is repeated
in QNN binary ops.
--
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
[GitHub] [tvm] ashutosh-arm commented on pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11732:
URL: https://github.com/apache/tvm/pull/11732#issuecomment-1157388698
cc: @Mousius @grant-arm @manupa-arm for code review.
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899281864
##########
tests/python/contrib/test_cmsisnn/test_binary_ops.py:
##########
@@ -145,6 +145,58 @@ def test_op_int8(
)
+@skip_if_no_reference_system
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("op", [relay.qnn.op.mul, relay.qnn.op.add])
+@pytest.mark.parametrize("relu_type", ["RELU", "NONE"])
+def test_same_input_to_binary_op(op, relu_type):
+ """Tests QNN binary operator for CMSIS-NN where both inputs are the same"""
+ interface_api = "c"
+ use_unpacked_api = True
+ test_runner = AOT_USMP_CORSTONE300_RUNNER
+
+ dtype = "int8"
+ shape = [1, 16, 16, 3]
+ input_ = generate_variable("input")
+ input_scale = 0.256
+ input_zero_point = 33
+
+ model = make_model(
+ op,
+ input_,
+ input_,
+ input_scale,
+ input_zero_point,
+ input_scale,
+ input_zero_point,
+ relu_type,
+ )
+ orig_mod = make_module(model)
+
+ cmsisnn_mod = cmsisnn.partition_for_cmsisnn(orig_mod)
+
+ # validate pattern matching
+ assert_partitioned_function(orig_mod, cmsisnn_mod)
+
+ # validate the output
+ in_min, in_max = get_range_for_dtype_str(dtype)
+ inputs = {
+ "input": np.random.randint(in_min, high=in_max, size=shape, dtype=dtype),
+ }
+ output_list = generate_ref_data(orig_mod["main"], inputs)
+ compile_and_run(
Review Comment:
I have added a check for this. Thanks @Mousius.
--
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
[GitHub] [tvm] ashutosh-arm commented on pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on PR #11732:
URL: https://github.com/apache/tvm/pull/11732#issuecomment-1158843111
Thanks @Mousius for the discussions and code review. Would you like to take a look at it again?
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899108072
##########
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py:
##########
@@ -256,6 +256,47 @@ def test_all_primary_operands_tensor_constants():
assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
+@tvm.testing.requires_cmsisnn
+def test_duplicate_constant_arguments():
+ """Tests the pass when repeating operands are arguments to the binary op"""
+ dtype = "int8"
+ shape = (1, 3, 3, 32)
+ operand0 = generate_variable("operand0", shape, dtype)
+ operand1 = generate_variable("operand0", shape, dtype)
Review Comment:
Good catch. Thanks!
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899106018
##########
python/tvm/relay/op/contrib/cmsisnn.py:
##########
@@ -229,17 +229,18 @@ def qnn_max_pool2d_pattern():
def check_qnn_max_pool2d(pattern):
"""Check if max pool2d is supported by CMSIS-NN."""
output = pattern
- input_var = _find_last(pattern)
+ input_op = None
Review Comment:
ACK
--
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
[GitHub] [tvm] Mousius commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
Mousius commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899042991
##########
tests/python/contrib/test_cmsisnn/test_binary_ops.py:
##########
@@ -145,6 +145,58 @@ def test_op_int8(
)
+@skip_if_no_reference_system
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("op", [relay.qnn.op.mul, relay.qnn.op.add])
+@pytest.mark.parametrize("relu_type", ["RELU", "NONE"])
+def test_same_input_to_binary_op(op, relu_type):
+ """Tests QNN binary operator for CMSIS-NN where both inputs are the same"""
+ interface_api = "c"
+ use_unpacked_api = True
+ test_runner = AOT_USMP_CORSTONE300_RUNNER
+
+ dtype = "int8"
+ shape = [1, 16, 16, 3]
+ input_ = generate_variable("input")
+ input_scale = 0.256
+ input_zero_point = 33
+
+ model = make_model(
+ op,
+ input_,
+ input_,
+ input_scale,
+ input_zero_point,
+ input_scale,
+ input_zero_point,
+ relu_type,
+ )
+ orig_mod = make_module(model)
+
+ cmsisnn_mod = cmsisnn.partition_for_cmsisnn(orig_mod)
+
+ # validate pattern matching
+ assert_partitioned_function(orig_mod, cmsisnn_mod)
+
+ # validate the output
+ in_min, in_max = get_range_for_dtype_str(dtype)
+ inputs = {
+ "input": np.random.randint(in_min, high=in_max, size=shape, dtype=dtype),
+ }
+ output_list = generate_ref_data(orig_mod["main"], inputs)
+ compile_and_run(
Review Comment:
Should we not test that the internal functions only have 1 parameter each?
##########
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py:
##########
@@ -256,6 +256,47 @@ def test_all_primary_operands_tensor_constants():
assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
+@tvm.testing.requires_cmsisnn
+def test_duplicate_constant_arguments():
+ """Tests the pass when repeating operands are arguments to the binary op"""
+ dtype = "int8"
+ shape = (1, 3, 3, 32)
+ operand0 = generate_variable("operand0", shape, dtype)
+ operand1 = generate_variable("operand0", shape, dtype)
Review Comment:
```suggestion
operand0 = generate_variable("operand0", shape, dtype)
operand1 = generate_variable("operand1", shape, dtype)
```
##########
python/tvm/relay/op/contrib/cmsisnn.py:
##########
@@ -229,17 +229,18 @@ def qnn_max_pool2d_pattern():
def check_qnn_max_pool2d(pattern):
"""Check if max pool2d is supported by CMSIS-NN."""
output = pattern
- input_var = _find_last(pattern)
+ input_op = None
Review Comment:
We can remove this as it's always set later.
##########
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py:
##########
@@ -256,6 +256,47 @@ def test_all_primary_operands_tensor_constants():
assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
+@tvm.testing.requires_cmsisnn
+def test_duplicate_constant_arguments():
+ """Tests the pass when repeating operands are arguments to the binary op"""
+ dtype = "int8"
+ shape = (1, 3, 3, 32)
+ operand0 = generate_variable("operand0", shape, dtype)
+ operand1 = generate_variable("operand0", shape, dtype)
+ binary_op = make_binary_op(
+ relay.qnn.op.add,
+ operand0,
+ operand0,
+ input_0_scale=0.0128,
+ input_0_zero_point=32,
+ input_1_scale=0.256,
+ input_1_zero_point=-64,
+ )
+
+ local_func = relay.Function([operand0, operand1], binary_op, relay.TensorType(shape, dtype))
+ local_func = set_composite_func_attr(local_func, "cmsis-nn.qnn_add")
+
+ rng = np.random.default_rng(12345)
+ arg0 = relay.const(rng.integers(-128, high=127, size=shape, dtype=dtype))
+ call_local_func = relay.Call(local_func, [arg0, arg0])
+ extern_func = relay.Function([], call_local_func, relay.TensorType(shape, dtype))
+
+ global_var = relay.GlobalVar("external_function")
+ extern_func = set_external_func_attr(extern_func, "cmsis-nn", global_var.name_hint)
+ call_extern_func = relay.Call(global_var, [])
+ main_func = relay.Function([], call_extern_func, relay.TensorType(shape, dtype))
+ main_var = relay.GlobalVar("main")
+
+ mod = tvm.IRModule()
+ mod[global_var] = extern_func
+ mod[main_var] = main_func
+
+ mod = relay.transform.InferType()(mod)
+ mod = ScalarToTensorConstants()(mod)
+ new_mod = relay.transform.InferType()(mod)
+ assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
Review Comment:
What is this checking? It appears to just check the body hasn't changed after `InferType`?
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899109224
##########
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py:
##########
@@ -256,6 +256,47 @@ def test_all_primary_operands_tensor_constants():
assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
+@tvm.testing.requires_cmsisnn
+def test_duplicate_constant_arguments():
+ """Tests the pass when repeating operands are arguments to the binary op"""
+ dtype = "int8"
+ shape = (1, 3, 3, 32)
+ operand0 = generate_variable("operand0", shape, dtype)
+ operand1 = generate_variable("operand0", shape, dtype)
+ binary_op = make_binary_op(
+ relay.qnn.op.add,
+ operand0,
+ operand0,
+ input_0_scale=0.0128,
+ input_0_zero_point=32,
+ input_1_scale=0.256,
+ input_1_zero_point=-64,
+ )
+
+ local_func = relay.Function([operand0, operand1], binary_op, relay.TensorType(shape, dtype))
+ local_func = set_composite_func_attr(local_func, "cmsis-nn.qnn_add")
+
+ rng = np.random.default_rng(12345)
+ arg0 = relay.const(rng.integers(-128, high=127, size=shape, dtype=dtype))
+ call_local_func = relay.Call(local_func, [arg0, arg0])
+ extern_func = relay.Function([], call_local_func, relay.TensorType(shape, dtype))
+
+ global_var = relay.GlobalVar("external_function")
+ extern_func = set_external_func_attr(extern_func, "cmsis-nn", global_var.name_hint)
+ call_extern_func = relay.Call(global_var, [])
+ main_func = relay.Function([], call_extern_func, relay.TensorType(shape, dtype))
+ main_var = relay.GlobalVar("main")
+
+ mod = tvm.IRModule()
+ mod[global_var] = extern_func
+ mod[main_var] = main_func
+
+ mod = relay.transform.InferType()(mod)
+ mod = ScalarToTensorConstants()(mod)
+ new_mod = relay.transform.InferType()(mod)
+ assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
Review Comment:
Before this commit, it was producing a different body in the end due to an error in the code. Since the relay model in this test does not contain any scalar constants, expectation is that the pass should not affect the graph.
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899109224
##########
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py:
##########
@@ -256,6 +256,47 @@ def test_all_primary_operands_tensor_constants():
assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
+@tvm.testing.requires_cmsisnn
+def test_duplicate_constant_arguments():
+ """Tests the pass when repeating operands are arguments to the binary op"""
+ dtype = "int8"
+ shape = (1, 3, 3, 32)
+ operand0 = generate_variable("operand0", shape, dtype)
+ operand1 = generate_variable("operand0", shape, dtype)
+ binary_op = make_binary_op(
+ relay.qnn.op.add,
+ operand0,
+ operand0,
+ input_0_scale=0.0128,
+ input_0_zero_point=32,
+ input_1_scale=0.256,
+ input_1_zero_point=-64,
+ )
+
+ local_func = relay.Function([operand0, operand1], binary_op, relay.TensorType(shape, dtype))
+ local_func = set_composite_func_attr(local_func, "cmsis-nn.qnn_add")
+
+ rng = np.random.default_rng(12345)
+ arg0 = relay.const(rng.integers(-128, high=127, size=shape, dtype=dtype))
+ call_local_func = relay.Call(local_func, [arg0, arg0])
+ extern_func = relay.Function([], call_local_func, relay.TensorType(shape, dtype))
+
+ global_var = relay.GlobalVar("external_function")
+ extern_func = set_external_func_attr(extern_func, "cmsis-nn", global_var.name_hint)
+ call_extern_func = relay.Call(global_var, [])
+ main_func = relay.Function([], call_extern_func, relay.TensorType(shape, dtype))
+ main_var = relay.GlobalVar("main")
+
+ mod = tvm.IRModule()
+ mod[global_var] = extern_func
+ mod[main_var] = main_func
+
+ mod = relay.transform.InferType()(mod)
+ mod = ScalarToTensorConstants()(mod)
+ new_mod = relay.transform.InferType()(mod)
+ assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
Review Comment:
Before this commit, it was producing a different body in the end due to an error in the code.
--
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
[GitHub] [tvm] Mousius commented on pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #11732:
URL: https://github.com/apache/tvm/pull/11732#issuecomment-1159054901
Thanks @ashutosh-arm 😸
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899281864
##########
tests/python/contrib/test_cmsisnn/test_binary_ops.py:
##########
@@ -145,6 +145,58 @@ def test_op_int8(
)
+@skip_if_no_reference_system
+@tvm.testing.requires_cmsisnn
+@pytest.mark.parametrize("op", [relay.qnn.op.mul, relay.qnn.op.add])
+@pytest.mark.parametrize("relu_type", ["RELU", "NONE"])
+def test_same_input_to_binary_op(op, relu_type):
+ """Tests QNN binary operator for CMSIS-NN where both inputs are the same"""
+ interface_api = "c"
+ use_unpacked_api = True
+ test_runner = AOT_USMP_CORSTONE300_RUNNER
+
+ dtype = "int8"
+ shape = [1, 16, 16, 3]
+ input_ = generate_variable("input")
+ input_scale = 0.256
+ input_zero_point = 33
+
+ model = make_model(
+ op,
+ input_,
+ input_,
+ input_scale,
+ input_zero_point,
+ input_scale,
+ input_zero_point,
+ relu_type,
+ )
+ orig_mod = make_module(model)
+
+ cmsisnn_mod = cmsisnn.partition_for_cmsisnn(orig_mod)
+
+ # validate pattern matching
+ assert_partitioned_function(orig_mod, cmsisnn_mod)
+
+ # validate the output
+ in_min, in_max = get_range_for_dtype_str(dtype)
+ inputs = {
+ "input": np.random.randint(in_min, high=in_max, size=shape, dtype=dtype),
+ }
+ output_list = generate_ref_data(orig_mod["main"], inputs)
+ compile_and_run(
Review Comment:
I have added a check for this above in L182. Thanks @Mousius.
--
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
[GitHub] [tvm] Mousius merged pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
Mousius merged PR #11732:
URL: https://github.com/apache/tvm/pull/11732
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #11732: [CMSIS-NN] Fixed the case with repeating operands in the QNN binary ops
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #11732:
URL: https://github.com/apache/tvm/pull/11732#discussion_r899281322
##########
tests/python/contrib/test_cmsisnn/test_scalar_to_tensor_constant.py:
##########
@@ -256,6 +256,47 @@ def test_all_primary_operands_tensor_constants():
assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
+@tvm.testing.requires_cmsisnn
+def test_duplicate_constant_arguments():
+ """Tests the pass when repeating operands are arguments to the binary op"""
+ dtype = "int8"
+ shape = (1, 3, 3, 32)
+ operand0 = generate_variable("operand0", shape, dtype)
+ operand1 = generate_variable("operand0", shape, dtype)
+ binary_op = make_binary_op(
+ relay.qnn.op.add,
+ operand0,
+ operand0,
+ input_0_scale=0.0128,
+ input_0_zero_point=32,
+ input_1_scale=0.256,
+ input_1_zero_point=-64,
+ )
+
+ local_func = relay.Function([operand0, operand1], binary_op, relay.TensorType(shape, dtype))
+ local_func = set_composite_func_attr(local_func, "cmsis-nn.qnn_add")
+
+ rng = np.random.default_rng(12345)
+ arg0 = relay.const(rng.integers(-128, high=127, size=shape, dtype=dtype))
+ call_local_func = relay.Call(local_func, [arg0, arg0])
+ extern_func = relay.Function([], call_local_func, relay.TensorType(shape, dtype))
+
+ global_var = relay.GlobalVar("external_function")
+ extern_func = set_external_func_attr(extern_func, "cmsis-nn", global_var.name_hint)
+ call_extern_func = relay.Call(global_var, [])
+ main_func = relay.Function([], call_extern_func, relay.TensorType(shape, dtype))
+ main_var = relay.GlobalVar("main")
+
+ mod = tvm.IRModule()
+ mod[global_var] = extern_func
+ mod[main_var] = main_func
+
+ mod = relay.transform.InferType()(mod)
+ mod = ScalarToTensorConstants()(mod)
+ new_mod = relay.transform.InferType()(mod)
+ assert tvm.ir.structural_equal(mod[global_var].body, new_mod[global_var].body)
Review Comment:
From offline discussion it was decided to leave the check for structural equality in there.
--
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