FrozenGene commented on issue #4282: Add test for the qnn_add operator
URL: https://github.com/apache/incubator-tvm/pull/4282#issuecomment-552069295
 
 
   > If I understand tolerance correctly, I think atol of 1 is good for 
integers and current implementation. Let me know if I am wrong.
   > 
   > For integers, either you say match exactly, or if you want to have a 
tolerance, it can be minimum 1. There is nothing in between. Currently, as 
there is a difference between TFLite and TVM rounding, there is bound to be 
some differences and the minimum difference can be 1.
   > 
   > This PR makes a big jump in improving the testing infrastructure for 
TFLite quantized networks and enabling unittests. So, I think this is a very 
good addition. The atol is an artifact of implementation which we can solve 
separately.
   > 
   > @FrozenGene Will you be willing to create a separate RFC for implementing 
TFLite rounding in TVM? I don't think we should stall this PR.
   
   @anijain2305  I only concerned we will let our wrong implementation off. For 
example, we implement one operator XXX, we follow the test infra setting abs 
tolerance be 1. However, our implementation of XXX is wrong and only different 
1 between correct result. This is my real concern before, not pointing to this 
pr.
   
   I would like to implement TFLite rounding next and wish we could change the 
tolerance after that. We could let this PR in firstly :-)
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to