[RELAY][OP] conv2d, ShapeExpr->IndexExpr #1798

tqchen · 2018-10-02T23:31:27Z

Changed ShapeExpr->IndexExpr as it is mostly used for index
port conv2d from nnvm spec, with the following changes:
- kernel_size, channels are no longer mandatory
- layout->data_layout
- kernel_layout -> weight_layout(to be consistent with the weight, data argument name)
- bias is no longer attached to conv2d
Port layout.h into relay with minimum modification

This PR represents what is generally need to be done to port an NNVM operator to relay, most of the logic remains the same

Implement the TypeRelation function
- The shapes represented by IndexExpr(symbolic integer)
- User reporter->Assign to set the inferred result
- Use reporter->AssertEQ to assert symbolic integer equivalence
  - It will return false if there is an unsatisfied constraint
Use tvm::Attrs to replace of dmlc::Parameter
We switch to directly create python wrappers by calling into positional functions so that the operator signature is explicit in python.

Examples

Although I am not completely happy with the current python DSL API(relay.ir_builder). The example test case does show some interesting improvement over what we can do previously, for example, we can already do limited symbolic inference with batch dimension as symbolic

def test_conv2d_infer_type():
    # symbolic in batch dimension
    ib = relay.ir_builder.IRBuilder()
    n, c, h, w = tvm.var("n"), 10, 224, 224
    x = ib.param("x", relay.ty.TensorType((n, c, h, w), "float32"))
    w = ib.param("w", relay.ty.IncompleteType())

    with ib.function(x, w) as func:
        ib.ret(relay.nn.conv2d(x.var, w.var,
                               kernel_size=(3, 3),
                               padding=(1, 1),
                               channels=2))
    ib.ret(func)
    func = relay.ir_pass.infer_type(ib.env, func.to_func())
    ftype = func.checked_type()
    assert ftype.ret_type == relay.ty.TensorType(
        (n, 2, 224, 224), "float32")
    assert ftype.arg_types[1] == relay.ty.TensorType(
        (10, 2, 3, 3), "float32")

    # infer by shape of w, mixed precision
    ib = relay.ir_builder.IRBuilder()
    n, c, h, w = tvm.var("n"), 10, 224, 224
    x = ib.param("x", relay.ty.TensorType((n, c, h, w), "int8"))
    w = ib.param("w", relay.ty.TensorType((2, 10, 3, 3), "int8"))
    with ib.function(x, w) as func:
        ib.ret(relay.nn.conv2d(x.var, w.var, out_dtype="int32"))
    ib.ret(func)
    func = relay.ir_pass.infer_type(ib.env, func.to_func())
    ftype = func.checked_type()
    assert ftype.ret_type == relay.ty.TensorType(
        (n, 2, 222, 222), "int32")

tqchen · 2018-10-02T23:32:11Z

cc @jroesch @zheng-da @merrymercy @yzhliu @ZihengJiang @zhiics @junrushao1994 @slyubomirsky

junrushao · 2018-10-03T00:10:14Z

Rushing for an exam. Will check it out tomorrow evening.

zhiics · 2018-10-03T02:21:17Z

src/relay/op/nn/layout.h

+   *        indicates the split dimension.
+   *        return undefined layout if "__undef__" is passed.
+   */
+  inline Layout(const std::string& layout) { // NOLINT(*)


I think there is no need to have inline. Definitions inside a class will be treated as inlined automatically. Same to the methods that implemented in this class below.

zhiics · 2018-10-03T02:37:51Z

src/relay/op/nn/layout.h

+ * \return shape in target layout
+ */
+inline std::vector<IndexExpr> ConvertLayout(
+    std::vector<IndexExpr> src,


const is not used in here as it can be used for copy-elison when directly returning it

zhiics · 2018-10-03T02:45:11Z

src/relay/op/nn/layout.h

+}
+
+inline std::vector<IndexExpr> ConvertLayout(
+    Array<IndexExpr> src,


zhiics · 2018-10-03T02:46:28Z

src/relay/pass/alpha_eq.cc

-  void ShapeEqual(Array<ShapeExpr> s1, Array<ShapeExpr> s2) {}
+  void ShapeEqual(Array<IndexExpr> s1, Array<IndexExpr> s2) {}

  void VisitType_(const TensorTypeNode *tt1, const Type& t2) final {


TensorTypeNode* tt1

MarisaKirisame · 2018-10-03T03:39:00Z

src/relay/op/nn/layout.h

+                            << ": invalid factor size " << factor
+                            << " before dimension " << c;
+        CHECK_EQ(superdim_pos_[pos], -1) << "Invalid layout " << layout
+                                           << ": duplicate dimension " << c;


remove two space?

MarisaKirisame · 2018-10-03T03:39:17Z

src/relay/op/nn/layout.h

+        int pos = c - 'a';
+        CHECK_GT(factor, 0) << "Invalid layout " << layout << ": invalid factor size "
+                            << factor << " for dimension " << c;
+        CHECK_EQ(subdim_pos_[pos], -1) << "Invalid layout " << layout


remove two space to align

srkreddy1238 · 2018-10-03T14:14:24Z

tests/python/relay/test_op_level2.py

+    ib.ret(func)
+    func = relay.ir_pass.infer_type(ib.env, func.to_func())
+    ftype = func.checked_type()
+    assert ftype.ret_type == relay.ty.TensorType(


@tqchen
I tried some hands on with this test case and observed this comparison hold good only for the dtype not for shape.

Am I missing some thing here?

Can you elaborate? The testcase should works for shape as well

assert ftype.ret_type == relay.ty.TensorType((n, 2, 224, 224), "float32")

To

assert ftype.ret_type == relay.ty.TensorType((n, 2, 100, 100), "float32")

doesn't fail.

Great catch, this is actually due to a problem in alpha_eq comparator, I have fixed this in recent commit

tqchen · 2018-10-04T01:38:48Z

@MarisaKirisame @zhiics Thanks for the comments, i have fixed them

junrushao · 2018-10-04T01:42:46Z

python/tvm/relay/op/nn.py

+        Number of groups for grouped convolution.
+
+    data_layout : str, optional
+        Layout of the input.


Should we mention the possible options for data_layout, weight_layout etc?

@junrushao1994

It can be a sample then complete list if required.
The possible options list is very long and is explained in layout header.

@srkreddy1238 My comment here is too picky. Definitely It is not a big deal.

I am just a little bit worried whether Python frontend users will be willing to check C APIs described in tvm::relay::ConvAttrs defined in include/tvm/relay/attrs/nn.h.

zhiics · 2018-10-04T02:27:14Z

@tqchen Seems some test cases failed. Please fix them.

tqchen · 2018-10-04T03:16:10Z

Thanks @srkreddy1238 's comment uncovering the problem of alpha eq, this leads to a few more problems to be uncovered. It is a good reminder for us to have good test coverage, in light of that. I have added more test cases for conv2d including the layout transformation.

@zhiics @srkreddy1238 @MarisaKirisame please review again

srkreddy1238

Layout conversion on oshape is also addressed now & LGTM.

srkreddy1238 · 2018-10-04T03:50:45Z

python/tvm/relay/op/nn.py

+        Number of groups for grouped convolution.
+
+    data_layout : str, optional
+        Layout of the input.


@junrushao1994

It can be a sample then complete list if required.
The possible options list is very long and is explained in layout header.

junrushao

The PR looks good to me.

junrushao · 2018-10-04T04:00:58Z

python/tvm/relay/op/nn.py

+        Number of groups for grouped convolution.
+
+    data_layout : str, optional
+        Layout of the input.


@srkreddy1238 My comment here is too picky. Definitely It is not a big deal.

I am just a little bit worried whether Python frontend users will be willing to check C APIs described in tvm::relay::ConvAttrs defined in include/tvm/relay/attrs/nn.h.

tqchen · 2018-10-04T04:59:02Z

Thanks @junrushao1994 @srkreddy1238 @zhiics @MarisaKirisame this is now merged. I agree with @junrushao1994 that we might want to list common options here, maybe not all of them

[RELAY][OP] conv2d, ShapeExpr->IndexExpr

68904e3

tqchen force-pushed the master branch from 7ef7093 to 68904e3 Compare October 2, 2018 23:33

tqchen mentioned this pull request Oct 3, 2018

[RELAY][OP] Relay Operator Sprint #1799

Closed

66 tasks

tqchen added 2 commits October 2, 2018 17:12

Fix attrs api

110d355

fix topi axis problem

e95402e

zhiics reviewed Oct 3, 2018

View reviewed changes

MarisaKirisame reviewed Oct 3, 2018

View reviewed changes

srkreddy1238 reviewed Oct 3, 2018

View reviewed changes

Fix review comments, fix AlphaEq for shape

6dc5a71

junrushao reviewed Oct 4, 2018

View reviewed changes

zhiics approved these changes Oct 4, 2018

View reviewed changes

Bug fix error caught by recovering alpha eq, more testcase on layout

9a6802a

srkreddy1238 approved these changes Oct 4, 2018

View reviewed changes

junrushao approved these changes Oct 4, 2018

View reviewed changes

tqchen merged commit 9afde69 into apache:master Oct 4, 2018

FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018

[RELAY][OP] conv2d, ShapeExpr->IndexExpr (apache#1798)

c5cff98

[RELAY][OP] conv2d, ShapeExpr->IndexExpr #1798

[RELAY][OP] conv2d, ShapeExpr->IndexExpr #1798

Uh oh!

Conversation

tqchen commented Oct 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Examples

Uh oh!

tqchen commented Oct 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

junrushao commented Oct 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tqchen commented Oct 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junrushao Oct 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhiics commented Oct 4, 2018

Uh oh!

tqchen commented Oct 4, 2018

Uh oh!

srkreddy1238 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

junrushao left a comment

Choose a reason for hiding this comment

Uh oh!

junrushao Oct 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tqchen commented Oct 4, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tqchen commented Oct 2, 2018 •

edited

Loading

tqchen commented Oct 2, 2018 •

edited

Loading

junrushao Oct 4, 2018 •

edited

Loading

junrushao Oct 4, 2018 •

edited

Loading