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/02/22 15:51:05 UTC
[GitHub] [tvm] srkreddy1238 opened a new pull request #7498: [TENSORFLOW] Tensorflow 2.x support
srkreddy1238 opened a new pull request #7498:
URL: https://github.com/apache/tvm/pull/7498
* Frontend supports concreate function along with graphdef.
* New test cases added to validate TF2.x functions.
* E2E testcases will use TFHub inputs.
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right entry point? Why not start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of the test utility code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right parameter for the TF front end parser? How about start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way perhaps there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of some utility code for convenience.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580714263
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
+ try:
+ from tensorflow.python.framework import convert_to_constants
+ from tensorflow.core.protobuf import config_pb2
+ except ImportError as e:
+ raise ImportError("Unable to import tensorflow which is required {}".format(e))
+ concrete_func = graph
+ graph = convert_to_constants.convert_variables_to_constants_v2(concrete_func).graph
+ signature = _build_signature_def(graph, concrete_func.inputs, concrete_func.outputs)
+ graph_def = graph.as_graph_def()
+
+ # Some optimization
+ config = config_pb2.ConfigProto()
+ rewriter_config = config.graph_options.rewrite_options
+ rewriter_config.optimizers[:] = [
+ "debug_stripper",
+ "arithmetic",
+ "dependency",
+ "arithmetic",
Review comment:
why is line 3752 a duplicate of line 3750; copy paste error?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#issuecomment-788514782
@srkreddy1238 can you try to install via https://github.com/apache/tvm/blob/main/tests/scripts/task_ci_setup.sh
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right parameter for the TF front end parser? Why not start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of the test utility code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] srkreddy1238 closed pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
srkreddy1238 closed pull request #7498:
URL: https://github.com/apache/tvm/pull/7498
--
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] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580714263
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
+ try:
+ from tensorflow.python.framework import convert_to_constants
+ from tensorflow.core.protobuf import config_pb2
+ except ImportError as e:
+ raise ImportError("Unable to import tensorflow which is required {}".format(e))
+ concrete_func = graph
+ graph = convert_to_constants.convert_variables_to_constants_v2(concrete_func).graph
+ signature = _build_signature_def(graph, concrete_func.inputs, concrete_func.outputs)
+ graph_def = graph.as_graph_def()
+
+ # Some optimization
+ config = config_pb2.ConfigProto()
+ rewriter_config = config.graph_options.rewrite_options
+ rewriter_config.optimizers[:] = [
+ "debug_stripper",
+ "arithmetic",
+ "dependency",
+ "arithmetic",
Review comment:
is line 3752 a duplicate of line 3750?
It appears that "arithmetic" and "dependency" optimizations may be ON by default, while "debug_stripper" is OFF by default: https://www.tensorflow.org/guide/graph_optimization. I have not verified this. Are these required?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] srkreddy1238 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r584028985
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
Wrapper will change frontend API. We could seek community opinion on this.
I thought of saved_model, but we already have TFParser (python/tvm/relay/frontend/tensorflow_parser.py) helper that handles saved_model formats. I think we could make ```from_tensorflow``` being single entry for GraphDef / ConcreteFunction / Saved Model Dir. TFParser can handle all transforms until it results GraphDef.
TFHub URL to GraphDef can also be added to TFParser, but hub.KerasLayer has bunch or args to handle some times. We could hold it for now.
I will amend this PR with these changes and also TF1.x / TF2.x compatibility we discussed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] srkreddy1238 commented on pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#issuecomment-787769638
@masahi @tqchen @siju-samuel @zhiics @anijain2305 Can you help review this PR.
This brings in TF2.x support with existing parser and re-arrangements to accommodate TF2.x brand new parser.
@tqchen We need to refresh the docker image to install tensorflow-hub. The e2e models now come from TFHub.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow_concrete_function() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right entry point? Why not start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of the test utility code.
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
+ try:
+ from tensorflow.python.framework import convert_to_constants
+ from tensorflow.core.protobuf import config_pb2
+ except ImportError as e:
+ raise ImportError("Unable to import tensorflow which is required {}".format(e))
+ concrete_func = graph
+ graph = convert_to_constants.convert_variables_to_constants_v2(concrete_func).graph
+ signature = _build_signature_def(graph, concrete_func.inputs, concrete_func.outputs)
+ graph_def = graph.as_graph_def()
+
+ # Some optimization
+ config = config_pb2.ConfigProto()
+ rewriter_config = config.graph_options.rewrite_options
+ rewriter_config.optimizers[:] = [
+ "debug_stripper",
+ "arithmetic",
+ "dependency",
+ "arithmetic",
Review comment:
typo: duplicate of line 3750
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right parameter for the TF front end parser? How about start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way perhaps there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of the test utility code if we have test which needs to start from ConcreteFunction;
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right parameter for the TF front end parser? Why not start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way perhaps there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of the test utility code if we have test which needs to start from ConcreteFunction;
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580714263
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
+ try:
+ from tensorflow.python.framework import convert_to_constants
+ from tensorflow.core.protobuf import config_pb2
+ except ImportError as e:
+ raise ImportError("Unable to import tensorflow which is required {}".format(e))
+ concrete_func = graph
+ graph = convert_to_constants.convert_variables_to_constants_v2(concrete_func).graph
+ signature = _build_signature_def(graph, concrete_func.inputs, concrete_func.outputs)
+ graph_def = graph.as_graph_def()
+
+ # Some optimization
+ config = config_pb2.ConfigProto()
+ rewriter_config = config.graph_options.rewrite_options
+ rewriter_config.optimizers[:] = [
+ "debug_stripper",
+ "arithmetic",
+ "dependency",
+ "arithmetic",
Review comment:
why is line 3752 a duplicate of line 3750
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] wx3000 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
wx3000 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r580712245
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
Review comment:
would it be better if we create a wrapper over from_tensorflow() instead of overloading the parameter "graph" to be either GraphDef or ConcreteFunction?
Another question: Is ConcreteFunction the right parameter for the TF front end parser? Why not start from the saved_model format? When TVM is used to compile a model for inference, it starts from a saved_model. This way perhaps there is no need to use _build_signature_def in order to optimize the graph. Maybe _build_signature_def can be part of the test utility code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] srkreddy1238 commented on a change in pull request #7498: [TENSORFLOW] Tensorflow 2.x support
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on a change in pull request #7498:
URL: https://github.com/apache/tvm/pull/7498#discussion_r584029723
##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -3682,6 +3729,30 @@ def from_tensorflow(graph, layout="NHWC", shape=None, outputs=None):
params : dict of str to tvm.nd.NDArray
Dict of converted parameters stored in tvm.nd.NDArray format
"""
+ from tensorflow.python.eager.function import ConcreteFunction
+
+ if isinstance(graph, ConcreteFunction):
+ try:
+ from tensorflow.python.framework import convert_to_constants
+ from tensorflow.core.protobuf import config_pb2
+ except ImportError as e:
+ raise ImportError("Unable to import tensorflow which is required {}".format(e))
+ concrete_func = graph
+ graph = convert_to_constants.convert_variables_to_constants_v2(concrete_func).graph
+ signature = _build_signature_def(graph, concrete_func.inputs, concrete_func.outputs)
+ graph_def = graph.as_graph_def()
+
+ # Some optimization
+ config = config_pb2.ConfigProto()
+ rewriter_config = config.graph_options.rewrite_options
+ rewriter_config.optimizers[:] = [
+ "debug_stripper",
+ "arithmetic",
+ "dependency",
+ "arithmetic",
Review comment:
"arithmetic" is not a duplicate, this transform happen again.
The transforms order was with a ref. from TensorflowJS project. This can be revised later as needed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org