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/24 05:44:54 UTC
[GitHub] [tvm] sisleyli opened a new issue, #11867: [Bug] Quantize description is inconsistent with the actual implementation
sisleyli opened a new issue, #11867:
URL: https://github.com/apache/tvm/issues/11867
There is the description of quantize in `/python/tvm/relay/qnn/op/qnn.py`
```python
def quantize(data, output_scale, output_zero_point, axis=-1, out_dtype="int8"):
r"""Quantize op
This operator takes float32 as input and produces quantized int8 or unit8 as output.
The input tensor can be of any shape. The output shape is the same as input shape.
Q_output = clamp((round(input_tensor/output_scale) + output_zero_point),
out_dtype::min,
out_dtype::max)
Parameters
```
Here it seems that the implementation is `clamp((round(input_tensor/output_scale) + output_zero_point)` ,
but in `/src/relay/qnn/op/quantize.cc`:
```C++
const int32_t min_val = GetQmin(out_dtype);
const int32_t max_val = GetQmax(out_dtype);
auto scale_data = Divide(input_tensor, expanded_output_scale);
auto add_zero_point = Add(scale_data, Cast(expanded_output_zero_point, DataType::Float(32)));
auto clamped_output = Clip(add_zero_point, min_val, max_val);
auto rounded_clamped_output = Round(clamped_output);
return Cast(rounded_clamped_output, out_dtype);
```
It seems that the actual implementation is `round(clamp((input_tensor/output_scale) + output_zero_point))`
Now I'd like to know which implementation tvm wants to implement, round first or clip first?
### Expected behavior
Quantize description is consistent with the actual implementation
### Actual behavior
Quantize description is inconsistent with the actual implementation
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] sisleyli commented on issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
sisleyli commented on issue #11867:
URL: https://github.com/apache/tvm/issues/11867#issuecomment-1165274596
>
@masahi
Yeah, python description is also more compatible with my program. For the order you mentioned, please wait for me to confirm. If there is no problem, I am sure willing to send a PR.
--
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] sisleyli commented on issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
sisleyli commented on issue #11867:
URL: https://github.com/apache/tvm/issues/11867#issuecomment-1165429667
I have sent a PR #11872 .Thanks masahi for code review. Now I close this issue.
--
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] masahi commented on issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #11867:
URL: https://github.com/apache/tvm/issues/11867#issuecomment-1165263283
I think the python description makes more sense to me. What do you think? Feel free to send a PR to fix the C++ implementation.
--
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] sisleyli commented on issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
sisleyli commented on issue #11867:
URL: https://github.com/apache/tvm/issues/11867#issuecomment-1165293216
> Actually, the order in the C++ implementation is done intentionally, see #9558
>
> cc @AndrewZhaoLuo
> This fixes this by moving the clipping operation to operate on the floating point values before casting to integer.
My focus is that `round` should be executed before `scale_data` add `expanded_output_zero_point`, right? It seems that no conflict with #9558 , which move clip before cast to integer.
--
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] masahi commented on issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #11867:
URL: https://github.com/apache/tvm/issues/11867#issuecomment-1165264955
Actually, the order in the C++ implementation is done intentionally, see https://github.com/apache/tvm/pull/9558
cc @AndrewZhaoLuo
--
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] sisleyli closed issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
sisleyli closed issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
URL: https://github.com/apache/tvm/issues/11867
--
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] masahi commented on issue #11867: [Bug] Quantize description is inconsistent with the actual implementation
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #11867:
URL: https://github.com/apache/tvm/issues/11867#issuecomment-1165307966
ah right, I was thinking about the order of round and clip, but I realized that it doesn't really matter.
--
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