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 2020/04/11 07:26:14 UTC
[GitHub] [incubator-tvm] masahi opened a new pull request #5306: [Torch]
Support Python list, more realistic recurrent networks
masahi opened a new pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306
This PR builds on the control flow support added in https://github.com/apache/incubator-tvm/pull/4964 and aims to support more realistic recurrent networks than the simple one in #4964. Specifically the goal is to enable translating LSTM models in the PyTorch repo https://github.com/pytorch/pytorch/tree/master/benchmarks/fastrnns described in [their blog post](https://pytorch.org/blog/optimizing-cuda-rnn-with-torchscript/).
Translating these models requires taking care of dynamic lists and tensor shape. I added necessary support in the Torch frontend using prelude List ADT, static Tensor array https://github.com/apache/incubator-tvm/pull/5103, and Any.
See the new test cases for the kinds of models we can support now. I added some variants of LSTMs:
* LSTM with layer normalization
* Bidirectional
* Stacked
* Stacked and Bidirectional
please review @kevinthesun @zhiics @MarisaKirisame @icemelon9 @jwfromm @wweic @alexwong
cc @tqchen @jroesch @ajtulloch @junrushao1994
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on issue #5306: [Torch]
Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on issue #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#issuecomment-612767725
Thanks @masahi @MarisaKirisame
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi edited a comment on issue #5306: [Torch]
Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi edited a comment on issue #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#issuecomment-612441129
> LGTM. Is torchscript list immutable or mutable like python's list?
Yes it is mutable. List `append` is mapped to `aten::append` in Torchscript and it is entirely side-effecting operation. See below for a simple module that only does list append and its Torchscript representation.
Even though variables that are updated in the loop are supposed to be passed to `prim::Loop` op to become loop variables, this does not apply to side effecting operations like list append. Translating this module to Relay is complicated because also in Relay loop variables are the only ones that can be updated between iteration. If we naively translate it, the list `outputs.1` below appears free in the loop and can not be updated.
```Py
class ListAppend(nn.Module):
def forward(self, input):
# type: (Tensor) -> List[Tensor]
outputs = []
for i in range(input.size(0)):
outputs.append(input)
return outputs
```
```
graph(%self : __torch__.ListAppend,
%input.1 : Tensor):
%8 : bool = prim::Constant[value=1]() # rnn_test.py:142:8
%4 : int = prim::Constant[value=0]() # rnn_test.py:142:34
%outputs.1 : Tensor[] = prim::ListConstruct()
%5 : int = aten::size(%input.1, %4) # rnn_test.py:142:23
= prim::Loop(%5, %8) # rnn_test.py:142:8
block0(%i : int):
%12 : Tensor[] = aten::append(%outputs.1, %input.1) # rnn_test.py:143:12
-> (%8)
return (%outputs.1)
```
To workaround the difficulty of list append, I use list concat to append one element at the tail of a list. The original LSTM models in Pytorch repo do not use list append either and use concat instead, probably for the same reason.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request
#5306: [Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407269285
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1077,6 +1186,50 @@ def _impl(inputs, input_types):
return _op.cast(inputs[0], "float32")
return _impl
+
+def _mm():
+ def _impl(inputs, input_types):
+ return _op.nn.dense(inputs[0], inputs[1])
+ return _impl
+
+
+def _list_getitem(prelude):
+ def _impl(inputs, input_types):
+ return prelude.nth(inputs[0], _wrap_const(inputs[1]))
+ return _impl
+
+
+def _list_len(prelude):
+ def _impl(inputs, input_types):
+ return prelude.length(inputs[0])
+ return _impl
+
+
+def _add(prelude):
+ # add_ is overloaded for tensor add and list concat
+ def _impl(inputs, input_types):
+ if input_types[0] == "ListType":
+ return prelude.concat(inputs[0], inputs[1])
+ return _elemwise("add")(inputs, input_types)
+ return _impl
+
+
+def _tensor_array_stack(prelude):
+ def _impl(inputs, input_types):
+ tensor_array = _convert_to_tensor_array(inputs[0], prelude)
+ shape = get_tensor_array_shape(tensor_array, "float32", prelude)
+ stack = prelude.get_var_static('tensor_array_stack', "float32", shape)
Review comment:
I think we need to register static tensor array op first?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] MarisaKirisame commented on issue #5306: [Torch]
Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on issue #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#issuecomment-612553948
From an outsider, it seems like the more principled approach is to translate list to Reference of List. We could then write passses to remove Reference if possible.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on issue #5306: [Torch] Support
Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#issuecomment-612441129
> LGTM. Is torchscript list immutable or mutable like python's list?
Yes it is mutable. List `append` is mapped to `aten::append` in Torchscript and it is entirely side-effecting operation. See below for a simple module that only does list append its Torchscript representation.
Even though variables that are updated in the loop are supposed to be passed to `prim::Loop` op to become loop variables, this does not apply to side effecting operations like list append. Translating this module to Relay is complicated because also in Relay loop variables are the only ones that can be updated between iteration. If we naively translate it, the list `outputs.1` below appears free in the loop and can not be updated.
```Py
class ListAppend(nn.Module):
def forward(self, input):
# type: (Tensor) -> List[Tensor]
outputs = []
for i in range(input.size(0)):
outputs.append(input)
return outputs
```
```
graph(%self : __torch__.ListAppend,
%input.1 : Tensor):
%8 : bool = prim::Constant[value=1]() # rnn_test.py:142:8
%4 : int = prim::Constant[value=0]() # rnn_test.py:142:34
%outputs.1 : Tensor[] = prim::ListConstruct()
%5 : int = aten::size(%input.1, %4) # rnn_test.py:142:23
= prim::Loop(%5, %8) # rnn_test.py:142:8
block0(%i : int):
%12 : Tensor[] = aten::append(%outputs.1, %input.1) # rnn_test.py:143:12
-> (%8)
return (%outputs.1)
```
To workaround the difficulty of list append, I use list concat to append one element at the tail of a list. The original LSTM models in Pytorch repo do not use list append either and use concat instead, probably for the same reason.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5306:
[Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407288699
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -184,11 +182,9 @@ def _impl(inputs, input_types):
def _concatenate(prelude):
def tensor_array_concat(lst, axis):
assert axis == 0, "Tensor array concat supported only for axis 0"
- shape = _infer_type_with_prelude(prelude.hd(lst), prelude).shape
- concat_shape = (Any(),) + tuple(shape[1:])
-
- tensor_array = _map_tensor_array_constructor(lst, prelude, shape)
- static_tensor_array_ops = StaticTensorArrayOps(prelude, "float32", concat_shape)
+ tensor_array, shape = _convert_to_tensor_array(lst, prelude)
+ concat_shape = (Any(),) + shape[1:]
+ static_tensor_array_ops = StaticTensorArrayOps(prelude, "float32", shape)
static_tensor_array_ops.define_tensor_get_data(concat_shape)
concat = prelude.get_var_static('tensor_array_concat', "float32", concat_shape)
Review comment:
fixed. So a rule of thumb is only `define_tensor_get_data` takes a new (concat-ed, stacked etc) shape?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request
#5306: [Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407287295
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -184,11 +182,9 @@ def _impl(inputs, input_types):
def _concatenate(prelude):
def tensor_array_concat(lst, axis):
assert axis == 0, "Tensor array concat supported only for axis 0"
- shape = _infer_type_with_prelude(prelude.hd(lst), prelude).shape
- concat_shape = (Any(),) + tuple(shape[1:])
-
- tensor_array = _map_tensor_array_constructor(lst, prelude, shape)
- static_tensor_array_ops = StaticTensorArrayOps(prelude, "float32", concat_shape)
+ tensor_array, shape = _convert_to_tensor_array(lst, prelude)
+ concat_shape = (Any(),) + shape[1:]
+ static_tensor_array_ops = StaticTensorArrayOps(prelude, "float32", shape)
static_tensor_array_ops.define_tensor_get_data(concat_shape)
concat = prelude.get_var_static('tensor_array_concat', "float32", concat_shape)
Review comment:
Also need to use shape here?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request
#5306: [Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407287501
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -103,11 +178,27 @@ def _impl(inputs, input_types):
return _op.transform.expand_dims(data, int(axis), 1)
return _impl
-def _concatenate():
+
+def _concatenate(prelude):
+ def tensor_array_concat(lst, axis):
+ assert axis == 0, "Tensor array concat supported only for axis 0"
+ tensor_array, shape = _convert_to_tensor_array(lst, prelude)
+ concat_shape = (Any(),) + shape[1:]
+ static_tensor_array_ops = StaticTensorArrayOps(prelude, "float32", shape)
+ static_tensor_array_ops.define_tensor_get_data(concat_shape)
+
+ concat = prelude.get_var_static('tensor_array_concat', "float32", concat_shape)
+ concatenated = concat(tensor_array)
+ get_tensor = prelude.get_var_static('tensor_get_data', "float32", concat_shape)
Review comment:
Same.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5306:
[Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407273147
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1077,6 +1186,50 @@ def _impl(inputs, input_types):
return _op.cast(inputs[0], "float32")
return _impl
+
+def _mm():
+ def _impl(inputs, input_types):
+ return _op.nn.dense(inputs[0], inputs[1])
+ return _impl
+
+
+def _list_getitem(prelude):
+ def _impl(inputs, input_types):
+ return prelude.nth(inputs[0], _wrap_const(inputs[1]))
+ return _impl
+
+
+def _list_len(prelude):
+ def _impl(inputs, input_types):
+ return prelude.length(inputs[0])
+ return _impl
+
+
+def _add(prelude):
+ # add_ is overloaded for tensor add and list concat
+ def _impl(inputs, input_types):
+ if input_types[0] == "ListType":
+ return prelude.concat(inputs[0], inputs[1])
+ return _elemwise("add")(inputs, input_types)
+ return _impl
+
+
+def _tensor_array_stack(prelude):
+ def _impl(inputs, input_types):
+ tensor_array = _convert_to_tensor_array(inputs[0], prelude)
+ shape = get_tensor_array_shape(tensor_array, "float32", prelude)
+ stack = prelude.get_var_static('tensor_array_stack', "float32", shape)
Review comment:
It is registered in `_convert_to_tensor_array` when creating a tensor array. Since tensor array is created only through this function, I think it is ok and cleaner than adding get_shape, `StaticTensorArrayOps(...)`, and register().
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5306:
[Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407273733
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -25,20 +25,97 @@
import numpy as np
import tvm
-from tvm.ir import module as _module
from .. import analysis as _analysis
from .. import expr as _expr
from .. import op as _op
+from ..function import Function
+from .. import transform
+from ..ty import TupleType, TensorType, Any
from ..loops import while_loop
from .common import get_relay_op
from .common import infer_shape as _infer_shape
from .common import infer_value as _infer_value
+from ..prelude import Prelude, StaticTensorArrayOps, get_tensor_array_shape
from . import qnn_torch
__all__ = ["from_pytorch"]
+
+# List ADT utilities
+def _infer_type_with_prelude(val, prelude):
Review comment:
Thanks for pointing this out. I added this when your TF frontend PR hasn't been merged. Now we can simplify this function using infer_type in common.py
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request
#5306: [Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407269094
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -103,11 +180,29 @@ def _impl(inputs, input_types):
return _op.transform.expand_dims(data, int(axis), 1)
return _impl
-def _concatenate():
+
+def _concatenate(prelude):
+ def tensor_array_concat(lst, axis):
+ assert axis == 0, "Tensor array concat supported only for axis 0"
+ shape = _infer_type_with_prelude(prelude.hd(lst), prelude).shape
+ concat_shape = (Any(),) + tuple(shape[1:])
+
+ tensor_array = _map_tensor_array_constructor(lst, prelude, shape)
+ static_tensor_array_ops = StaticTensorArrayOps(prelude, "float32", concat_shape)
Review comment:
We should register shape instead of concat_shape. Take a look at tensor array concat in tensorflow frontend: https://github.com/apache/incubator-tvm/blob/master/python/tvm/relay/frontend/tensorflow.py#L1090-L1100
The reason is that if shape is fully static, we don't need an unnecessary shape function and runtime memory allocation.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun merged pull request #5306: [Torch]
Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun merged pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] kevinthesun commented on a change in pull request
#5306: [Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
kevinthesun commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407268130
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -25,20 +25,97 @@
import numpy as np
import tvm
-from tvm.ir import module as _module
from .. import analysis as _analysis
from .. import expr as _expr
from .. import op as _op
+from ..function import Function
+from .. import transform
+from ..ty import TupleType, TensorType, Any
from ..loops import while_loop
from .common import get_relay_op
from .common import infer_shape as _infer_shape
from .common import infer_value as _infer_value
+from ..prelude import Prelude, StaticTensorArrayOps, get_tensor_array_shape
from . import qnn_torch
__all__ = ["from_pytorch"]
+
+# List ADT utilities
+def _infer_type_with_prelude(val, prelude):
Review comment:
Can we reuse infer_type defined in common.py. The only difference is that we need to get checked_type for the return of infer_type.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on issue #5306: [Torch] Support
Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#issuecomment-612701691
@kevinthesun Thanks for the review! Please have a look at the last 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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] masahi commented on a change in pull request #5306:
[Torch] Support Python list, more realistic recurrent networks
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5306: [Torch] Support Python list, more realistic recurrent networks
URL: https://github.com/apache/incubator-tvm/pull/5306#discussion_r407273147
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1077,6 +1186,50 @@ def _impl(inputs, input_types):
return _op.cast(inputs[0], "float32")
return _impl
+
+def _mm():
+ def _impl(inputs, input_types):
+ return _op.nn.dense(inputs[0], inputs[1])
+ return _impl
+
+
+def _list_getitem(prelude):
+ def _impl(inputs, input_types):
+ return prelude.nth(inputs[0], _wrap_const(inputs[1]))
+ return _impl
+
+
+def _list_len(prelude):
+ def _impl(inputs, input_types):
+ return prelude.length(inputs[0])
+ return _impl
+
+
+def _add(prelude):
+ # add_ is overloaded for tensor add and list concat
+ def _impl(inputs, input_types):
+ if input_types[0] == "ListType":
+ return prelude.concat(inputs[0], inputs[1])
+ return _elemwise("add")(inputs, input_types)
+ return _impl
+
+
+def _tensor_array_stack(prelude):
+ def _impl(inputs, input_types):
+ tensor_array = _convert_to_tensor_array(inputs[0], prelude)
+ shape = get_tensor_array_shape(tensor_array, "float32", prelude)
+ stack = prelude.get_var_static('tensor_array_stack', "float32", shape)
Review comment:
It is registered in `_convert_to_tensor_array` when creating a tensor array. Since tensor array is created only through this function, I think it is ok and cleaner than adding get_shape, `StaticTensorArrayOps(...)`, and register() everywhere.
----------------------------------------------------------------
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
With regards,
Apache Git Services