Skip to content
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, QNN] Slice op #6217

Closed
wants to merge 1 commit into from
Closed

Conversation

anijain2305
Copy link
Contributor

@anijain2305 anijain2305 commented Aug 5, 2020

TFLite quantized slice op has same input and output qnn params. Just adding a check and a test case.

@d-smirnov Please take a look at this PR if it can help simplify - #6018 My goal was to keep the changes minimal here.

@u99127

@anijain2305
Copy link
Contributor Author

@siju-samuel Please review.

assert len(output_tensors) == 1, "There should be only 1 output tensor"
output_tensor = output_tensors[0]
assert self.has_same_qnn_params(input_tensor, output_tensor), \
"TFLite reshape requires input and output scale and zero points to be equal"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tflite reshape -> Tflite slice

Comment on lines +380 to +381
input_range = {'in_data': (-100, 100)} if quantized else None
compare_tflite_with_tvm([data], ['in_data:0'], [in_data], [out],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 2 common statements, put outside if-else block.

Copy link
Member

@siju-samuel siju-samuel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anijain2305 please followup the review comments.

@u99127
Copy link
Contributor

u99127 commented Aug 21, 2020

TFLite quantized slice op has same input and output qnn params. Just adding a check and a test case.

@d-smirnov Please take a look at this PR if it can help simplify - #6018 My goal was to keep the changes minimal here.

@u99127

LGTM modulo @siju-samuel 's comments.

@tqchen tqchen closed this Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants