[GitHub] [tvm] AndrewZhaoLuo commented on a diff in pull request #13578: [FQ2I] Support converting `dense` -> `add` to `qnn.dense` -> `add` -> `requantize`

[GitBox <gi...@apache.org>]

AndrewZhaoLuo commented on code in PR #13578:
URL: https://github.com/apache/tvm/pull/13578#discussion_r1043831311


##########
python/tvm/relay/transform/fake_quantization_to_integer.py:
##########
@@ -502,6 +502,33 @@ def register_binary_qnn(op_name, op):
 
     def binary(expr, type_map):
         left, right, left_t, right_t, out_t = get_binary_types(expr, type_map)
+
+        if (
+            approx_equal(left_t.scale, right_t.scale)

Review Comment:
   Hmm I don't believe this work around is a good idea because it does not handle overflow well where qnn ops will. 
   
   e.g. if we had 128 * 128 --> we will have issues.  
   
   Hmm the main problem we have if I understand correctly is dealing with scales and zero points which are of shape [1] rather than be scalars. 
   
   Can we just unpack scales and zero points into scalars then?



##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -1391,7 +1391,7 @@ def _impl_v1(cls, inputs, attr, params):
         dtype = input0_state.checked_type.dtype
         # Y = alpha * A * B + beta * C
         alpha = float(attr.get("alpha", 1.0))
-        beta = float(attr.get("beta", 1.0))
+        beta = float(attr.get("beta"))

Review Comment:
   Though the change on L1409 might not be needed since if beta == 1, it can be removed with constant folding.



##########
python/tvm/relay/frontend/onnx.py:
##########
@@ -1391,7 +1391,7 @@ def _impl_v1(cls, inputs, attr, params):
         dtype = input0_state.checked_type.dtype
         # Y = alpha * A * B + beta * C
         alpha = float(attr.get("alpha", 1.0))
-        beta = float(attr.get("beta", 1.0))
+        beta = float(attr.get("beta"))

Review Comment:
   I would keep the original line of `.get('beta', 1.0)` since you cannot call `float()` on `None` which `attr.get` can return.
   
   Then below on L1409, you can just do `if beta is None` --> `if 'beta' not in attr.keys()` or something.



-- 
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