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/14 07:52:47 UTC
[GitHub] [tvm] rohanmukh opened a new pull request #8251: [Frontend, Tensorflow] Support for broadcasting in batch_matmul when shapes differ
rohanmukh opened a new pull request #8251:
URL: https://github.com/apache/tvm/pull/8251
Current implementation of `batch_matmul` in TF frontend is not able to handle cases where the shape of the second input differs from the first and a broadcast is needed to complete the operation. Also, in the current logic it always assumed that the shape of second input `shape_y` is atleast of length 3. This is not the case is some TF2 models like (efficientdet)[https://tfhub.dev/tensorflow/efficientdet/d0/1]. This PR handles these use case.
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] rohanmukh commented on a change in pull request #8251: [Frontend, Tensorflow] Support for broadcasting in batch_matmul when shapes differ
Posted by GitBox <gi...@apache.org>.
rohanmukh commented on a change in pull request #8251:
URL: https://github.com/apache/tvm/pull/8251#discussion_r652209025
##########
File path: python/tvm/relay/frontend/tensorflow_ops.py
##########
@@ -1162,6 +1163,9 @@ def _impl(inputs, attr, params, mod):
adj_x = attr["adj_x"]
adj_y = attr["adj_y"]
input_x = _op.transpose(input_x, axes=[0, 2, 1]) if adj_x else input_x
+ shape_y = _infer_shape(input_y, mod)
+ if len(shape_y) < 3:
+ input_y = _op.reshape(input_y, (1, orig_shape_y[-2], orig_shape_y[-1]))
Review comment:
Thanks @comaniac . It is important for cases where `ndim=len(shape_x)` is <=3. I have test cases that fail without this line. Like `_test_batch_matmul((1, 8, 64), (64, 1), "float32", False, False)`. However the case that you mentioned can also happen for certain input configurations. I refactored the logic to avoid that.
--
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 #8251: [Frontend, Tensorflow] Support for broadcasting in batch_matmul when shapes differ
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8251:
URL: https://github.com/apache/tvm/pull/8251#discussion_r652201290
##########
File path: python/tvm/relay/frontend/tensorflow_ops.py
##########
@@ -1162,6 +1163,9 @@ def _impl(inputs, attr, params, mod):
adj_x = attr["adj_x"]
adj_y = attr["adj_y"]
input_x = _op.transpose(input_x, axes=[0, 2, 1]) if adj_x else input_x
+ shape_y = _infer_shape(input_y, mod)
+ if len(shape_y) < 3:
+ input_y = _op.reshape(input_y, (1, orig_shape_y[-2], orig_shape_y[-1]))
Review comment:
Is this required for static shape? It seems to me that you'll get two reshapes, although it should be simplified later by the SimplifyExpr pass.
```
%1 = reshape(%y, (1, p, q));
%2 = reshape(%1, (1, p, q));
```
--
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] mbrookhart commented on a change in pull request #8251: [Frontend, Tensorflow] Support for broadcasting in batch_matmul when shapes differ
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #8251:
URL: https://github.com/apache/tvm/pull/8251#discussion_r651276848
##########
File path: python/tvm/relay/frontend/tensorflow_ops.py
##########
@@ -1157,11 +1154,18 @@ def _impl(inputs, attr, params, mod):
new_shape_y = _op.concatenate(_op.Tuple(new_shape_y), axis=0)
input_x = _op.reshape(input_x, newshape=new_shape_x)
- input_y = _op.reshape(input_y, newshape=new_shape_y)
+
+ if np.prod(orig_shape_y) < np.prod(new_shape_y):
+ input_y = _op.broadcast_to(input_y, new_shape_y)
Review comment:
Broadcasting here could cause memory sizes to explode, would it be better to use implicit broadcasting in the batch_matmul kernel if possible? We do that in the onnx importer, but converting back to ND can be a pain.
--
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 pull request #8251: [Frontend, Tensorflow] Support for broadcasting in batch_matmul when shapes differ
Posted by GitBox <gi...@apache.org>.
rohanmukh commented on pull request #8251:
URL: https://github.com/apache/tvm/pull/8251#issuecomment-860924081
@trevor-m @yongwww @comaniac @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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on a change in pull request #8251: [Frontend, Tensorflow] Support for broadcasting in batch_matmul when shapes differ
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #8251:
URL: https://github.com/apache/tvm/pull/8251#discussion_r651303466
##########
File path: python/tvm/relay/frontend/tensorflow_ops.py
##########
@@ -1157,11 +1154,18 @@ def _impl(inputs, attr, params, mod):
new_shape_y = _op.concatenate(_op.Tuple(new_shape_y), axis=0)
input_x = _op.reshape(input_x, newshape=new_shape_x)
- input_y = _op.reshape(input_y, newshape=new_shape_y)
+
+ if np.prod(orig_shape_y) < np.prod(new_shape_y):
+ input_y = _op.broadcast_to(input_y, new_shape_y)
Review comment:
Agree. Please refer to ONNX and PyTorch frontend to avoid explicit broadcasting. Now both x86 and CUDA implementations of batch_matmul support implicit broadcasting, so simply `expand_dims(input_y)` to make it `(1, k, n)` would be sufficient.
--
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