You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/06/30 09:00:56 UTC
[GitHub] [tvm] u99127 opened a new pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
u99127 opened a new pull request #8375:
URL: https://github.com/apache/tvm/pull/8375
Tensorflow Lite has changed the opcode for BuiltinOperators
to be represented as 32 bit integers instead of 8 bit integers
in the schema.
This is an attempt to fix this in a way that is clean to handle
multiple versions of tensorflow lite in the frontend.
@ANSHUMAN87 , @mbrookhart
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart merged pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
mbrookhart merged pull request #8375:
URL: https://github.com/apache/tvm/pull/8375
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] u99127 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661295402
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
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 ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ANSHUMAN87 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661281565
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
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)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] u99127 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661295402
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
Using the environment as it would be once the CI images are baked I cannot seem to find schema_util and visualise . 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 importing tflite.python and tensorflow.lite.python but to no avail.
Can we fix up the CI breakages with this PR now and follow up with a cleaner fix / cleanup ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ANSHUMAN87 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661591551
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ANSHUMAN87 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661300462
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
In Tensorflow 2.3.1 , above APIs are not present, we can have fallback to previous implementation before recent upgrade in TVM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] u99127 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661557628
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
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 ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] u99127 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661295402
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
Using the environment as it would be once the CI images are baked I cannot seem to find schema_util and visualise . 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 ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] u99127 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
u99127 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661302075
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
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 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] ANSHUMAN87 commented on a change in pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#discussion_r661310820
##########
File path: python/tvm/relay/frontend/tflite.py
##########
@@ -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:
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] mbrookhart commented on pull request #8375: Fix issue with importing models using Tensorflow Lite 2.4.x schema
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #8375:
URL: https://github.com/apache/tvm/pull/8375#issuecomment-871483204
Thanks @u99127 @ANSHUMAN87 @leandron
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org