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

[Frontend][TFLite] Add MIRROR_PAD operator #4822

Merged
merged 1 commit into from
Feb 7, 2020
Merged

Conversation

wyc-ruiker
Copy link
Contributor

Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers by @ them in the pull request thread.

@FrozenGene @inadob @anijain2305 @apivovarov Could you please help to review this patch.

Copy link
Member

@FrozenGene FrozenGene left a comment

Choose a reason for hiding this comment

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

LGTM.

# the quantized form MirrorPad is not yet implemented in TFLite.
if self.is_quantized(op):
raise tvm.error.OpNotImplemented(
'TFlite quantized MIRROR_PAD operator is not supported yet.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Pedantry : A not implemented error suggests to me as a user that the support exists in the tflite tooling but the TVM stack has not yet implemented the support i.e. the tflite tooling can't yet produce the mirror_pad operation, do we need a different notification here actually asking the user to report an issue asking for a feature to be added ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this exception will not be triggered when quantized MIRROR_PAD is not implemented by TFLite. If the exception is triggered, it certainly means the TVM stack doesn't support quantized MIRROR_PAD.

@FrozenGene
Copy link
Member

@u99127 @inadob Please approve explicitly if you think this PR is good now.

@inadob
Copy link
Contributor

inadob commented Feb 7, 2020

LGTM

@FrozenGene
Copy link
Member

LGTM

@inadob Could you do the approve operation as this guide? https://docs.tvm.ai/contribute/code_review.html#approve-and-request-changes-explicitly Thanks!

@FrozenGene FrozenGene merged commit 30b7d83 into apache:master Feb 7, 2020
@FrozenGene
Copy link
Member

Thanks @wyc-ruiker @inadob It is merged now.

anijain2305 pushed a commit to anijain2305/tvm that referenced this pull request Feb 10, 2020
alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 26, 2020
alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 28, 2020
zhiics pushed a commit to neo-ai/tvm that referenced this pull request Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants