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/08/24 01:46:27 UTC
[GitHub] [incubator-tvm] masahi opened a new pull request #6327: [OpFusion] Make the max number of fused ops configurable
masahi opened a new pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327
This adds support for configuring the number of ops in one fused function. The motivation is for a use case like
[hummingbird ](https://github.com/microsoft/hummingbird) where there can be unbounded number of operations fused into one function, depending on the dataset. This leads to huge compilation time.
More details explained in
https://discuss.tvm.ai/t/aggressive-operator-fusion-and-its-consequence-of-huge-inlined-tir-expression/7687
There is already a hard coded constant kMaxFusedOps and the condition to check the number of fused ops:
https://github.com/apache/incubator-tvm/blob/a8e44710c6472a2ee5cb66283c7f5e77f4e4ca0d/src/relay/transforms/fuse_ops.cc#L82
https://github.com/apache/incubator-tvm/blob/a8e44710c6472a2ee5cb66283c7f5e77f4e4ca0d/src/relay/transforms/fuse_ops.cc#L678
But this condition is simply not correct, since for a diamond structure there are multiple paths connecting a child node (`group_node` above) and its dom parent. Correctly calculating the number of fused nodes requires more care, and I believe my implementation is correct.
Please review @tqchen @zhiics
----------------------------------------------------------------
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] junrushao1994 commented on pull request #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327#issuecomment-678901665
Thanks for the information @masahi! It is definitely helpful!
----------------------------------------------------------------
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] tqchen merged pull request #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327
----------------------------------------------------------------
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 #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327#issuecomment-678901165
> LGTM! Could you also share the compilation time impact of hummingbird by tuning this option? Thanks!
It's hard to say. I've only tested on my laptop, but without this change (unbounded fuse depth) I cannot compile for even a tiny dataset of size (20, 10) in 1 hour. Compiling for a real world dataset is infeasible.
With configurable depth, the time it takes to run `relay.build` on the dataset of size (20, 10) is roughly:
* max fuse depth = 15: 1 sec
* max fuse depth = 50: 10 sec
----------------------------------------------------------------
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] tqchen commented on pull request #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327#issuecomment-679342779
Thanks @masahi @zhiics @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
[GitHub] [incubator-tvm] masahi commented on a change in pull request #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327#discussion_r475603574
##########
File path: src/relay/transforms/fuse_ops.cc
##########
@@ -83,6 +83,8 @@ constexpr uint32_t kMaxFusedOps = 256;
static const Op& stop_fusion_op = Op::Get("annotation.stop_fusion");
+TVM_REGISTER_PASS_CONFIG_OPTION("relay.max_fuse_depth", Integer);
Review comment:
done
----------------------------------------------------------------
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] tqchen commented on a change in pull request #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327#discussion_r475599518
##########
File path: src/relay/transforms/fuse_ops.cc
##########
@@ -83,6 +83,8 @@ constexpr uint32_t kMaxFusedOps = 256;
static const Op& stop_fusion_op = Op::Get("annotation.stop_fusion");
+TVM_REGISTER_PASS_CONFIG_OPTION("relay.max_fuse_depth", Integer);
Review comment:
consider use the naming convention: `relay.PassName.option_name`
`relay.FuseOps.max_depth`
----------------------------------------------------------------
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 edited a comment on pull request #6327: [OpFusion] Make the max number of fused ops configurable
Posted by GitBox <gi...@apache.org>.
masahi edited a comment on pull request #6327:
URL: https://github.com/apache/incubator-tvm/pull/6327#issuecomment-678901165
> LGTM! Could you also share the compilation time impact of hummingbird by tuning this option? Thanks!
It's hard to say. I've only tested on my laptop, but without this change (unbounded fuse depth) I cannot compile for even a tiny dataset of size (20, 10) in 1 hour. Compiling for a real world dataset is infeasible.
With configurable depth, the time it takes to run `relay.build` on the dataset of size (20, 10) is roughly:
* max fuse depth = 15: 1 sec
* max fuse depth = 50: 10 sec
Note that with this change, fusion is no longer a problem for compiling hummingbird models, no matter what the dataset size is. I can now compile for the dataset of size (500, 10), but (1000, 10) gives a segfault for a different reason not related to fusion.
----------------------------------------------------------------
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