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/01 15:06:52 UTC
[GitHub] [incubator-tvm] jjohnson-arm opened a new pull request #5204:
[Frontend][Torch] Fix up graph input handling
jjohnson-arm opened a new pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204
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.
From: https://discuss.tvm.ai/t/pytorch-frontend-graph-input-names-can-change-using-loaded-torchscript/6055
Split as two commits to make it easier to review:
* Simplify operator input handling
* remove outputs and output_index_map and extend use of input_vars
* Allow user supplied input names to override graph inputs
* PyTorch inputs now expected as list rather than dictionary - [(name, shape), (name, shape)...]
* Input names given with from_pytorch() are now set as the graph input names and should be used in set_input()
Review request: @masahi
----------------------------------------------------------------
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 #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607354782
cc @alexwong @jwfromm @pyjhzwh This is a API change and will break your code, but for a good reason. See the discussion above.
----------------------------------------------------------------
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 #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607334916
@jjohnson-arm need to update the tutorial too.
----------------------------------------------------------------
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 #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607343902
please update the doc here https://github.com/apache/incubator-tvm/blob/e722301a1c8be3c7052273961b8a408ca5524c76/python/tvm/relay/frontend/pytorch.py#L1434-L1436
We should warn that this names need be around until deployment time. Our suggestion is to choose something obvious, that doesn't require remembering. Something like "input0", "input1" etc
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401744457
##########
File path: python/tvm/relay/frontend/qnn_torch.py
##########
@@ -101,20 +101,19 @@ def get_weight_quant_params(script_module):
return quant_params
-def add_quant_params_to_outputs(outputs, output_index_map,
- packed_param_map, quant_params):
+def add_quant_params_to_inputs(input_vars, packed_param_map,
+ quant_params):
"""
- Add quant params to outputs so that they can be referenced by other
+ Add quant params to inputs so that they can be referenced by other
ops later. Weights are quantized here.
Review comment:
For L104 and L107, please keep `outputs`
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401741564
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1139,10 +1149,20 @@ def _get_operator_nodes(nodes):
return ops
-def _get_relay_input_vars(input_shapes):
- """ Return Relay vars from input shapes """
- return {iname: _expr.var(iname, shape=ishape)
- for iname, ishape in input_shapes.items()}
+def _get_relay_input_vars(graph, input_shapes):
+ """
+ Return Relay vars from input shapes and create entries based on
+ expected graph inputs - to allow translation
+ """
+ input_vars = {}
+ ir_inputs = _get_graph_input_names(graph)
+ for idx, ir_input in enumerate(ir_inputs):
Review comment:
How about
```
for ir_input, (name, shape) in zip(ir_inputs, input_shapes):
...
```
----------------------------------------------------------------
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] jjohnson-arm commented on issue #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
jjohnson-arm commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607778261
> @jjohnson-arm Unfortunately, you've just hit a known flaky test failure. Please comment out the get_valid_count test. See [#4901 (comment)](https://github.com/apache/incubator-tvm/pull/4901#issuecomment-595040094)
>
> Also have you verified that torch frontend tests work with this PR? I'm not sure some of the usage of `update()` method is supported.
Will comment out the test.
I have been runnning the `tests/python/frontend/pytorch/test_forward.py` tests for all my changes, and it works fine (using Python 3.6.10). Seems that python from 3.5 has supported tuples etc - https://docs.python.org/3.5/library/stdtypes.html?highlight=update#dict.update, so should be okay?
----------------------------------------------------------------
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] jjohnson-arm edited a comment on issue #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
jjohnson-arm edited a comment on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607695008
> @jjohnson-arm need to update the tutorial too.
Will look at this now.
----------------------------------------------------------------
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 #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607773765
@jjohnson-arm Unfortunately, you've just hit a known flaky test failure. Please comment out the get_valid_count test. See https://github.com/apache/incubator-tvm/pull/4901#issuecomment-595040094
Also have you verified that torch frontend tests work with this PR? I'm not sure some of the usage of `update()` method is supported.
----------------------------------------------------------------
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] jjohnson-arm commented on issue #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
jjohnson-arm commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607664632
> * Update the tutorial
> * Update the doc
> * Remove `_update_inputs_from_pairs` and use Dict's update method directly everywhere
> * Minor code style change (for loop)
> * `input_vars` -> `outputs` and `inputs` -> `outputs`.
Ok - will do. I wasn't sure about the inputs rename, happy to change it back.
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401745032
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1007,16 +1007,13 @@ def _get_input_names(node_or_graph):
return [inp.debugName() for inp in node_or_graph.inputs()]
-def _get_op_inputs(op_node, outputs, output_index_map):
- input_names = [output_index_map[name]
- for name in _get_input_names(op_node)]
- return [outputs[name] for name in input_names]
+def _get_op_inputs(op_node, input_vars):
Review comment:
`input_vars` -> `outputs`
Because inputs are not `relay.Var`.
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401731798
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1007,16 +1007,13 @@ def _get_input_names(node_or_graph):
return [inp.debugName() for inp in node_or_graph.inputs()]
-def _get_op_inputs(op_node, outputs, output_index_map):
- input_names = [output_index_map[name]
- for name in _get_input_names(op_node)]
- return [outputs[name] for name in input_names]
+def _get_op_inputs(op_node, input_vars):
+ return [input_vars[name] for name in _get_input_names(op_node)]
-def _update_outputs_from_pairs(name_output_pairs, outputs, output_index_map):
- for output_name, output in name_output_pairs:
- output_index_map[output_name] = len(outputs)
- outputs.append(output)
+def _update_inputs_from_pairs(name_input_pairs, input_vars):
Review comment:
I think we don't need this function anymore. Dict's `update` method can be used.
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi edited a comment on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607346078
I still like to retain the original meaning of `input_vars`, because this is really `relay.Var` and represents values that come from outside. `outputs` is for storing intermediate outputs from preceding relay ops.
I think you can simply replace `input_vars` with `outputs` and also `inputs` with `outputs` 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
[GitHub] [incubator-tvm] masahi commented on issue #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607346078
I still like to retain the original meaning of `input_vars`, because this is really `relay.Var` and represents values that come from outside. `outputs` is for storing intermediate outputs from preceding relay ops.
I think you can simply replace `input_vars` with `outputs` and also `inputs` with `outputs` everywhere except a few places.
----------------------------------------------------------------
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] jjohnson-arm commented on issue #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
jjohnson-arm commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607695008
> @jjohnson-arm need to update the tutorial too.
Will look at this now.
----------------------------------------------------------------
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 merged pull request #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401745032
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1007,16 +1007,13 @@ def _get_input_names(node_or_graph):
return [inp.debugName() for inp in node_or_graph.inputs()]
-def _get_op_inputs(op_node, outputs, output_index_map):
- input_names = [output_index_map[name]
- for name in _get_input_names(op_node)]
- return [outputs[name] for name in input_names]
+def _get_op_inputs(op_node, input_vars):
Review comment:
`input_vars` -> `outputs`
----------------------------------------------------------------
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 #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607796595
ok good to know. I was thinking the arg of update should be dict.
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401747683
##########
File path: tests/python/frontend/pytorch/test_forward.py
##########
@@ -890,11 +889,12 @@ def test_3d_models():
def verify_script_model(pt_model, ishapes):
script_module = torch.jit.script(pt_model)
- input_names = get_graph_input_names(script_module)
- input_shapes = dict(zip(input_names, ishapes))
- inputs = [torch.randn(input_shapes[input_name], dtype=torch.float)
- for input_name in input_names]
+ input_names = ["i{}".format(idx) for idx, ish in enumerate(ishapes)]
+ input_shapes = list(zip(input_names, ishapes))
+
+ inputs = [torch.randn(shape, dtype=torch.float)
+ for name, shape in input_shapes]
Review comment:
`for shape in ishapes`
----------------------------------------------------------------
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 #5204:
[Frontend][Torch] Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#discussion_r401743343
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -1423,30 +1438,28 @@ def from_pytorch(script_module, input_shapes, custom_convert_map=None):
op_names = get_all_op_names(graph)
_report_missing_conversion(op_names)
- _check_input_names(script_module, input_shapes)
+ _check_inputs(graph, input_shapes)
params = script_module.state_dict()
- input_vars = _get_relay_input_vars(input_shapes)
+ input_vars = _get_relay_input_vars(graph, input_shapes)
Review comment:
`input_vars` -> `outputs`
----------------------------------------------------------------
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 #5204: [Frontend][Torch]
Fix up graph input handling
Posted by GitBox <gi...@apache.org>.
masahi commented on issue #5204: [Frontend][Torch] Fix up graph input handling
URL: https://github.com/apache/incubator-tvm/pull/5204#issuecomment-607917152
Thanks @jjohnson-arm this is merged!
----------------------------------------------------------------
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