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 2021/03/13 01:06:17 UTC
[GitHub] [tvm] comaniac opened a new pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
comaniac opened a new pull request #7656:
URL: https://github.com/apache/tvm/pull/7656
This PR adds a simplify pattern to the SImplifyExpr pass. The pattern simplifies back-to-back transpose and layout_transform ops, which can be introduced by Relay frontends or ConvertLayout pass.
cc @mbrookhart @icemelon9 @anijain2305
----------------------------------------------------------------
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] [tvm] comaniac commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-798769277
It seems to me that the current pattern already covers this case? You can refer to the test cases I added, which have a ReLU followed by transposes. Is this what you want?
----------------------------------------------------------------
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] [tvm] ANSHUMAN87 commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-797873713
Overall pretty good for me.
I just have 2 points which needs clarification:
1:> When the axes has negative values, we should handle it.
2:> Now we are just looking at the start of the chain of transpose, i suggest we look at intermediate residue as well.
Please let me know in case my points are not clear. 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] [tvm] ANSHUMAN87 commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-798933062
> It seems to me that the current pattern already covers this case? You can refer to the test cases I added, which have a ReLU followed by transposes. Is this what you want?
Thanks @comaniac for clarification. It is indeed supported.
Can we add 1 more test case as below, it will help reviewer to understand the flow is supported:
```
def before4():
x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
y = relay.nn.relu(x)
#y = relay.transpose(y, axes=[0, 2, 3, 1])
y = relay.transpose(y) # Reverse
y = relay.transpose(y) # Reverse
y = relay.transpose(y, axes=[0, 2, 3, 1])
y = relay.transpose(y) # Reverse
y = relay.transpose(y) # Reverse
return relay.Function([x], y)
def expected4():
x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
y = relay.nn.relu(x)
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
```
----------------------------------------------------------------
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] [tvm] comaniac commented on a change in pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#discussion_r594544046
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -60,6 +60,54 @@ def symbolic():
assert tvm.ir.structural_equal(zz, after)
+def test_simplify_transpose():
+ def before1():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.transpose(x, axes=[0, 2, 3, 1]) # To NHWC
+ y = relay.layout_transform(y, "NHWC", "HWCN") # To HWCN
+ y = relay.transpose(y, axes=[3, 0, 1, 2]) # To NHWC
+ return relay.Function([x], y)
+
+ def expected1():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.transpose(x, axes=[0, 2, 3, 1]) # To NHWC
+ return relay.Function([x], y)
+
+ def before2():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ y = relay.transpose(y, axes=[0, 2, 3, 1]) # To NHWC
+ y = relay.transpose(y, axes=[1, 2, 3, 0]) # To HWCN
+ y = relay.transpose(y, axes=[3, 2, 0, 1]) # To NCHW
+ return relay.Function([x], y)
+
+ def expected2():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before3():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ y = relay.transpose(y) # Reverse
+ y = relay.transpose(y) # Reverse
+ return relay.Function([x], y)
+
+ def expected3():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ for before, expected in [
+ [before1(), expected1()],
+ [before2(), expected2()],
+ [before3(), expected3()],
+ ]:
+ after = run_opt_pass(before, transform.SimplifyExpr())
+ expected = run_opt_pass(expected, transform.InferType())
+ assert tvm.ir.structural_equal(after, expected)
Review comment:
Changed.
----------------------------------------------------------------
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] [tvm] ANSHUMAN87 commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-798144818
> > 2:> Now we are just looking at the start of the chain of transpose, i suggest we look at intermediate residue as well.
>
> Could you provide an example of this case for better illustration? Thanks.
For example we have below case:
Expression:
y = nn.conv(x)
y = transpose(y, axes=0, 3, 1, 2)
y = transpose(y)
y = transpose(y)
Expected transformation:
y = nn.conv(x)
y = transpose(y, axes=0, 3, 1, 2)
Above such cases can be multiple in a sequence as well. So we need to have a solution which can handle multiple simplification in one chain. Please let me know, in case i have not clearly expressed. 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] [tvm] ANSHUMAN87 commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-797870713
Thanks @comaniac for the PR! I was also working on similar approach. This solves my case 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
[GitHub] [tvm] comaniac commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-799633968
> One possible extension would be to try to combine this with the simplify reshape above and target patterns like reshape->transpose->reshape or transpose->reshape->transpose
That's an interesting point and I just thought about it. However, reshape and transpose cannot be equivalent in any cases, because reshape just changes the definition of accessing the tensor; while transpose reorganizes the element order in memory. Even we combine them to the same patterm, we still need to deal with each case separately and result in no code reuse. Please let me know if I missed something. 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] [tvm] comaniac commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-797881808
Thanks for the comments.
> 1:> When the axes has negative values, we should handle it.
Good point. I'll add the support for it later.
> 2:> Now we are just looking at the start of the chain of transpose, i suggest we look at intermediate residue as well.
Could you provide an example of this case for better illustration? 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] [tvm] comaniac commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-799869778
Thanks @mbrookhart @ANSHUMAN87
----------------------------------------------------------------
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] [tvm] comaniac commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-799609488
@ANSHUMAN87 comments are addressed.
Also cc @mbrookhart @icemelon9 @anijain2305
----------------------------------------------------------------
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] [tvm] comaniac merged pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #7656:
URL: https://github.com/apache/tvm/pull/7656
----------------------------------------------------------------
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] [tvm] ANSHUMAN87 commented on a change in pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#discussion_r593922096
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -60,6 +60,54 @@ def symbolic():
assert tvm.ir.structural_equal(zz, after)
+def test_simplify_transpose():
+ def before1():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.transpose(x, axes=[0, 2, 3, 1]) # To NHWC
+ y = relay.layout_transform(y, "NHWC", "HWCN") # To HWCN
+ y = relay.transpose(y, axes=[3, 0, 1, 2]) # To NHWC
+ return relay.Function([x], y)
+
+ def expected1():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.transpose(x, axes=[0, 2, 3, 1]) # To NHWC
+ return relay.Function([x], y)
+
+ def before2():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ y = relay.transpose(y, axes=[0, 2, 3, 1]) # To NHWC
+ y = relay.transpose(y, axes=[1, 2, 3, 0]) # To HWCN
+ y = relay.transpose(y, axes=[3, 2, 0, 1]) # To NCHW
+ return relay.Function([x], y)
+
+ def expected2():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before3():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ y = relay.transpose(y) # Reverse
+ y = relay.transpose(y) # Reverse
+ return relay.Function([x], y)
+
+ def expected3():
+ x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ for before, expected in [
+ [before1(), expected1()],
+ [before2(), expected2()],
+ [before3(), expected3()],
+ ]:
+ after = run_opt_pass(before, transform.SimplifyExpr())
+ expected = run_opt_pass(expected, transform.InferType())
+ assert tvm.ir.structural_equal(after, expected)
Review comment:
Can we change it to like below:
assert tvm.ir.structural_equal(after, expected), "\nafter: {} \nexpected: {}".format(after, expected)
It will help when fails.
----------------------------------------------------------------
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] [tvm] mbrookhart commented on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-799649960
You're right. I think a set of modular patterns is probably best
----------------------------------------------------------------
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] [tvm] ANSHUMAN87 edited a comment on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 edited a comment on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-798933062
> It seems to me that the current pattern already covers this case? You can refer to the test cases I added, which have a ReLU followed by transposes. Is this what you want?
Thanks @comaniac for clarification. It is indeed supported.
Can we add 1 more test case as below, it will help reviewer to understand the flow is supported:
```
def before4():
x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
y = relay.nn.relu(x)
y = relay.transpose(y) # Reverse
y = relay.transpose(y) # Reverse
y = relay.transpose(y, axes=[0, 2, 3, 1])
y = relay.transpose(y) # Reverse
y = relay.transpose(y) # Reverse
return relay.Function([x], y)
def expected4():
x = relay.var("x", shape=(1, 3, 224, 224), dtype="float32") # NCHW
y = relay.nn.relu(x)
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
```
----------------------------------------------------------------
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] [tvm] ANSHUMAN87 edited a comment on pull request #7656: [Relay][Pass] Simplify consecutive transpose/layout_transform
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 edited a comment on pull request #7656:
URL: https://github.com/apache/tvm/pull/7656#issuecomment-798144818
> > 2:> Now we are just looking at the start of the chain of transpose, i suggest we look at intermediate residue as well.
>
> Could you provide an example of this case for better illustration? Thanks.
For example we have below case:
Expression:
y = nn.conv(x)
y = transpose(y, axes=0, 3, 1, 2)
y = transpose(y)
y = transpose(y)
Expected transformation:
y = nn.conv(x)
y = transpose(y, axes=0, 3, 1, 2)
Above such cases can be multiple in a sequence as well. So we need to have a solution which can handle multiple simplification in one chain. Please let me know, in case i have not clearly expressed. Thanks!
I understand we can have these points covered in follow up PRs as well. If we are not planning to do it in this PR. Suggest to add TODOs for these points in the source code, which will help in future.
----------------------------------------------------------------
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