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/04/21 19:44:45 UTC

[GitHub] [tvm] srinidhigoud opened a new pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

srinidhigoud opened a new pull request #7901:
URL: https://github.com/apache/tvm/pull/7901


   Adding support in tf parser for new ops SelectV2 and BroadcastArgs introduced by tf2. This is focused on compiling and running the tf2 version of the object detection models such as faster rcnn model 


-- 
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] comaniac commented on a change in pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#discussion_r619443233



##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -1765,6 +1765,44 @@ def _impl(inputs, attr, params, mod):
     return _impl
 
 
+def _broadcast_args():
+    def _impl(inputs, attr, params, mod):
+        if isinstance(inputs[0], _expr.Var):
+            s0 = params[inputs[0].name_hint]
+        else:
+            s0 = _infer_value(inputs[0], params, mod)
+        if isinstance(inputs[1], _expr.Var):
+            s1 = params[inputs[1].name_hint]
+        else:
+            s1 = _infer_value(inputs[1], params, mod)
+        s0 = list(s0.asnumpy().reshape([-1]))
+        s1 = list(s1.asnumpy().reshape([-1]))
+        s0_size, s1_size = len(s0), len(s1)
+        from collections import deque

Review comment:
       Move import to the top of this file.

##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -2833,6 +2872,7 @@ def _impl(inputs, attr, params, mod):
     "Round": AttrCvt("round"),
     "Rsqrt": _rsqrt(),
     "Select": _where(),
+    "SelectV2": _where(),

Review comment:
       Do we have a test for it?




-- 
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] comaniac merged pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #7901:
URL: https://github.com/apache/tvm/pull/7901


   


-- 
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] comaniac merged pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #7901:
URL: https://github.com/apache/tvm/pull/7901


   


-- 
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] srinidhigoud commented on a change in pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
srinidhigoud commented on a change in pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#discussion_r617979127



##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -1765,6 +1765,44 @@ def _impl(inputs, attr, params, mod):
     return _impl
 
 
+def _broadcast_args():
+    def _impl(inputs, attr, params, mod):
+        if isinstance(inputs[0], _expr.Var):
+            s0 = params[inputs[0].name_hint]
+        else:
+            s0 = _infer_value(inputs[0], params, mod)
+        if isinstance(inputs[1], _expr.Var):
+            s1 = params[inputs[1].name_hint]
+        else:
+            s1 = _infer_value(inputs[1], params, mod)
+        s0 = list(s0.asnumpy().reshape([-1]))
+        s1 = list(s1.asnumpy().reshape([-1]))
+        s0_size, s1_size = len(s0), len(s1)
+        from collections import deque
+
+        out = deque([])
+        for i in range(1, min(s0_size, s1_size) + 1):
+            if s0[s0_size - i] == s1[s1_size - i]:
+                out.appendleft(s0[s0_size - i])
+            elif s0[s0_size - i] == 1:
+                out.appendleft(s1[s1_size - i])
+            else:
+                assert s1[s1_size - i] == 1, "Incompatible broadcast type %s and %s" % (
+                    s0[s0_size - i],
+                    s1[s1_size - i],
+                )
+                out.appendleft(s0[s0_size - i])
+        if s0_size < s1_size:
+            for i in range(s0_size + 1, s1_size + 1):
+                out.appendleft(s1[s1_size - i])
+        if s1_size < s0_size:
+            for i in range(s1_size + 1, s0_size + 1):
+                out.appendleft(s0[s0_size - i])

Review comment:
       This will require atleast two list inversions (both the inputs as we have to iterate in reverse to verify broadcasting rules). Fill values will only make sense if I use a zip. It is only two loops (one of last two will fail) and merging them to one will give marginal optimization. 




-- 
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] rohanmukh commented on a change in pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
rohanmukh commented on a change in pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#discussion_r617970388



##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -1765,6 +1765,44 @@ def _impl(inputs, attr, params, mod):
     return _impl
 
 
+def _broadcast_args():
+    def _impl(inputs, attr, params, mod):
+        if isinstance(inputs[0], _expr.Var):
+            s0 = params[inputs[0].name_hint]
+        else:
+            s0 = _infer_value(inputs[0], params, mod)
+        if isinstance(inputs[1], _expr.Var):
+            s1 = params[inputs[1].name_hint]
+        else:
+            s1 = _infer_value(inputs[1], params, mod)
+        s0 = list(s0.asnumpy().reshape([-1]))
+        s1 = list(s1.asnumpy().reshape([-1]))
+        s0_size, s1_size = len(s0), len(s1)
+        from collections import deque

Review comment:
       Better to have import on the top to avoid loading this multiple times while parsing gdef.




-- 
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] comaniac commented on pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#issuecomment-826123628


   Thanks @srinidhigoud @rohanmukh 


-- 
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] srinidhigoud commented on pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
srinidhigoud commented on pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#issuecomment-824373069


   @zhiics    @yongwww  @comaniac 


-- 
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] rohanmukh commented on a change in pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
rohanmukh commented on a change in pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#discussion_r617970021



##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -1765,6 +1765,44 @@ def _impl(inputs, attr, params, mod):
     return _impl
 
 
+def _broadcast_args():
+    def _impl(inputs, attr, params, mod):
+        if isinstance(inputs[0], _expr.Var):
+            s0 = params[inputs[0].name_hint]
+        else:
+            s0 = _infer_value(inputs[0], params, mod)
+        if isinstance(inputs[1], _expr.Var):
+            s1 = params[inputs[1].name_hint]
+        else:
+            s1 = _infer_value(inputs[1], params, mod)
+        s0 = list(s0.asnumpy().reshape([-1]))
+        s1 = list(s1.asnumpy().reshape([-1]))
+        s0_size, s1_size = len(s0), len(s1)
+        from collections import deque
+
+        out = deque([])
+        for i in range(1, min(s0_size, s1_size) + 1):
+            if s0[s0_size - i] == s1[s1_size - i]:
+                out.appendleft(s0[s0_size - i])
+            elif s0[s0_size - i] == 1:
+                out.appendleft(s1[s1_size - i])
+            else:
+                assert s1[s1_size - i] == 1, "Incompatible broadcast type %s and %s" % (
+                    s0[s0_size - i],
+                    s1[s1_size - i],
+                )
+                out.appendleft(s0[s0_size - i])
+        if s0_size < s1_size:
+            for i in range(s0_size + 1, s1_size + 1):
+                out.appendleft(s1[s1_size - i])
+        if s1_size < s0_size:
+            for i in range(s1_size + 1, s0_size + 1):
+                out.appendleft(s0[s0_size - i])

Review comment:
       Is it better to use itertools.zip_longest() on the reversed array instead of running three separate loops? Might be helpful to have fill_values set at 1 to avoid handling None values. 




-- 
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] srinidhigoud edited a comment on pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
srinidhigoud edited a comment on pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#issuecomment-824373069


   @zhiics    @yongwww  @comaniac @rohanmukh 


-- 
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] comaniac commented on pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#issuecomment-826123628


   Thanks @srinidhigoud @rohanmukh 


-- 
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] srinidhigoud commented on a change in pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
srinidhigoud commented on a change in pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#discussion_r619547779



##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -1765,6 +1765,44 @@ def _impl(inputs, attr, params, mod):
     return _impl
 
 
+def _broadcast_args():
+    def _impl(inputs, attr, params, mod):
+        if isinstance(inputs[0], _expr.Var):
+            s0 = params[inputs[0].name_hint]
+        else:
+            s0 = _infer_value(inputs[0], params, mod)
+        if isinstance(inputs[1], _expr.Var):
+            s1 = params[inputs[1].name_hint]
+        else:
+            s1 = _infer_value(inputs[1], params, mod)
+        s0 = list(s0.asnumpy().reshape([-1]))
+        s1 = list(s1.asnumpy().reshape([-1]))
+        s0_size, s1_size = len(s0), len(s1)
+        from collections import deque

Review comment:
       Addressed in the latest commit

##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -1765,6 +1765,44 @@ def _impl(inputs, attr, params, mod):
     return _impl
 
 
+def _broadcast_args():
+    def _impl(inputs, attr, params, mod):
+        if isinstance(inputs[0], _expr.Var):
+            s0 = params[inputs[0].name_hint]
+        else:
+            s0 = _infer_value(inputs[0], params, mod)
+        if isinstance(inputs[1], _expr.Var):
+            s1 = params[inputs[1].name_hint]
+        else:
+            s1 = _infer_value(inputs[1], params, mod)
+        s0 = list(s0.asnumpy().reshape([-1]))
+        s1 = list(s1.asnumpy().reshape([-1]))
+        s0_size, s1_size = len(s0), len(s1)
+        from collections import deque

Review comment:
       Addressed in the latest commit




-- 
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] srinidhigoud commented on a change in pull request #7901: [Frontend][Tensorflow] SelectV2 and BroadcastArgs op support for tf2 models

Posted by GitBox <gi...@apache.org>.
srinidhigoud commented on a change in pull request #7901:
URL: https://github.com/apache/tvm/pull/7901#discussion_r619547983



##########
File path: python/tvm/relay/frontend/tensorflow.py
##########
@@ -2833,6 +2872,7 @@ def _impl(inputs, attr, params, mod):
     "Round": AttrCvt("round"),
     "Rsqrt": _rsqrt(),
     "Select": _where(),
+    "SelectV2": _where(),

Review comment:
       since we are using _where op, just as in the case for "Select", it is the same test as that of _where. Added a new comment in the latest commit that references to SelectV2 test




-- 
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