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/09 00:19:22 UTC
[GitHub] [tvm] sfvaroglu opened a new pull request, #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
sfvaroglu opened a new pull request, #10952:
URL: https://github.com/apache/tvm/pull/10952
We are seeing some models with 1-element vectors for input_scale and input_zero_point in `qnn.conv2d_transpose`. This PR adds support to handle those shapes.
--
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] sfvaroglu commented on pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
sfvaroglu commented on PR #10952:
URL: https://github.com/apache/tvm/pull/10952#issuecomment-1093637113
@AndrewZhaoLuo @anwang2009 please 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] AndrewZhaoLuo commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r848863908
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -91,12 +91,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
Review Comment:
subtract should already have numpy style broadcasting so I'm not sure you need to replace the things here,
##########
src/relay/qnn/op/convolution_transpose.cc:
##########
@@ -107,12 +107,22 @@ bool QnnConv2DTransposeRel(const Array<Type>& types, int num_inputs, const Attrs
return false;
}
}
- ICHECK(IsScalarType(types[2], DataType::Int(32))); // input_zero_point
const auto* weight_zp_type = types[3].as<TensorTypeNode>();
ICHECK(weight_zp_type->dtype == DataType::Int(32)); // weight_zero_point
- ICHECK(IsScalarType(types[4], DataType::Float(32))); // input_scale
Review Comment:
This appears to be the main change
--
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] AndrewZhaoLuo commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852437203
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -91,12 +91,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
Review Comment:
I think the preferable way is to use `relay.broadcast_to_like` or `relay.broadcast_to`. It looks like bias_add `include/tvm/topi/nn/bias_add.h` makes some additional assumptions about the input shape.
--
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] sfvaroglu commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
sfvaroglu commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852475648
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -91,12 +91,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
Review Comment:
I tried it but got "incompatible broadcast type" [32] --> [32, 32, 3, 3] down the line. I was actually looking at how this was handled in `helper_no_fast_int8_hw_legalization` and it seems `biad_add` is the preferred way?
--
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] sfvaroglu commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
sfvaroglu commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852397609
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -91,12 +91,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
Review Comment:
Without this change, the model I'm looking at fails with `Incompatible broadcast type TensorType([32, 32, 3, 3], int16) and TensorType([32], int16)`. Here is the op for more info:
```
%2181 = qnn.conv2d_transpose(%2180, meta[relay.Constant][1003] /* ty=Tensor[(32, 32, 3, 3), int8] */, -26 /* ty=int32 */, meta[relay.Constant][1004] /* ty=Tensor[(32), int32] */, 0.196949f /* ty=float32 */, meta[relay.Constant][1005] /* ty=Tensor[(32), float32] */, channels=32, kernel_size=[3, 3], strides=[2, 2], padding=[0, 0, 1, 1], kernel_layout="IOHW", out_dtype="int32") /* ty=Tensor[(1, 32, 100, 100), int32] */;
```
--
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] AndrewZhaoLuo merged pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo merged PR #10952:
URL: https://github.com/apache/tvm/pull/10952
--
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] AndrewZhaoLuo commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852476644
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -91,12 +91,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
Review Comment:
Hmm ok. that's weird
--
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] AndrewZhaoLuo commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852434189
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -91,12 +91,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
Review Comment:
Ah I see yeah that should be correct, simple arithmetic should follow generic broadcasting rules: https://numpy.org/doc/stable/user/basics.broadcasting.html#general-broadcasting-rules.
--
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] sfvaroglu commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
sfvaroglu commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852479246
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -92,12 +92,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
- relay.cast(data, dtype="int16"), relay.cast(input_zero_point, "int16")
- )
- shift_kernel = relay.subtract(
- relay.cast(kernel, dtype="int16"), relay.cast(kernel_zero_point, "int16")
- )
+ # If input zero point is a scalar, we can directly subtract it.
+ if len(types[2].shape) == 0:
Review Comment:
It doesn't work for both cases. Only needed for the else branch.
--
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] AndrewZhaoLuo commented on a diff in pull request #10952: [QNN] Support input scale and zp of 1-element vector in qnn.conv2d_transpose
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on code in PR #10952:
URL: https://github.com/apache/tvm/pull/10952#discussion_r852476788
##########
python/tvm/relay/qnn/op/legalizations.py:
##########
@@ -92,12 +92,30 @@ def qnn_conv2d_transpose_legalize(attrs, inputs, types):
# Collect the input exprs.
data, kernel, input_zero_point, kernel_zero_point, _, _ = inputs
- shift_data = relay.subtract(
- relay.cast(data, dtype="int16"), relay.cast(input_zero_point, "int16")
- )
- shift_kernel = relay.subtract(
- relay.cast(kernel, dtype="int16"), relay.cast(kernel_zero_point, "int16")
- )
+ # If input zero point is a scalar, we can directly subtract it.
+ if len(types[2].shape) == 0:
Review Comment:
Do you need bias add for the else branch or can you use it in both cases?
--
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