You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "p3achyjr (via GitHub)" <gi...@apache.org> on 2023/09/30 02:42:15 UTC
[GitHub] [tvm] p3achyjr opened a new pull request, #15844: [TFLite][Frontend] Support quantized floor_mod
p3achyjr opened a new pull request, #15844:
URL: https://github.com/apache/tvm/pull/15844
Tests were failing b/c TVM and TFLite disagree on div-by-zero cases.
floor_mod still fails, this is probably a behaviorial difference.
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "p3achyjr (via GitHub)" <gi...@apache.org>.
p3achyjr commented on code in PR #15844:
URL: https://github.com/apache/tvm/pull/15844#discussion_r1346036159
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2916,11 +2929,6 @@ def test_all_elemwise():
_test_forward_elemwise(_test_floor_divide)
_test_forward_elemwise_quantized(_test_floor_divide)
_test_forward_elemwise(_test_floor_mod)
- # This test of quantized floor mod is currently disabled due
Review Comment:
added back--sorry about that
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on code in PR #15844:
URL: https://github.com/apache/tvm/pull/15844#discussion_r1343143062
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2480,6 +2481,16 @@ def __test_elemwise(in_data):
inq0_min, inq0_max = (out_min, out_max)
inq1_min, inq1_max = (out_min, out_max)
+ if exclude_zero_point:
+ if inq1_max == inq1_min:
+ raise ZeroDivisionError("Input range is 0.")
+
+ # only compute for rhs.
+ quant_scale = 255 / (inq1_max - inq1_min)
+ zero_point = int(round(-inq1_min * quant_scale))
+ data[1][data[1] == zero_point] += 1
+ data[1][data[1] == 0] += 1
Review Comment:
Is this second adjustment required? Looking at the two next two each other, its seems like the first (when `data[1] == zero_point`) would only be required when these are pre-quantized values, and the second (when `data[1] == 0`) would only be required when these are post-quantization values, and that only one of those can be true at a time.
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "p3achyjr (via GitHub)" <gi...@apache.org>.
p3achyjr commented on PR #15844:
URL: https://github.com/apache/tvm/pull/15844#issuecomment-1753189968
@Lunderberg hi! sorry to bug. is this PR still needed?
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on code in PR #15844:
URL: https://github.com/apache/tvm/pull/15844#discussion_r1344178068
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2480,6 +2481,16 @@ def __test_elemwise(in_data):
inq0_min, inq0_max = (out_min, out_max)
inq1_min, inq1_max = (out_min, out_max)
+ if exclude_zero_point:
+ if inq1_max == inq1_min:
+ raise ZeroDivisionError("Input range is 0.")
+
+ # only compute for rhs.
+ quant_scale = 255 / (inq1_max - inq1_min)
+ zero_point = int(round(-inq1_min * quant_scale))
+ data[1][data[1] == zero_point] += 1
+ data[1][data[1] == 0] += 1
Review Comment:
Interesting, and thank you for checking. Possibly due to tests that quantize one or the other argument, leading `data` to be used in both contexts.
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on code in PR #15844:
URL: https://github.com/apache/tvm/pull/15844#discussion_r1344176674
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2916,11 +2929,6 @@ def test_all_elemwise():
_test_forward_elemwise(_test_floor_divide)
_test_forward_elemwise_quantized(_test_floor_divide)
_test_forward_elemwise(_test_floor_mod)
- # This test of quantized floor mod is currently disabled due
Review Comment:
Should we keep this explanation, since we presumably want to resolve this issue at some point?
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #15844:
URL: https://github.com/apache/tvm/pull/15844#issuecomment-1755560175
@p3achyjr Thank you for the reminder, and it would still be good to get this PR in, even if re-enabling the flaky `floor_mod` test is saved for a follow-up 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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg merged PR #15844:
URL: https://github.com/apache/tvm/pull/15844
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "Aleksei-grovety (via GitHub)" <gi...@apache.org>.
Aleksei-grovety commented on code in PR #15844:
URL: https://github.com/apache/tvm/pull/15844#discussion_r1352800533
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2480,6 +2481,16 @@ def __test_elemwise(in_data):
inq0_min, inq0_max = (out_min, out_max)
inq1_min, inq1_max = (out_min, out_max)
+ if exclude_zero_point:
+ if inq1_max == inq1_min:
+ raise ZeroDivisionError("Input range is 0.")
+
+ # only compute for rhs.
+ quant_scale = 255 / (inq1_max - inq1_min)
Review Comment:
Shouldn't there be a dependency on the data type for the value 255? Since the data type can be int8, int16.
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "p3achyjr (via GitHub)" <gi...@apache.org>.
p3achyjr commented on PR #15844:
URL: https://github.com/apache/tvm/pull/15844#issuecomment-1755646894
@Lunderberg thanks for getting back!!
--
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
Re: [PR] [TFLite][Frontend] Fix test failures caused by div-by-zero [tvm]
Posted by "p3achyjr (via GitHub)" <gi...@apache.org>.
p3achyjr commented on code in PR #15844:
URL: https://github.com/apache/tvm/pull/15844#discussion_r1343153900
##########
tests/python/frontend/tflite/test_forward.py:
##########
@@ -2480,6 +2481,16 @@ def __test_elemwise(in_data):
inq0_min, inq0_max = (out_min, out_max)
inq1_min, inq1_max = (out_min, out_max)
+ if exclude_zero_point:
+ if inq1_max == inq1_min:
+ raise ZeroDivisionError("Input range is 0.")
+
+ # only compute for rhs.
+ quant_scale = 255 / (inq1_max - inq1_min)
+ zero_point = int(round(-inq1_min * quant_scale))
+ data[1][data[1] == zero_point] += 1
+ data[1][data[1] == 0] += 1
Review Comment:
I agree--but when I was testing, I needed to keep the second adjustment for all tests to pass.
--
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