Open Silvan-K opened 3 years ago
Yes, it will lead to incorrect result if activation is quantized symmetrically, which is not a common case for ORT now. The fix is to: disable symmetric quant for the input of Relu/Clip or add a quantized op for Relu/Clip.
Ok, thanks @yufenglee for confirming! I just wanted to stress that this is not just an issue with symmetric quantization. Both symmetric and asymmetric quantization modes will give wrong results when signed int8 is used. It might be worth adding an error message when signed int8 is used and Relu is included in ops_types_to_quantize
so that other users of the code don't unknowingly run into this issue.
Yes, it will lead to incorrect result if activation is quantized symmetrically, which is not a common case for ORT now. The fix is to: disable symmetric quant for the input of Relu/Clip or add a quantized op for Relu/Clip.
@yufenglee - do you have a sense of when this could be fixed?
Thanks for verifying!
Hey @yufenglee , we dug a bit deeper and discovered that the problem was just a mismatch of the minimum/maximum values of the signed int8 data type. onnxruntime assumes [-127,+127], while our architecture provides [-128,+127]. If we make this change in onnxruntime and use asymmetric quantization, then we do get the correct results. Would you be open to making this switchable in onnxruntime?
@SilvanK4t1qbit, yes, it'd better to change the range to [-128, 127] for activation, but for weight, we need to keep it as [-127, 127]. You're welcome to make a PR to fix this.
This issue has been automatically marked as stale due to inactivity and will be closed in 7 days if no further activity occurs. If further support is needed, please provide an update and/or more details.
Yes, it will lead to incorrect result if activation is quantized symmetrically, which is not a common case for ORT now. The fix is to: disable symmetric quant for the input of Relu/Clip or add a quantized op for Relu/Clip.
When symmetric quant and keep relu,Onnxruntime still cannot run normally because relu does not support int8
Describe the bug It seems as if Relu nodes that immediately follow Conv nodes are getting dropped during quantization (if included in ops_to_quantize). If I understand things correctly, then this should still give correct results if unsigned int8 is used for quantization because the quantization parameters for the output of the QLinearConv node are determined from the output of the Relu that follows the Conv node in the original model (negative values would then just get clipped to zero). But if signed int8 is used, then negative values don't get clipped anymore and the absence of the Relu leads to wrong results. Using symmetrized activation also leads to wrong results, even if unsigned int8 is used (demonstrated in the attached script).
The solution for my specific use case could be to exclude Relu from the ops_to_quantize argument. Omitting the Relu from ops_to_quantize has the unwanted side effect that a Maxpool following the Relu doesn't get quantized anymore, however (separate issue: #9428).
Urgency
Development of a backend is blocked by this, so it would be great if someone could provide some insights as soon as possible.
System information
To Reproduce Run code below