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/01 08:53:41 UTC

[GitHub] [tvm] dchauhan-arm opened a new pull request, #11519: [TFLite] Support quantized GREATER op in TFLite frontend

dchauhan-arm opened a new pull request, #11519:
URL: https://github.com/apache/tvm/pull/11519

   Support GREATER quantization operation conversion as part of issue #9187


-- 
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] dchauhan-arm commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
dchauhan-arm commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1189920748

   cc @lhutton1 for a second (second) 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] dchauhan-arm commented on a diff in pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
dchauhan-arm commented on code in PR #11519:
URL: https://github.com/apache/tvm/pull/11519#discussion_r889995981


##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2486,9 +2492,9 @@ def test_all_elemwise():
     _test_forward_elemwise_quantized(_test_maximum)
     _test_forward_elemwise(_test_minimum)
     _test_forward_elemwise_quantized(_test_minimum)
-    _test_forward_elemwise(_test_greater)
     _test_forward_elemwise(_test_squared_difference)
     _test_forward_elemwise(_test_greater_equal)
+    _test_forward_elemwise(_test_greater)

Review Comment:
   That is correct, got a patch in the pipes to get that in, a couple functions need tweaking for comparison operators like these.



-- 
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] NicolaLancellotti commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
NicolaLancellotti commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1195634974

   With tflite 2.9.1 `test_elemwise[_test_greater]` does not fail any more. We should wait for #12130 and #12131 to be merged.


-- 
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 #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
leandron commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1188923037

   @lhutton1 can you have another look on this one?


-- 
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] dchauhan-arm closed pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
dchauhan-arm closed pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend
URL: https://github.com/apache/tvm/pull/11519


-- 
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] dchauhan-arm commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
dchauhan-arm commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1168801437

   > @dchauhan-arm just a friendly ping to update this PR with quantised testing for the operator.
   
   yes, what I think I'll do is rewrite a test specifically for the greater op, and compare output of tvm against tflite. 


-- 
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] dchauhan-arm commented on a diff in pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
dchauhan-arm commented on code in PR #11519:
URL: https://github.com/apache/tvm/pull/11519#discussion_r925358461


##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2480,26 +2493,15 @@ def _test_forward_elemwise(testop):
     )
 
 
-def _test_forward_elemwise_quantized(testop):
-    if testop is not _test_equal:
-        testop(
-            [
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.uint8),
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.uint8),
-            ],
-            quantized=True,
-            qnn_op=testop,
-        )
-    else:
-        # no need for fake_quant to hold tensors in float32 until conversion
-        testop(
-            [
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.float32),
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.float32),
-            ],
-            quantized=True,
-            qnn_op=testop,
-        )
+def _test_forward_elemwise_quantized(testop, comparison_op=False):

Review Comment:
   good spot, 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] leandron commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
leandron commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1242128747

   This is now unblocked by https://github.com/apache/tvm/pull/12738 - @dchauhan-arm can you remove the conflict and resubmit this for CI to check?


-- 
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] lhutton1 commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1189000078

   I believe its still waiting on an update, @dchauhan-arm please let me know if that's not the case


-- 
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] lhutton1 commented on a diff in pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #11519:
URL: https://github.com/apache/tvm/pull/11519#discussion_r889967832


##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2486,9 +2492,9 @@ def test_all_elemwise():
     _test_forward_elemwise_quantized(_test_maximum)
     _test_forward_elemwise(_test_minimum)
     _test_forward_elemwise_quantized(_test_minimum)
-    _test_forward_elemwise(_test_greater)
     _test_forward_elemwise(_test_squared_difference)
     _test_forward_elemwise(_test_greater_equal)
+    _test_forward_elemwise(_test_greater)

Review Comment:
   Do we also need a call to `_test_forward_elemwise_quantized` to test the quantized version of "greater"?



-- 
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] lhutton1 commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1162984925

   If I inspect the Relay graph(s) that is tested when running `_test_forward_elemwise_quantized(_test_greater, True)`, it seems as though it doesn't get quantized in any way e.g.
   ```
   def @main(%inq_0: Tensor[(3, 6), float32] /* ty=Tensor[(3, 6), float32] */, %inq_1: Tensor[(3, 6), float32] /* ty=Tensor[(3, 6), float32] */, output_tensor_names=["Greater"]) -> Tensor[(3, 6), bool] {
     greater(%inq_0, %inq_1) /* ty=Tensor[(3, 6), bool] */
   }
   ```
   So then it seems we're still only testing the float32 implementation of the operator? For a quantized operator I'd expect int/uint inputs being passed to the `greater` op - maybe I missed 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


[GitHub] [tvm] dchauhan-arm commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
dchauhan-arm commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1162871276

   cc @leandron and @lhutton1 for a second 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] lhutton1 commented on a diff in pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #11519:
URL: https://github.com/apache/tvm/pull/11519#discussion_r925348956


##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2480,26 +2493,15 @@ def _test_forward_elemwise(testop):
     )
 
 
-def _test_forward_elemwise_quantized(testop):
-    if testop is not _test_equal:
-        testop(
-            [
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.uint8),
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.uint8),
-            ],
-            quantized=True,
-            qnn_op=testop,
-        )
-    else:
-        # no need for fake_quant to hold tensors in float32 until conversion
-        testop(
-            [
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.float32),
-                np.array(np.random.uniform(0, 255, (3, 6)), dtype=np.float32),
-            ],
-            quantized=True,
-            qnn_op=testop,
-        )
+def _test_forward_elemwise_quantized(testop, comparison_op=False):

Review Comment:
   Nit: looks like `comparison_op` here is not used, can we remove it?



-- 
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] lhutton1 commented on pull request #11519: [TFLite] Support quantized GREATER op in TFLite frontend

Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #11519:
URL: https://github.com/apache/tvm/pull/11519#issuecomment-1190544017

   hmmm looks like there's a seg fault happening in `test_elemwise[_test_greater]` on aarch64 CI, so maybe there is an issue with this version of tensorflow on aarch64


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