Skip to content

Fix issue with importing models using Tensorflow Lite 2.4.x schema #8375

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.

Sign up for GitHub

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

Merged
mbrookhart merged 1 commit into from
Jun 30, 2021
Merged

Fix issue with importing models using Tensorflow Lite 2.4.x schema #8375

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion python/tvm/relay/frontend/tflite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,30 @@ def get_op_code_str(self, op):
raise ImportError("The tflite package must be installed")

op_code_list_idx = op.OpcodeIndex()
op_code_id = self.model.OperatorCodes(op_code_list_idx).BuiltinCode()

op_c = self.model.OperatorCodes(op_code_list_idx)
# In TFlite 2.4.x there was a change where the type of the field that contained
# the builtin code changed from int8 to int32 in the flat buffer representation.
# However to retain support for old flat buffers that were created, they retained
# the original 8 bit encoding for the operator but in a new field accessed by the
# DeprecatedBuiltinCode method.
# This means that the API function BuiltinCode() is used on an operator
# which was originally encoded as an 8 bit quantity it would look for the
# code in the new int32 field in the schema and this creates the need
# for the check for the magic number of 127 which is indicated by
# BuiltinOperator.PLACEHOLDER_FOR_GREATER_OP_CODES
# Remember however that this value came into existence only after Tensorflow
# lite 2.4.x and hence encase it in a try -except block.
# Phew !
try:
if op_c.BuiltinCode() < BuiltinOperator.PLACEHOLDER_FOR_GREATER_OP_CODES:
Copy link
Contributor

@ANSHUMAN87 ANSHUMAN87 Jun 30, 2021

Choose a reason for hiding this comment

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

We are duplicating the implementation. Should not do that. Should always use higher level APIs, so that in future any bug fix or version upgrade, the underlying changes will be taken care automatically.

Use below APIs:

 from tensorflow.lite.python import schema_util
 from tensorflow.lite.tools import visualize
   

    op_code_list_idx = op.OpcodeIndex()
    op_code_id = self.model.OperatorCodes(op_code_list_idx).BuiltinCode()
    op_code_id = schema_util.get_builtin_code_from_operator_code(op_code_id)
    op_code_str = visualize.BuiltinCodeToName(op_code_id)`

Copy link
Author

@u99127 u99127 Jun 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Using the environment as it would be once the CI images are baked (i.e. with Tensorflow and Tensorflow lite 2.4.2) I cannot seem to find schema_util and visualise packages. Is there something else that you need to do in terms of building tensor flow and tensor flow lite to use this ?

I've tried both:

from tensorflow.lite.python import schema_util
and
from tflite.python import schema_util

Can we fix up the CI breakages with this PR now and follow up with a cleaner fix / cleanup ?

Copy link
Contributor

@ANSHUMAN87 ANSHUMAN87 Jun 30, 2021

Choose a reason for hiding this comment

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

In Tensorflow 2.3.1 , above APIs are not present, we can have fallback to previous implementation before recent upgrade in TVM.

Copy link
Author

@u99127 u99127 Jun 30, 2021

Choose a reason for hiding this comment

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

I'm pretty sure I've attempted that in the fall back by catching an AttributeError which is what one would get with a missing enum value .

Copy link
Contributor

@ANSHUMAN87 ANSHUMAN87 Jun 30, 2021

Choose a reason for hiding this comment

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

Current upgrade to Tensorflow 2.4.2, also does not install schema_util as part of pip package.
SO we can go ahead with current change. Later on when Tensorflow has the bug fixed, we can take the suggested APIs change.

Copy link
Author

@u99127 u99127 Jun 30, 2021

Choose a reason for hiding this comment

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

Sure, I'll file an issue for it instead of leaving a comment . Further, could you file file a bug in the Tensorflow project or is this a known issue there ?

Copy link
Contributor

@ANSHUMAN87 ANSHUMAN87 Jun 30, 2021

Choose a reason for hiding this comment

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

No need to file bug in Tensorflow.
I have verified my changes in Tf 2.5.0, it works fine.
Hence when we do next Tensorflow version upgrade in TVM. We can incorporate using these Apis.

opc = op_c.DeprecatedBuiltinCode()
else:
opc = op_c.BuiltinCode()
except AttributeError:
opc = op_c.BuiltinCode()

op_code_id = opc
try:
op_code_str = self.builtin_op_code[op_code_id]
except KeyError:
Expand Down