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/09/01 22:07:18 UTC
[GitHub] [incubator-tvm] masahi opened a new pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
masahi opened a new pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374
----------------------------------------------------------------
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] [incubator-tvm] siju-samuel commented on pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
siju-samuel commented on pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374#issuecomment-685830256
Thanks @masahi @t-vi . This PR 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
[GitHub] [incubator-tvm] siju-samuel merged pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
siju-samuel merged pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374
----------------------------------------------------------------
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] [incubator-tvm] t-vi commented on a change in pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
t-vi commented on a change in pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374#discussion_r481836239
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -110,12 +102,20 @@ def inplace_add_to_add(op_name):
if len(intersect) > 0 and intersect != set(["aten::add"]):
return True
- if is_used_by_list_add(filter(lambda use: use.user.kind() != "prim::Loop", uses)):
- return True
+ # if add op outputs list, it is dynamic so we need to construct List ADT
+ for use in filter(lambda use: use.user.kind() in ["aten::add", "aten::add_"], uses):
Review comment:
I'm a bit weary that this is very brittle. Probably not more than before, though.
----------------------------------------------------------------
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] [incubator-tvm] masahi commented on a change in pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374#discussion_r481849518
##########
File path: python/tvm/relay/frontend/pytorch.py
##########
@@ -110,12 +102,20 @@ def inplace_add_to_add(op_name):
if len(intersect) > 0 and intersect != set(["aten::add"]):
return True
- if is_used_by_list_add(filter(lambda use: use.user.kind() != "prim::Loop", uses)):
- return True
+ # if add op outputs list, it is dynamic so we need to construct List ADT
+ for use in filter(lambda use: use.user.kind() in ["aten::add", "aten::add_"], uses):
Review comment:
Yes, with this PR `_should_construct_dynamic_list` runs more often. I got an error from this function when `_get_node_type` was called on a op which returns multiple output. I've cleaned up this function a bit so that `_get_node_type` will only run on `aten::add` and `aten::add_`. So in that sense it is no worse than before.
I agree that this function is a bit hacky. I added this when I was trying to support Python list. It works on the test cases I was working on, but probably it is not robust.
----------------------------------------------------------------
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] [incubator-tvm] t-vi commented on pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
t-vi commented on pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374#issuecomment-685453781
Yeah, the thing that makes it ad hoc in my eyes is that
- we don't have a view as which ops all lead to dynamic lists (e.g. append),
- very likely we should have some criterion for recursion (e.g. handling slicing, which might be not that uncommon when you think of some "manual broadcasting"-style things).
But again, I see the PR as a good improvement over the status quo and so I think its good to have. Thank you!
----------------------------------------------------------------
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] [incubator-tvm] t-vi edited a comment on pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
t-vi edited a comment on pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374#issuecomment-685453781
Yeah, the thing that makes it ad hoc in my eyes is that
- we don't have a view as which ops all lead to dynamic lists (e.g. append),
- very likely we should have some criterion for recursion (e.g. handling slicing, which might be not that uncommon when you think of some "manual broadcasting"-style things).
But again, I see the PR as a good improvement over the status quo and so I think its good to have. Thank you!
(Also it's cool that you scan the hummingbird issues, I think this kind of thing really is key to people taking up TVM.)
----------------------------------------------------------------
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] [incubator-tvm] masahi commented on pull request #6374: [Torch] Support logsumexp, clean up unnecessary infer_shape usage
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6374:
URL: https://github.com/apache/incubator-tvm/pull/6374#issuecomment-685422732
I should clarify that, with this change, `prim::ListConstruct` converts to either
* List ADT (dynamic case, e.g. when the list is appended a new item at runtime)
* List of integer or tensors (static case, preserve Python list during conversion)
----------------------------------------------------------------
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