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 2023/01/12 14:27:39 UTC
[GitHub] [tvm] Alexey-Yazev opened a new pull request, #13772: [microNPU] Add hardware constraints for binary elementwise
Alexey-Yazev opened a new pull request, #13772:
URL: https://github.com/apache/tvm/pull/13772
Does not fuse min and max operations with requantize if there are different scales as it is not supported on NPU. Since there are hardware constraints, we cannot perform min or max operation fused with requantize (please look at NPU_SET_OFM_SCALE register description https://developer.arm.com/documentation/102420/0200/Programmers-model/Command-stream/cmd1-commands-) when we have different scales.
min/max operations with matching scales are offloaded to NPU as ethosu_binary_elementwise
min/max operations with different scales are offloaded to NPU as ethosu_binary_elementwise + ethosu_identity.
cc @leandron, @ekalda, @lhutton1
--
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] ekalda commented on a diff in pull request #13772: [microNPU] Add hardware constraints for binary elementwise
Posted by GitBox <gi...@apache.org>.
ekalda commented on code in PR #13772:
URL: https://github.com/apache/tvm/pull/13772#discussion_r1073653209
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -951,20 +962,30 @@ def verify(ext_func):
assert op.checked_type.dtype == dtype
assert op.attrs.operator_type == operator_type
assert op.attrs.reversed_operands == reversed_operands
- if activation_function == "RELU":
+ if activation_function != None:
assert str(op.attrs.activation) == "CLIP"
if operator_type in ["MIN", "MAX"]:
- # MIN and MAX with an activation must have a requantize operation
- # baked into the output. To check the extra requantize node was
- # picked up by the pattern, we can make sure the quantization
- # information is not default.
- assert float(op.attrs.ifm_scale) != 1.0
- assert int(op.attrs.ifm_zero_point) != 0
- assert float(op.attrs.ifm2_scale) != 1.0
- assert int(op.attrs.ifm2_zero_point) != 0
- assert float(op.attrs.ofm_scale) != 1.0
- assert int(op.attrs.ofm_zero_point) != 0
+ if has_separate_requantize:
+ # In case when requantize cannot be fused with MIN/MAX + CLIP due to hardware constraints
+ # there should be default quantization values since requantize is separate operation.
+ assert float(op.attrs.ifm_scale) == 1.0
+ assert int(op.attrs.ifm_zero_point) == 0
+ assert float(op.attrs.ifm2_scale) == 1.0
+ assert int(op.attrs.ifm2_zero_point) == 0
+ assert float(op.attrs.ofm_scale) == 1.0
+ assert int(op.attrs.ofm_zero_point) == 0
+ else:
+ # MIN and MAX with an activation must have a requantize operation
+ # baked into the output. To check the extra requantize node was
+ # picked up by the pattern, we can make sure the quantization
+ # information is not default.
+ assert float(op.attrs.ifm_scale) != 1.0
+ assert int(op.attrs.ifm_zero_point) != 0
+ assert float(op.attrs.ifm2_scale) != 1.0
+ assert int(op.attrs.ifm2_zero_point) != 0
+ assert float(op.attrs.ofm_scale) != 1.0
+ assert int(op.attrs.ofm_zero_point) != 0
Review Comment:
Ok cool, thanks for clarifying :)
--
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] Alexey-Yazev commented on a diff in pull request #13772: [microNPU] Add hardware constraints for binary elementwise
Posted by GitBox <gi...@apache.org>.
Alexey-Yazev commented on code in PR #13772:
URL: https://github.com/apache/tvm/pull/13772#discussion_r1071956641
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -881,7 +888,7 @@ def verify(ext_func):
([1, 4, 4], [4, 1], False),
],
)
-@pytest.mark.parametrize("activation_function", ["NONE", "RELU"])
+@pytest.mark.parametrize("activation_function", [None, tf.nn.relu, tf.nn.relu6, relu_n1_to_1])
Review Comment:
I agree with this, I will add separate test for case with MAX operation and relu_n1_to_1 activation.
--
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] ekalda merged pull request #13772: [microNPU] Add hardware constraints for binary elementwise
Posted by GitBox <gi...@apache.org>.
ekalda merged PR #13772:
URL: https://github.com/apache/tvm/pull/13772
--
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] Alexey-Yazev commented on a diff in pull request #13772: [microNPU] Add hardware constraints for binary elementwise
Posted by GitBox <gi...@apache.org>.
Alexey-Yazev commented on code in PR #13772:
URL: https://github.com/apache/tvm/pull/13772#discussion_r1071955879
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -951,20 +962,30 @@ def verify(ext_func):
assert op.checked_type.dtype == dtype
assert op.attrs.operator_type == operator_type
assert op.attrs.reversed_operands == reversed_operands
- if activation_function == "RELU":
+ if activation_function != None:
assert str(op.attrs.activation) == "CLIP"
if operator_type in ["MIN", "MAX"]:
- # MIN and MAX with an activation must have a requantize operation
- # baked into the output. To check the extra requantize node was
- # picked up by the pattern, we can make sure the quantization
- # information is not default.
- assert float(op.attrs.ifm_scale) != 1.0
- assert int(op.attrs.ifm_zero_point) != 0
- assert float(op.attrs.ifm2_scale) != 1.0
- assert int(op.attrs.ifm2_zero_point) != 0
- assert float(op.attrs.ofm_scale) != 1.0
- assert int(op.attrs.ofm_zero_point) != 0
+ if has_separate_requantize:
+ # In case when requantize cannot be fused with MIN/MAX + CLIP due to hardware constraints
+ # there should be default quantization values since requantize is separate operation.
+ assert float(op.attrs.ifm_scale) == 1.0
+ assert int(op.attrs.ifm_zero_point) == 0
+ assert float(op.attrs.ifm2_scale) == 1.0
+ assert int(op.attrs.ifm2_zero_point) == 0
+ assert float(op.attrs.ofm_scale) == 1.0
+ assert int(op.attrs.ofm_zero_point) == 0
+ else:
+ # MIN and MAX with an activation must have a requantize operation
+ # baked into the output. To check the extra requantize node was
+ # picked up by the pattern, we can make sure the quantization
+ # information is not default.
+ assert float(op.attrs.ifm_scale) != 1.0
+ assert int(op.attrs.ifm_zero_point) != 0
+ assert float(op.attrs.ifm2_scale) != 1.0
+ assert int(op.attrs.ifm2_zero_point) != 0
+ assert float(op.attrs.ofm_scale) != 1.0
+ assert int(op.attrs.ofm_zero_point) != 0
Review Comment:
Yes, both of these blocks get run, the first block is run for cases with MAX operation and relu_n1_to_1 activation for example test_tflite_binary_elemwise_legalize[relu_n1_to_1-ifm_shape0-ifm2_shape0-False-MAX]
```
fn (%tvmgen_default_ethos_u_main_0_ifms: Tensor[(48), int8] /* ty=Tensor[(48), int8] */, Inline=1, Compiler="ethos-u", global_symbol="tvmgen_default_ethos_u_main_0", Primitive=1) -> Tensor[(1, 2, 3, 4), int8] {
%0 = split(%tvmgen_default_ethos_u_main_0_ifms, indices_or_sections=[24]) /* ty=(Tensor[(24), int8], Tensor[(24), int8]) */;
%1 = %0.0 /* ty=Tensor[(24), int8] */;
%2 = %0.1 /* ty=Tensor[(24), int8] */;
%3 = reshape(%1, newshape=[1, 2, 3, 4]) /* ty=Tensor[(1, 2, 3, 4), int8] */;
%4 = reshape(%2, newshape=[1, 2, 3, 4]) /* ty=Tensor[(1, 2, 3, 4), int8] */;
%5 = contrib.ethosu.binary_elementwise(%3, %4, meta[relay.Constant][0] /* ty=Tensor[(0), int8] */, operator_type="MAX", ifm_scale=1f, ifm_zero_point=0, ifm2_scale=1f, ifm2_zero_point=0, ofm_scale=1f, ofm_zero_point=0, ifm_channels=4, ifm2_channels=4, activation="CLIP", clip_min=-128, ofm_dtype="int8") /* ty=Tensor[(1, 2, 3, 4), int8] */;
contrib.ethosu.identity(%5, meta[relay.Constant][1] /* ty=Tensor[(0), int8] */, ifm_scale=0.00783747f, ifm_zero_point=-128, ofm_scale=0.00392157f, ofm_zero_point=-128) /* ty=Tensor[(1, 2, 3, 4), int8] */
} /* ty=fn (Tensor[(48), int8]) -> Tensor[(1, 2, 3, 4), int8] */
```
in this cases the scales are different in others the same.
--
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] ekalda commented on a diff in pull request #13772: [microNPU] Add hardware constraints for binary elementwise
Posted by GitBox <gi...@apache.org>.
ekalda commented on code in PR #13772:
URL: https://github.com/apache/tvm/pull/13772#discussion_r1071276333
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -951,20 +962,30 @@ def verify(ext_func):
assert op.checked_type.dtype == dtype
assert op.attrs.operator_type == operator_type
assert op.attrs.reversed_operands == reversed_operands
- if activation_function == "RELU":
+ if activation_function != None:
assert str(op.attrs.activation) == "CLIP"
if operator_type in ["MIN", "MAX"]:
- # MIN and MAX with an activation must have a requantize operation
- # baked into the output. To check the extra requantize node was
- # picked up by the pattern, we can make sure the quantization
- # information is not default.
- assert float(op.attrs.ifm_scale) != 1.0
- assert int(op.attrs.ifm_zero_point) != 0
- assert float(op.attrs.ifm2_scale) != 1.0
- assert int(op.attrs.ifm2_zero_point) != 0
- assert float(op.attrs.ofm_scale) != 1.0
- assert int(op.attrs.ofm_zero_point) != 0
+ if has_separate_requantize:
+ # In case when requantize cannot be fused with MIN/MAX + CLIP due to hardware constraints
+ # there should be default quantization values since requantize is separate operation.
+ assert float(op.attrs.ifm_scale) == 1.0
+ assert int(op.attrs.ifm_zero_point) == 0
+ assert float(op.attrs.ifm2_scale) == 1.0
+ assert int(op.attrs.ifm2_zero_point) == 0
+ assert float(op.attrs.ofm_scale) == 1.0
+ assert int(op.attrs.ofm_zero_point) == 0
+ else:
+ # MIN and MAX with an activation must have a requantize operation
+ # baked into the output. To check the extra requantize node was
+ # picked up by the pattern, we can make sure the quantization
+ # information is not default.
+ assert float(op.attrs.ifm_scale) != 1.0
+ assert int(op.attrs.ifm_zero_point) != 0
+ assert float(op.attrs.ifm2_scale) != 1.0
+ assert int(op.attrs.ifm2_zero_point) != 0
+ assert float(op.attrs.ofm_scale) != 1.0
+ assert int(op.attrs.ofm_zero_point) != 0
Review Comment:
Do both of these blocks get run? It looks like we are using the same method of generating representative dataset (which will determine the qnn params) for all the tests, so I suspect we will always create IFMs with differing qnn params and therefore test only one of the patterns here.
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -881,7 +888,7 @@ def verify(ext_func):
([1, 4, 4], [4, 1], False),
],
)
-@pytest.mark.parametrize("activation_function", ["NONE", "RELU"])
+@pytest.mark.parametrize("activation_function", [None, tf.nn.relu, tf.nn.relu6, relu_n1_to_1])
Review Comment:
```suggestion
@pytest.mark.parametrize("activation_function", [None, tf.nn.relu])
```
Since this test looks at the graph entirely at Relay level, where all the RELUs are just Relay clip operations, I don't think we benefit much from extra 70 tests with just different min and max attributes to clip.
--
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] tvm-bot commented on pull request #13772: [microNPU] Add hardware constraints for binary elementwise
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13772:
URL: https://github.com/apache/tvm/pull/13772#issuecomment-1380438263
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @Mousius, @leandron, @lhutton1 <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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