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/04/07 18:59:11 UTC
[GitHub] [tvm] csullivan opened a new pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
csullivan opened a new pull request #7807:
URL: https://github.com/apache/tvm/pull/7807
This PR adds support to SimplifyTranspose to simplify consecutive transpose<->layout_transform operations when the layout transform involves a rank expansion or contraction. The added unit test fails on main because the current SimplifyTranspose only doesn't consider the case when the layout transform and transpose may act on tensors of different rank. As an example,
`NCHW4c -> LayoutTransform -> NCHW -> Transpose -> NHWC`
now simplifies to
`NCHW4c -> LayoutTransform -> NHWC`.
The reverse, rank expansion, is also 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
[GitHub] [tvm] comaniac merged pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
comaniac merged pull request #7807:
URL: https://github.com/apache/tvm/pull/7807
--
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] csullivan commented on pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
csullivan commented on pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#issuecomment-823682676
Thanks @comaniac @ANSHUMAN87! I've extended the support for various rank changing layout transform + transpose patterns. Please see the additional tests for an overview and let me know other change requests you may have.
--
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] csullivan commented on pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
csullivan commented on pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#issuecomment-815150872
cc @comaniac @ANSHUMAN87 @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] [tvm] csullivan commented on a change in pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617116749
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
Review comment:
I added some checks, please let me know if you find them sufficient.
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617697768
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,28 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
Review comment:
Thanks. Looks great 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
[GitHub] [tvm] comaniac commented on a change in pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r608943282
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -91,36 +91,15 @@ class SimplifyTranspose : public DFPatternRewrite {
Expr Callback(const Expr& pre, const Expr& post,
const Map<DFPattern, Array<Expr>>& node_map) const override {
// Helper function to get the axes from call node attribute
- auto get_axes_from_call = [](const Call trans_call, int ndim) {
- std::vector<int> attr_axes;
- if (auto attr = trans_call->attrs.as<TransposeAttrs>()) {
- if (attr->axes.defined()) {
- for (int i = 0; i < ndim; ++i) {
- int64_t axis = attr->axes[i];
- axis += (axis < 0) ? ndim : 0;
- attr_axes.push_back(axis);
- }
- } else {
- // Empty axes means reverse
- for (int i = ndim - 1; i >= 0; --i) {
- attr_axes.push_back(i);
- }
- }
- } else if (auto attr = trans_call->attrs.as<LayoutTransformAttrs>()) {
- Layout src_layout(attr->src_layout);
- Layout dst_layout(attr->dst_layout);
- for (int i = 0; i < ndim; ++i) {
- attr_axes.push_back(src_layout.IndexOf(dst_layout[i]));
- }
- } else {
- CHECK(false) << "Expected transpose or layout_transform, but got "
- << Downcast<Op>(trans_call->op)->name;
- }
- return std::move(attr_axes);
- };
auto x = node_map[x_][0];
+ Call trans_call = Downcast<Call>(post);
+
+ if (auto layout_trans = FoldRankChangingLayoutTrans(x, trans_call)) {
Review comment:
Add comments.
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -91,36 +91,15 @@ class SimplifyTranspose : public DFPatternRewrite {
Expr Callback(const Expr& pre, const Expr& post,
const Map<DFPattern, Array<Expr>>& node_map) const override {
// Helper function to get the axes from call node attribute
Review comment:
Remove this comment as there's no helper function defined here anymore.
##########
File path: src/relay/transforms/type_infer.cc
##########
@@ -741,7 +741,6 @@ class TypeInferencer::Resolver : public MixedModeMutator, PatternMutator {
Expr TypeInferencer::Infer(GlobalVar var, Function function) {
// Set the current function being type checked.
this->current_func_ = var;
-
Review comment:
Mis-deleted?
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
+ std::string old_layout{layout};
+ for (auto axis : axes) {
+ new_layout += old_layout[axis];
+ }
+ return String(new_layout);
+ }
+
+ Optional<Expr> FoldRankChangingLayoutTrans(const Expr& data, const Call& call) const {
+ Optional<Expr> layout_trans;
+ if (auto attr = call->attrs.as<LayoutTransformAttrs>()) {
+ Layout src_layout(attr->src_layout);
+ Layout dst_layout(attr->dst_layout);
+ if (src_layout->axes.size() != dst_layout->axes.size()) {
Review comment:
It would be better to lift this logic to the beginning of this function so that people can clearly know this function only dealing with the case that source and destination layout axis numbers are mismatching.
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
+ std::string old_layout{layout};
+ for (auto axis : axes) {
+ new_layout += old_layout[axis];
+ }
+ return String(new_layout);
+ }
+
+ Optional<Expr> FoldRankChangingLayoutTrans(const Expr& data, const Call& call) const {
+ Optional<Expr> layout_trans;
+ if (auto attr = call->attrs.as<LayoutTransformAttrs>()) {
+ Layout src_layout(attr->src_layout);
+ Layout dst_layout(attr->dst_layout);
+ if (src_layout->axes.size() != dst_layout->axes.size()) {
+ auto axes = GetTransposeAxisOrder(Downcast<Call>(call->args[0]), src_layout->axes.size());
+ std::vector<int> inverse(axes.size());
+ for (size_t i = 0; i < axes.size(); i++) {
+ inverse[axes[i]] = i;
+ }
+ String new_layout = PermuteLayout(attr->src_layout, inverse);
+ layout_trans = MakeLayoutTransform(data, new_layout, dst_layout->name);
+ }
+ } else if (auto attr = Downcast<Call>(call->args[0])->attrs.as<LayoutTransformAttrs>()) {
Review comment:
I'm not sure if `call->args[0]->attrs` is guaranteed to be available. May need examples for `if` and `else if` here to help understand the handled cases.
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
+ std::string old_layout{layout};
+ for (auto axis : axes) {
+ new_layout += old_layout[axis];
+ }
+ return String(new_layout);
+ }
+
+ Optional<Expr> FoldRankChangingLayoutTrans(const Expr& data, const Call& call) const {
Review comment:
Please add comments to explain each step in this function.
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
mbrookhart commented on pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#issuecomment-831340178
@csullivan Thanks for the contribution. What's the status on this? I looks like it has a lint error.
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617695429
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,112 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
+ def before4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.transpose(x, axes=[0, 3, 1, 2]) # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.transpose(y, axes=[0, 2, 3, 1]) # To NHWC
+ return relay.Function([x], y)
+
+ def expected4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def expected5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NCHW", "NHWC")
+ y = relay.layout_transform(y, "NHWC", "NCHW")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before7():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW8c")
+ y = relay.layout_transform(y, "NCHW8c", "NCHW4c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected7():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before8():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW")
+ y = relay.layout_transform(y, "NCHW", "NCHW8c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected8():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW8c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def before9():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NCHW", "NCHW4c") # To NCHW4c
Review comment:
The comment for x say the layout is # NHWC , but the transform below does from "NCHW" --> "NCHW4c".
It does not match. Is it intended ?
Same case in test_case 6 also.
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#issuecomment-831757808
Thanks @csullivan @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] csullivan commented on a change in pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617114300
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,28 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
Review comment:
I added many more variants :), please review them
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617695429
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,112 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
+ def before4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.transpose(x, axes=[0, 3, 1, 2]) # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.transpose(y, axes=[0, 2, 3, 1]) # To NHWC
+ return relay.Function([x], y)
+
+ def expected4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def expected5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NCHW", "NHWC")
+ y = relay.layout_transform(y, "NHWC", "NCHW")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before7():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW8c")
+ y = relay.layout_transform(y, "NCHW8c", "NCHW4c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected7():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before8():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW")
+ y = relay.layout_transform(y, "NCHW", "NCHW8c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected8():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW8c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def before9():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NCHW", "NCHW4c") # To NCHW4c
Review comment:
The comment for x say the layout is # NHWC , but the transform below does from "NCHW" --> "NCHW4c".
It does not match. Is it intended ?
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617687443
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
Review comment:
Its not important, i will leave it to your call :slightly_smiling_face:
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
comaniac commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617736990
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,112 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
+ def before4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.transpose(x, axes=[0, 3, 1, 2]) # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.transpose(y, axes=[0, 2, 3, 1]) # To NHWC
+ return relay.Function([x], y)
+
+ def expected4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def expected5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
Review comment:
According to the following transforms this should be
```suggestion
x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NCHW
```
but its shape looks like NHWC. Please correct either of them.
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617700835
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
Review comment:
Looks great 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
[GitHub] [tvm] ANSHUMAN87 commented on a change in pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r609786864
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
Review comment:
nit: Can we use String instead of std::string ?
Just to maintain uniformity.
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
Review comment:
I find this is prone for errors.
So can we add list of possible values allowed in layout ? If not we can add some validation before accessing layout.
Also i think it would be good to add debug logs here as the layout has changed from x --> y.
It will help detect errors in future developments.
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,28 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
Review comment:
This test case looks great. Pretty clear to understand what is happening.
I think it would be great we can add one more fusion scenario as well, like some of the test cases before and the new one.
Please let me know in case i am not clear.
--
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] csullivan commented on a change in pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617113932
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
Review comment:
AFAICT it would require extra string copying and more code to use String here due to the in place updates. But if you feel strongly about this we can do it.
--
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 #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617694296
##########
File path: tests/python/relay/test_pass_simplify_expr.py
##########
@@ -106,10 +106,112 @@ def expected3():
y = relay.transpose(y, axes=[0, 2, 3, 1])
return relay.Function([x], y)
+ # Test a series of transpose and rank changing layout_transform
+ def before4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.transpose(x, axes=[0, 3, 1, 2]) # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.transpose(y, axes=[0, 2, 3, 1]) # To NHWC
+ return relay.Function([x], y)
+
+ def expected4():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.layout_transform(y, "NCHW", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def expected5():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NHWC", "NCHW4c") # To NCHW4c
+ y = relay.nn.relu(y)
+ y = relay.layout_transform(y, "NCHW4c", "NHWC") # To NHWC
+ return relay.Function([x], y)
+
+ def before6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NCHW", "NHWC")
+ y = relay.layout_transform(y, "NHWC", "NCHW")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected6():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before7():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW8c")
+ y = relay.layout_transform(y, "NCHW8c", "NCHW4c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected7():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before8():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW")
+ y = relay.layout_transform(y, "NCHW", "NCHW8c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected8():
+ x = relay.var("x", shape=(1, 32, 56, 56, 4), dtype="float32") # NCHW4c
+ y = relay.layout_transform(x, "NCHW4c", "NCHW8c")
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def before9():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.layout_transform(x, "NCHW", "NCHW4c") # To NCHW4c
+ y = relay.layout_transform(y, "NCHW4c", "NCHW") # To NCHW
+ y = relay.nn.relu(y)
+ return relay.Function([x], y)
+
+ def expected9():
+ x = relay.var("x", shape=(1, 56, 56, 128), dtype="float32") # NHWC
+ y = relay.nn.relu(x)
+ return relay.Function([x], y)
+
+ def before10():
+ x = relay.var("x", shape=(1, 128, 56, 56), dtype="float32") # NHWC
Review comment:
```suggestion
x = relay.var("x", shape=(1, 128, 56, 56), dtype="float32") # NCHW
```
--
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] csullivan commented on a change in pull request #7807: [Relay][Pass] Update SimplifyTranspose to correctly simplify rank changing layout transforms
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #7807:
URL: https://github.com/apache/tvm/pull/7807#discussion_r617118760
##########
File path: src/relay/transforms/simplify_expr.cc
##########
@@ -163,6 +141,69 @@ class SimplifyTranspose : public DFPatternRewrite {
return x;
}
+ String PermuteLayout(const String& layout, std::vector<int> axes) const {
+ std::string new_layout{};
+ std::string old_layout{layout};
+ for (auto axis : axes) {
+ new_layout += old_layout[axis];
+ }
+ return String(new_layout);
+ }
+
+ Optional<Expr> FoldRankChangingLayoutTrans(const Expr& data, const Call& call) const {
+ Optional<Expr> layout_trans;
+ if (auto attr = call->attrs.as<LayoutTransformAttrs>()) {
+ Layout src_layout(attr->src_layout);
+ Layout dst_layout(attr->dst_layout);
+ if (src_layout->axes.size() != dst_layout->axes.size()) {
+ auto axes = GetTransposeAxisOrder(Downcast<Call>(call->args[0]), src_layout->axes.size());
+ std::vector<int> inverse(axes.size());
+ for (size_t i = 0; i < axes.size(); i++) {
+ inverse[axes[i]] = i;
+ }
+ String new_layout = PermuteLayout(attr->src_layout, inverse);
+ layout_trans = MakeLayoutTransform(data, new_layout, dst_layout->name);
+ }
+ } else if (auto attr = Downcast<Call>(call->args[0])->attrs.as<LayoutTransformAttrs>()) {
Review comment:
Attrs is guaranteed to be available for a Call, no? Are you worried about arg[0] not being of type Call? I believe the latter is guaranteed by the pattern matcher in this case. Let me know if I missed your meaning
--
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