-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TFLite] Support quantized GREATER op in TFLite frontend #11519
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
0ecaa0d
to
c5ea612
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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. |
@lhutton1 can you have another look on this one? |
I believe its still waiting on an update, @dchauhan-arm please let me know if that's not the case |
cc @lhutton1 for a second (second) look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for persisting with this @dchauhan-arm, the quantized graph we are testing looks better now! Just a small nit, but otherwise LGTM :)
hmmm looks like there's a seg fault happening in |
This is now unblocked by #12738 - @dchauhan-arm can you remove the conflict and resubmit this for CI to check? |
4b1339d
to
1d32c40
Compare
Support GREATER quantization operation conversion as part of issue apache#9187 Continuation of apache#11519
Support GREATER quantization operation conversion as part of issue apache#9187 Continuation of apache#11519.
Support GREATER quantization operation conversion as part of issue #9187