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, Tensorflow2] Adding TF2 frontend code with support for control flow ops #8142

Merged
merged 13 commits into from
Jun 8, 2021

Conversation

rohanmukh
Copy link
Contributor

@rohanmukh rohanmukh commented May 26, 2021

This PR introduces a new parser tensorflow2.py. To keep the code separate from original parser code and introduce TF2 related changes, much of the existing code is ported into this frontend code. This commit follows the original plan of commits as proposed to support TF2 frontend in relay here: #4102. Exact commit plan here.

Some important points on introducing the TF2 frontend:

  1. All of the existing ops as available in tensorflow.py has been reused.
  2. Support for ops like PartitionedCall, StatelessPartitionedCall invoked as functional nodes.
  3. The control flow logic in TF1 is not used, and in that high level control flow structures like If, While, StatelessWhile has been introduced which are invoked as functional node.

Some points in testing:

  1. All of the unit tests available in tests/python/frontend/tensorflow2/ were used in evaluating the new parser. Previous unit tests cover the support for ops like Add2D, Relu etc. in test_functional.py and some small end-to-end models in test_sequential.py.
  2. Additional tests to test the control flow structures have been introduced in this PR in test_functional.py for ops like If, While and StatelessWhile.

Co-authored-by: David Huang davhuan@amazon.com
Co-authored-by: Rohan Mukherjee mukrohan@amazon.com
Co-authored-by: Srinidhi Goud srinidhi.goud29@gmail.com
Co-authored-by: Xingyu Zhou zhoxingy@amazon.com
Co-authored-by: Xiao weix@amazon.com

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 by @ them in the pull request thread.

rohanmukh and others added 6 commits May 26, 2021 18:59
Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>
Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>
Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>
@rohanmukh rohanmukh marked this pull request as ready for review June 1, 2021 22:29
@rohanmukh rohanmukh changed the title [WIP] adding tf2 control flow ops with a different frontend code [Frontend][Tensorflow2] adding tf2 control flow ops with a different frontend code Jun 2, 2021
@rohanmukh
Copy link
Contributor Author

@@ -23,8 +23,7 @@

from tvm.runtime.vm import VirtualMachine
import tvm.contrib.graph_executor as runtime
from tvm.relay.frontend.tensorflow import from_tensorflow

from tvm.relay.frontend.tensorflow2 import from_tensorflow
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously the existing parser from relay/frontend/tensorflow.py was being used to test the basic TF2 ops, functions and models. From this PR, all of those test as well as the new ones are being passed through the new frontend parser for TF2, namely relay/frontend/tensorflow2.py

else:
sym = [_expr.var(node.name, shape=input_shape, dtype=attr["dtype"].name)]
return input_shape, sym

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The utility functions on top are imported from tensorflow.py. Since they were methods inside class they had to extracted. Future refactor PR can address this to allow better code reuse. However all TF1 unit tests and models need to be tested alongside for such a refactor.

@rohanmukh
Copy link
Contributor Author

@srinidhigoud

@rohanmukh rohanmukh changed the title [Frontend][Tensorflow2] adding tf2 control flow ops with a different frontend code [Frontend, Tensorflow2] adding tf2 control flow ops with a different frontend code Jun 2, 2021
@rohanmukh rohanmukh changed the title [Frontend, Tensorflow2] adding tf2 control flow ops with a different frontend code [Frontend, Tensorflow2] Adding TF2 frontend code with support for control flow ops Jun 2, 2021
Comment on lines 28 to 30
from tensorflow.python.framework import function_def_to_graph
from tensorflow.python.framework import tensor_util
from tensorflow.python.framework import dtypes
Copy link
Member

Choose a reason for hiding this comment

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

could be merged

def triple(x):
return 3 * x

cond = True
Copy link
Member

Choose a reason for hiding this comment

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

add case for False

Comment on lines 100 to 111
// tensorflow/core/framework/attr_value.proto
message AttrValue {
oneof value {
bytes s = 2; // "string"
int64 i = 3; // "int"
float f = 4; // "float"
bool b = 5; // "bool"
DataType type = 6; // "type"
TensorShapeProto shape = 7; // "shape"
TensorProto tensor = 8; // "tensor"
ListValue list = 1; // any "list(...)" }
}
Copy link
Member

Choose a reason for hiding this comment

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

post a link instead of pasting the proto def should be good

"""
fields = ["s", "i", "f", "b", "type", "shape", "tensor", "func"]

x = buf
Copy link
Member

Choose a reason for hiding this comment

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

use buf directly?

return attrs


def convert_place_holder(shape, node, in_type=None):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def convert_place_holder(shape, node, in_type=None):
def convert_placeholder(shape, node, in_type=None):



def convert_place_holder(shape, node, in_type=None):
"""convert tf place holder into relay var.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""convert tf place holder into relay var.
"""convert tf placeholder into relay var.

Comment on lines 213 to 215
self.mod = IRModule({}) # relay function and type definitions. defined in tvm/ir/module.py
self.params = {} # for constants (weights) in the entire relay module
self.prelude = Prelude(self.mod) # relay.prelude needed for tensorlist ops
Copy link
Member

Choose a reason for hiding this comment

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

remove the comments?


Parameters
----------
op_name : str
Copy link
Member

Choose a reason for hiding this comment

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

add definition for graph, node_name as well

from ..loops import while_loop as _while_loop
from .common import infer_type as _infer_type

from .tensorflow import _convert_map as _convert_map_tf1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from .tensorflow import _convert_map as _convert_map_tf1
from .tensorflow import _convert_map as _convert_map_common

None,
)
loop_inputs = convert_vars(inputs, while_func.signature.input_arg)
# in_shapes = nodes[node_name].attr["output_shapes"].list.shape
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# in_shapes = nodes[node_name].attr["output_shapes"].list.shape

@yongwww
Copy link
Member

yongwww commented Jun 3, 2021

@kevinthesun

@rohanmukh
Copy link
Contributor Author

Thanks for the comments @yongwww. I addressed the changes in the last commit.

Copy link
Contributor

@trevor-m trevor-m left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@zhiics zhiics left a comment

Choose a reason for hiding this comment

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

lgtm

@zhiics zhiics merged commit 64a8e81 into apache:main Jun 8, 2021
@zhiics
Copy link
Member

zhiics commented Jun 8, 2021

Thanks @yongwww @trevor-m

@rohanmukh
Copy link
Contributor Author

Thanks @zhiics @yongwww @trevor-m

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
…trol flow ops (apache#8142)

* adding tf control flow ops with a different frontend code

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>

* Some minor fixes

* Fixing output order in TF2 outputs

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>

* Using black

* Refactoring

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>

* resolving a bug with passing output tensors for Functional Graphs

* fixing multi output for graph runtime

* adding docstring edits

* linting + black

* linting + black

* linting + black

* removing unnecessary output propagation across function

* addressed comments in PR

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
…trol flow ops (apache#8142)

* adding tf control flow ops with a different frontend code

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>

* Some minor fixes

* Fixing output order in TF2 outputs

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>

* Using black

* Refactoring

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Rohan Mukherjee <mukrohan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>

* resolving a bug with passing output tensors for Functional Graphs

* fixing multi output for graph runtime

* adding docstring edits

* linting + black

* linting + black

* linting + black

* removing unnecessary output propagation across function

* addressed comments in PR

Co-authored-by: David Huang <davhuan@amazon.com>
Co-authored-by: Srinidhi Goud <srinidhi.goud29@gmail.com>
Co-authored-by: Xingyu Zhou <zhoxingy@amazon.com>
Co-authored-by: Xiao <weix@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants