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/04/14 11:10:31 UTC
[GitHub] [tvm] lhutton1 opened a new pull request, #11010: [microNPU] Match requantize in min/max with activation pattern
lhutton1 opened a new pull request, #11010:
URL: https://github.com/apache/tvm/pull/11010
Optimizes a corner case where min/max + clip also produces a re-quantize operation. Previously the re-quantize was lowered separately as an identity operation which is unnecessary. Now the quantization parameters from re-quantize will be used by the lowered min/max operation.
cc @ekalda @NicolaLancellotti @manupa-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] manupa-arm commented on pull request #11010: [microNPU] Match requantize in min/max with activation pattern
Posted by GitBox <gi...@apache.org>.
manupa-arm commented on PR #11010:
URL: https://github.com/apache/tvm/pull/11010#issuecomment-1116328110
Thanks @lhutton1 @NicolaLancellotti @ekalda !
--
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] lhutton1 commented on a diff in pull request #11010: [microNPU] Match requantize in min/max with activation pattern
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #11010:
URL: https://github.com/apache/tvm/pull/11010#discussion_r853163602
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -637,6 +637,18 @@ def verify(ext_func):
if activation_function == "RELU":
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
Review Comment:
Good catch, will add :)
##########
python/tvm/relay/op/contrib/ethosu.py:
##########
@@ -618,19 +618,28 @@ class BinaryElementwiseParams:
and extract the parameter information.
"""
- def __init__(self, func_body: Call, operator_type: str, has_quantization_parameters: bool):
+ def __init__(self, func_body: Call, operator_type: str, is_quantized_operation: bool):
from tvm.relay.backend.contrib.ethosu.util import BinaryElementwiseArgs
+ from tvm.relay.backend.contrib.ethosu.util import RequantArgs
+ current_call = func_body
clip = None
- if str(func_body.op) == "clip":
- clip = func_body
- binary_op = clip.args[0]
+ requantize = None
+
+ if is_quantized_operation:
+ if str(current_call.op) == "clip":
+ clip = current_call
+ current_call = clip.args[0]
else:
- binary_op = func_body
+ if str(current_call.op) == "qnn.requantize":
+ requantize = current_call
+ clip = current_call.args[0]
+ current_call = clip.args[0]
Review Comment:
Due to the pattern there should always be a clip here whenever there is a re-quantize. Strictly speaking I think it is possible to have min/max -> requantize, but these will be offloaded as separate operations, so the condition here won't be met. I think this case is another one that would fall under the responsibility of the identity optimizer pass.
--
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 #11010: [microNPU] Match requantize in min/max with activation pattern
Posted by GitBox <gi...@apache.org>.
ekalda commented on code in PR #11010:
URL: https://github.com/apache/tvm/pull/11010#discussion_r853088956
##########
python/tvm/relay/op/contrib/ethosu.py:
##########
@@ -618,19 +618,28 @@ class BinaryElementwiseParams:
and extract the parameter information.
"""
- def __init__(self, func_body: Call, operator_type: str, has_quantization_parameters: bool):
+ def __init__(self, func_body: Call, operator_type: str, is_quantized_operation: bool):
from tvm.relay.backend.contrib.ethosu.util import BinaryElementwiseArgs
+ from tvm.relay.backend.contrib.ethosu.util import RequantArgs
+ current_call = func_body
clip = None
- if str(func_body.op) == "clip":
- clip = func_body
- binary_op = clip.args[0]
+ requantize = None
+
+ if is_quantized_operation:
+ if str(current_call.op) == "clip":
+ clip = current_call
+ current_call = clip.args[0]
else:
- binary_op = func_body
+ if str(current_call.op) == "qnn.requantize":
+ requantize = current_call
+ clip = current_call.args[0]
+ current_call = clip.args[0]
Review Comment:
Is there always a clip between requantize and binary op?
##########
tests/python/contrib/test_ethosu/test_legalize.py:
##########
@@ -637,6 +637,18 @@ def verify(ext_func):
if activation_function == "RELU":
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
Review Comment:
A bit unlikely, but to be on the safe side, maybe it's worth setting the seed in the test to make sure we don't end up with these values as a result of the random numbers we generate?
--
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] manupa-arm merged pull request #11010: [microNPU] Match requantize in min/max with activation pattern
Posted by GitBox <gi...@apache.org>.
manupa-arm merged PR #11010:
URL: https://github.com/apache/tvm/pull/11010
--
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] lhutton1 commented on pull request #11010: [microNPU] Match requantize in min/max with activation pattern
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #11010:
URL: https://github.com/apache/tvm/pull/11010#issuecomment-1115881515
friendly ping @manupa-arm, when you get some time would you be able to take a look at this?
--
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