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/07/23 06:13:58 UTC
[GitHub] [incubator-tvm] zhanghaohit opened a new pull request #6124: change device annotation from post DFS to recursive
zhanghaohit opened a new pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124
This is related to #5840 and split from PR #5842
Originally, device type is propagated based on the post DFS traversed graph, which may not be consistent if the argument order changes. In addition, it may handle some cases wrongly, e.g., the first residual block in Resnet50. The first few layers in Resnet50 are depicted in the following figure (top to bottom is in DFS order). Basically, we want to let all the layers run on FPGA device, except the first and last few layers. In the original device propagation algorithm, based on the post DFS order, the conv2d layers in grey will be propagated with `CPU` device type as we encounter `copy2` first, following which the three grey conv2d nodes are marked as the source device type of `copy2` (i.e., `CPU`), which is not correct.
<img src="https://raw.githubusercontent.com/4paradigm/incubator-tvm/feature/images/docs/resnet50.png"
alt="Resnet50"
width=300
style="float: centre; margin-left: 50px;" />
By change the device annotation behaviour, we can support more complex graph structure.
----------------------------------------------------------------
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] tmoreau89 merged pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
tmoreau89 merged pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124
----------------------------------------------------------------
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] jroesch commented on pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#issuecomment-662877111
cc @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] [incubator-tvm] zhanghaohit closed pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
zhanghaohit closed pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124
----------------------------------------------------------------
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] zhanghaohit commented on pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
zhanghaohit commented on pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#issuecomment-665540228
----------------------------------------------------------------
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] mbrookhart commented on a change in pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#discussion_r464491292
##########
File path: tests/python/relay/test_pass_annotation.py
##########
@@ -309,6 +309,76 @@ def test_visitor_annotation():
test_visitor_annotation()
+def test_propogation():
+ R""" The network and device type is as following:
+ x 1
+ |
+ log 1
+ / \
+ log2 log10 2
+ \ /
+ add 2
+ |
+ tan 1
+ """
+ ctx1 = tvm.context(1)
+ ctx2 = tvm.context(2)
+
+ expected_dev_type = {
+ 'log': ctx1,
+ 'log2': ctx2,
+ 'log10': ctx2,
+ 'add': ctx2,
+ 'tan': ctx1
+ }
+
+ x = relay.var("x", shape=(3,))
+
+ def annotated():
+ log = relay.log(x)
+ _log = relay.annotation.on_device(log, expected_dev_type['log'])
+ log2 = relay.log2(_log)
+ _log2 = relay.annotation.on_device(log2, expected_dev_type['log2'])
+ log10 = relay.log10(_log)
+ _log10 = relay.annotation.on_device(log10, expected_dev_type['log10'])
+ add = relay.add(_log2, _log10)
+ _add = relay.annotation.on_device(add, expected_dev_type['add'])
+ tan = relay.tan(_add)
+ _tan = relay.annotation.on_device(tan, expected_dev_type['tan'])
+
+ func = run_opt_pass(_tan, transform.RewriteAnnotatedOps(ctx1.device_type))
+ return func
+
+ def expected():
+ log = relay.log(x)
+ _log_left = relay.device_copy(log, ctx1, ctx2)
+ _log_right = relay.device_copy(log, ctx1, ctx2)
+ log2 = relay.log2(_log_left)
+ log10 = relay.log10(_log_right)
+ add = relay.add(log2, log10)
+ _add = relay.device_copy(add, ctx2, ctx1)
+ tan = relay.tan(_add)
+
+ func = run_opt_pass(tan, transform.InferType())
+ return func
+
+ annotated_expr = annotated()
+ expected_expr = expected()
+ assert tvm.ir.structural_equal(annotated_expr, expected_expr)
Review comment:
:+1: You're right, I misread my first 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
[GitHub] [incubator-tvm] zhiics commented on pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
zhiics commented on pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#issuecomment-668093739
please trigger the ci again.
----------------------------------------------------------------
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] zhanghaohit commented on pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
zhanghaohit commented on pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#issuecomment-675833690
@tmoreau89 Could you help merge this PR? Thanks.
----------------------------------------------------------------
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] zhanghaohit commented on a change in pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
zhanghaohit commented on a change in pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#discussion_r462690226
##########
File path: tests/python/relay/test_pass_annotation.py
##########
@@ -309,6 +309,76 @@ def test_visitor_annotation():
test_visitor_annotation()
+def test_propogation():
+ R""" The network and device type is as following:
+ x 1
+ |
+ log 1
+ / \
+ log2 log10 2
+ \ /
+ add 2
+ |
+ tan 1
+ """
+ ctx1 = tvm.context(1)
+ ctx2 = tvm.context(2)
+
+ expected_dev_type = {
+ 'log': ctx1,
+ 'log2': ctx2,
+ 'log10': ctx2,
+ 'add': ctx2,
+ 'tan': ctx1
+ }
+
+ x = relay.var("x", shape=(3,))
+
+ def annotated():
+ log = relay.log(x)
+ _log = relay.annotation.on_device(log, expected_dev_type['log'])
+ log2 = relay.log2(_log)
+ _log2 = relay.annotation.on_device(log2, expected_dev_type['log2'])
+ log10 = relay.log10(_log)
+ _log10 = relay.annotation.on_device(log10, expected_dev_type['log10'])
+ add = relay.add(_log2, _log10)
+ _add = relay.annotation.on_device(add, expected_dev_type['add'])
+ tan = relay.tan(_add)
+ _tan = relay.annotation.on_device(tan, expected_dev_type['tan'])
+
+ func = run_opt_pass(_tan, transform.RewriteAnnotatedOps(ctx1.device_type))
+ return func
+
+ def expected():
+ log = relay.log(x)
+ _log_left = relay.device_copy(log, ctx1, ctx2)
+ _log_right = relay.device_copy(log, ctx1, ctx2)
+ log2 = relay.log2(_log_left)
+ log10 = relay.log10(_log_right)
+ add = relay.add(log2, log10)
+ _add = relay.device_copy(add, ctx2, ctx1)
+ tan = relay.tan(_add)
+
+ func = run_opt_pass(tan, transform.InferType())
+ return func
+
+ annotated_expr = annotated()
+ expected_expr = expected()
+ assert tvm.ir.structural_equal(annotated_expr, expected_expr)
Review comment:
I think master fails on line 379, right? the device type of `log2` is not correctly marked.
Up to this line, `annotated_expr` and `expected_expr` are exactly the same. The `device_copy` op is inserted correctly. We haven't go through the device propagation yet.
----------------------------------------------------------------
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] tmoreau89 commented on pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
tmoreau89 commented on pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#issuecomment-676540442
Thank you @zhanghaohit , @mbrookhart, @junrushao1994, @zhiics the PR has been 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] zhanghaohit removed a comment on pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
zhanghaohit removed a comment on pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#issuecomment-665540228
> Thanks @zhanghaohit for this PR, I second @mbrookhart's request to add a test case.
----------------------------------------------------------------
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] mbrookhart commented on a change in pull request #6124: [Relay] change device annotation from post DFS to recursive
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on a change in pull request #6124:
URL: https://github.com/apache/incubator-tvm/pull/6124#discussion_r462608830
##########
File path: src/relay/transforms/device_annotation.cc
##########
@@ -414,22 +427,23 @@ class DeviceInfo {
void VisitExpr_(const TupleGetItemNode* op) final { ExprVisitor::VisitExpr_(op); }
void VisitExpr_(const VarNode* vn) final {
- post_dfs_order_.push_back(std::make_pair(vn, has_copy_));
+ device_tag_[vn] = dev_type_;
}
void VisitExpr_(const LetNode* ln) final {
ExprVisitor::VisitExpr_(ln);
- post_dfs_order_.push_back(std::make_pair(ln, has_copy_));
+ device_tag_[ln] = dev_type_;
}
void VisitExpr_(const IfNode* in) final {
ExprVisitor::VisitExpr_(in);
- post_dfs_order_.push_back(std::make_pair(in, has_copy_));
+ device_tag_[in] = dev_type_;
}
int num_device_copy_ops_{0};
- bool has_copy_ = false;
- std::vector<std::pair<const ExprNode*, bool>> post_dfs_order_;
+ int dev_type_ = -1;
+ int out_dev_type_ = -1;
Review comment:
I don't love making this class state, precisely because you have to do crazy things with maintaining state in your recursive calls. You could do it as a set of recursive arguments, but that kind of requires re-implementing with ExprFunctor...so maybe this is the cleanest solution.
##########
File path: tests/python/relay/test_pass_annotation.py
##########
@@ -309,6 +309,76 @@ def test_visitor_annotation():
test_visitor_annotation()
+def test_propogation():
+ R""" The network and device type is as following:
+ x 1
+ |
+ log 1
+ / \
+ log2 log10 2
+ \ /
+ add 2
+ |
+ tan 1
+ """
+ ctx1 = tvm.context(1)
+ ctx2 = tvm.context(2)
+
+ expected_dev_type = {
+ 'log': ctx1,
+ 'log2': ctx2,
+ 'log10': ctx2,
+ 'add': ctx2,
+ 'tan': ctx1
+ }
+
+ x = relay.var("x", shape=(3,))
+
+ def annotated():
+ log = relay.log(x)
+ _log = relay.annotation.on_device(log, expected_dev_type['log'])
+ log2 = relay.log2(_log)
+ _log2 = relay.annotation.on_device(log2, expected_dev_type['log2'])
+ log10 = relay.log10(_log)
+ _log10 = relay.annotation.on_device(log10, expected_dev_type['log10'])
+ add = relay.add(_log2, _log10)
+ _add = relay.annotation.on_device(add, expected_dev_type['add'])
+ tan = relay.tan(_add)
+ _tan = relay.annotation.on_device(tan, expected_dev_type['tan'])
+
+ func = run_opt_pass(_tan, transform.RewriteAnnotatedOps(ctx1.device_type))
+ return func
+
+ def expected():
+ log = relay.log(x)
+ _log_left = relay.device_copy(log, ctx1, ctx2)
+ _log_right = relay.device_copy(log, ctx1, ctx2)
+ log2 = relay.log2(_log_left)
+ log10 = relay.log10(_log_right)
+ add = relay.add(log2, log10)
+ _add = relay.device_copy(add, ctx2, ctx1)
+ tan = relay.tan(_add)
+
+ func = run_opt_pass(tan, transform.InferType())
+ return func
+
+ annotated_expr = annotated()
+ expected_expr = expected()
+ assert tvm.ir.structural_equal(annotated_expr, expected_expr)
Review comment:
I'm curious that master passes this check, but fails on line 377. Why doesn't structural equal properly resolve the error in the device copy op?
----------------------------------------------------------------
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