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/07/08 16:04:31 UTC
[GitHub] [tvm] leandron opened a new pull request, #12042: [TFLite] Enable int64 biases for int16 quantized operators
leandron opened a new pull request, #12042:
URL: https://github.com/apache/tvm/pull/12042
This enables int64 biases for quantized fully connected, requantize and transpose convolution in TFLite networks. It goes on top of existing int16 support for TFLite frontend.
I'm trying to find publicly available networks to increase test coverage in a follow-up patch.
cc @areusch for reviews
--
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] leandron commented on pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
leandron commented on PR #12042:
URL: https://github.com/apache/tvm/pull/12042#issuecomment-1309099200
Please have a another look.
--
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] leandron commented on pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
leandron commented on PR #12042:
URL: https://github.com/apache/tvm/pull/12042#issuecomment-1307482657
> Hello @leandron,
>
> I'm working on similar lines & have a model with conv2d_transpose & all the other ops are already supported from your already merged commit. I've made the same changes you've done for conv2d_transpose from this patch, but the dequantize layer at the end is getting int64 input which isn't right. Am I missing something that needs to be changed?
>
> Thanks in advance!
In TFlite as of now, biases are set by default to be int64 when int16 quantisation is used.
I have [this model](https://github.com/ARM-software/ML-zoo/blob/48f458af1e9065d9aad2ad94d24b58d6e7c00817/models/keyword_spotting/ds_cnn_small/tflite_int16/ds_cnn_quantized.tflite) which was created using the [default int16 flow](https://www.tensorflow.org/lite/performance/post_training_integer_quant_16x8), and can be used to check these internal data types with e.g. Netron
--
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 #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #12042:
URL: https://github.com/apache/tvm/pull/12042#issuecomment-1308764032
<!---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-docs-start-->
* Built docs for commit 1846d0019db0e341b056131a714931ccdd0c345c can be found [here](https://pr-docs.tlcpack.ai/PR-12042/5/docs/index.html).<!--bot-comment-docs-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
[GitHub] [tvm] Mousius merged pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
Mousius merged PR #12042:
URL: https://github.com/apache/tvm/pull/12042
--
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] leandron commented on a diff in pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
leandron commented on code in PR #12042:
URL: https://github.com/apache/tvm/pull/12042#discussion_r1018973953
##########
tests/python/contrib/test_ethosn/test_convert_equivalents.py:
##########
@@ -227,7 +227,7 @@ def expected():
@requires_ethosn
@pytest.mark.parametrize(
"dtype,shape,constant_shape",
- [("int16", (1, 16, 12, 4), None)],
+ [("float32", (1, 16, 12, 4), None)],
Review Comment:
The use of `int16` here was just to create an invalid test case, that raises an exception. With `int16` now supported, then using `float32` will make that effect.
--
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] kuladeep2706 commented on pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
kuladeep2706 commented on PR #12042:
URL: https://github.com/apache/tvm/pull/12042#issuecomment-1271529666
Hello @leandron,
I'm working on similar lines & have a model with conv2d_transpose & all the other ops are already supported from your already merged commit. I've made the same changes you've done for conv2d_transpose from this patch, but the dequantize layer at the end is getting int64 input which isn't right. Am I missing something that needs to be changed?
Thanks in advance!
--
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 #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #12042:
URL: https://github.com/apache/tvm/pull/12042#discussion_r1018896087
##########
src/relay/qnn/op/dense.cc:
##########
@@ -47,12 +47,14 @@ bool QnnDenseRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
if (data == nullptr || weight == nullptr) return false;
const auto* param = attrs.as<DenseAttrs>();
ICHECK(param != nullptr) << "DenseAttrs cannot be nullptr.";
- ICHECK(data->dtype == DataType::Int(8) || data->dtype == DataType::UInt(8))
- << "Expected quantized dense type(int8, uint8) for input but was " << data->dtype;
+ ICHECK(data->dtype == DataType::Int(8) || data->dtype == DataType::UInt(8) ||
+ data->dtype == DataType::Int(16) || data->dtype == DataType::UInt(16))
+ << "Expected quantized dense type(int8, uint8, int16, uint16) for input but was "
+ << data->dtype;
ICHECK(weight->dtype == DataType::Int(8) || weight->dtype == DataType::UInt(8))
<< "Expected quantized dense type(int8, uint8) for weight but was " << weight->dtype;
- ICHECK(param->out_dtype == DataType::Int(32))
- << "Expected quantized dense type(int32) for output but was " << param->out_dtype;
+ ICHECK(param->out_dtype == DataType::Int(32) || param->out_dtype == DataType::Int(64))
+ << "Expected quantized dense type(int32, int64) for output but was " << param->out_dtype;
Review Comment:
Do we need additional dtype checks for bias as well based on output dtype? If yes, then probably we should check the QNN Conv2D type checker too.
##########
tests/python/contrib/test_ethosn/test_convert_equivalents.py:
##########
@@ -227,7 +227,7 @@ def expected():
@requires_ethosn
@pytest.mark.parametrize(
"dtype,shape,constant_shape",
- [("int16", (1, 16, 12, 4), None)],
+ [("float32", (1, 16, 12, 4), None)],
Review Comment:
Just curious: why the dtype 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] leandron commented on a diff in pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
leandron commented on code in PR #12042:
URL: https://github.com/apache/tvm/pull/12042#discussion_r1018987074
##########
src/relay/qnn/op/dense.cc:
##########
@@ -47,12 +47,14 @@ bool QnnDenseRel(const Array<Type>& types, int num_inputs, const Attrs& attrs,
if (data == nullptr || weight == nullptr) return false;
const auto* param = attrs.as<DenseAttrs>();
ICHECK(param != nullptr) << "DenseAttrs cannot be nullptr.";
- ICHECK(data->dtype == DataType::Int(8) || data->dtype == DataType::UInt(8))
- << "Expected quantized dense type(int8, uint8) for input but was " << data->dtype;
+ ICHECK(data->dtype == DataType::Int(8) || data->dtype == DataType::UInt(8) ||
+ data->dtype == DataType::Int(16) || data->dtype == DataType::UInt(16))
+ << "Expected quantized dense type(int8, uint8, int16, uint16) for input but was "
+ << data->dtype;
ICHECK(weight->dtype == DataType::Int(8) || weight->dtype == DataType::UInt(8))
<< "Expected quantized dense type(int8, uint8) for weight but was " << weight->dtype;
- ICHECK(param->out_dtype == DataType::Int(32))
- << "Expected quantized dense type(int32) for output but was " << param->out_dtype;
+ ICHECK(param->out_dtype == DataType::Int(32) || param->out_dtype == DataType::Int(64))
+ << "Expected quantized dense type(int32, int64) for output but was " << param->out_dtype;
Review Comment:
Based on the investigation of TFLite int16 networks, the output here will be within the checks. Internally, there will be a quantise/requantise just after, so that it will set the appropriate output dtype we see in the model, which is `int16`.
I double checked the model (the one in the test case in the PR) after reading your comment. Also, Conv2D was similarly covered by https://github.com/apache/tvm/pull/10915.
--
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 #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #12042:
URL: https://github.com/apache/tvm/pull/12042#discussion_r1018975218
##########
tests/python/contrib/test_ethosn/test_convert_equivalents.py:
##########
@@ -227,7 +227,7 @@ def expected():
@requires_ethosn
@pytest.mark.parametrize(
"dtype,shape,constant_shape",
- [("int16", (1, 16, 12, 4), None)],
+ [("float32", (1, 16, 12, 4), None)],
Review Comment:
Got it. 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] Mousius commented on pull request #12042: [TFLite] Enable int64 biases for int16 quantized operators
Posted by GitBox <gi...@apache.org>.
Mousius commented on PR #12042:
URL: https://github.com/apache/tvm/pull/12042#issuecomment-1315111601
Sorry for the delay - thanks @leandron 😸
--
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