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

[RFC] Make tflite frontend more data driven / improve errors. #5519

Closed
wants to merge 2 commits into from

Conversation

u99127
Copy link
Contributor

@u99127 u99127 commented May 5, 2020

This is a draft PR and only for discussion but not for merging as is.

These are a couple of commits that show a proof of concept about how we could restructure and improve the tflite frontend. I've lightly tested these by compiling a couple of tflite models to give me some confidence that they work. If this looks reasonable I'm happy to pull things out into separate individual pull requests that could allow us to migrate to this with newer operators that get added and refactor the existing operators in a piecemeal fashion with time and availability of folks.

The first commit tries to make the tflite operator table more data driven, the observation being that the common case is that of input and output operands being subject to an equality check that we can commonize into the top level. This means we can reduce the number of individual checks in every helper function and thus making it simpler for our developers to add new operators.

The second commit here shows another aspect of the tflite frontend which is the use of asserts instead of using tvm.error . If we have a set of errors that can be raised by the various frontends attached to tvm, we should use them more efficiently.

Thanks,
Ramana

@tqchen , @anijain2305 , @FrozenGene - comments / reviews would be welcome.

The idea here is to minimise the code duplication for
asserting that the number of input_tensors and output_tensors
is as expected for the operator.
There are a number of places in the tflite frontend where we use
asserts to issue an error to the user about something not working.

Instead of using asserts these should be raising a proper
error that can be caught.

This is an example of how this restructure can happen over
time and for something to be done around this in the future.
@maheshambule
Copy link
Contributor

@u99127, I am also working on refactoring TFLite frontend. This is a sample implementation #5528. I would love to hear your opinion on it.

I am using a decorator to do all the checks, common stuff, and dynamic imports of options APIs.

@tqchen tqchen closed this Oct 11, 2020
@tqchen tqchen reopened this Oct 11, 2020
@tqchen tqchen changed the base branch from master to main October 11, 2020 18:42
@jroesch
Copy link
Member

jroesch commented Aug 27, 2021

@u99127 I am doing triage on old PRs, going to close this, please feel free to follow up if you would like to still merge these changes. Thanks for your contributions!.

@jroesch jroesch closed this Aug 27, 2021
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.

5 participants