[RELAY]Ops Dense, leaky_relu #1828

siju-samuel · 2018-10-05T10:13:40Z

Dense
Leaky Relu

Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from others in the community.

siju-samuel · 2018-10-05T16:43:50Z

@srkreddy1238 @yuruofeifei @tqchen please review.

yuruofeifei · 2018-10-05T19:55:29Z

src/relay/op/tensor/nn.cc

+    runtime::detail::unpack_call<Expr, 3>(MakeDense, args, rv);
+  });
+
+RELAY_REGISTER_OP("nn.dense")


maybe add bias support?

@tqchen how about bias add support? needed? bias is optional for dense. any example for optional arg processing?

I think like conv where we kept bias away, we could do same here.

python/tvm/relay/op/nn/nn.py

tqchen · 2018-10-06T04:04:26Z

python/tvm/relay/op/nn/nn.py

+    weight : relay.Expr
+        The weight expressions.
+
+    units : int


make units optional, and allow infer from weights just as channels in conv2d

tqchen · 2018-10-06T04:05:55Z

please rebase against the most recent master

siju-samuel · 2018-10-06T10:10:08Z

src/relay/op/tensor/nn.cc

+  Array<tvm::Expr> wshape = weight->shape;
+
+  if (param->units != 0) {
+    // CHECK_EQ(param->units == wshape[wshape.size() - 1])


i want to do this comparison (Integer and Expr).. @srkreddy1238 @tqchen How can i do this comparison?

do CHECK(reporter->AssertEQ(param->units, wshape[wshape.size()-1]))

tqchen · 2018-10-08T04:18:50Z

include/tvm/relay/attrs/nn.h

@@ -173,6 +173,26 @@ struct UpSamplingAttrs : public tvm::AttrsNode<UpSamplingAttrs> {
 };


+/*! \brief Attributes for dense operator */
+struct DenseAttrs : public tvm::AttrsNode<DenseAttrs> {
+  int units;


make units optional(None) instead of 0 as None so use IndexExpr here, see conv2d as an example

tqchen · 2018-10-09T05:41:31Z

src/relay/op/nn/nn.cc

+  CHECK(static_cast<int>(data->shape.size()) != 0);
+  Array<tvm::Expr> wshape = weight->shape;
+
+  if (param->units.defined()) {


if units is defined, we don't need weight to be present, instead we can directly assign to the weight, see the implementation of conv2d

Also, need to assign to the weight here, this will infer the shape of the weight if necessary. Please also add a testcase similar to https://github.com/dmlc/tvm/blob/master/tests/python/relay/test_op_level2.py#L14 where weight's shape is not specified but get inferred

tqchen · 2018-10-09T16:13:28Z

src/relay/op/nn/nn.cc

+  CHECK(static_cast<int>(data->shape.size()) != 0);
+  Array<tvm::Expr> wshape = weight->shape;
+
+  if (param->units.defined()) {


Also, need to assign to the weight here, this will infer the shape of the weight if necessary. Please also add a testcase similar to https://github.com/dmlc/tvm/blob/master/tests/python/relay/test_op_level2.py#L14 where weight's shape is not specified but get inferred

tqchen · 2018-10-11T00:16:09Z

src/relay/op/nn/nn.cc

+
+    // validate the weight shape is proper if defined
+    if (weight != nullptr) {
+      CHECK(reporter->AssertEQ(weight->shape[0], dshape[dshape.size() - 1]))


weight != nullptr but units can still be not defined, we could dispatch by weight units is defined, the require weight to be not null when units is not defined

tqchen · 2018-10-12T05:06:06Z

src/relay/op/nn/nn.cc

+    if (weight != nullptr) {
+      CHECK(reporter->AssertEQ(weight->shape[0], dshape[dshape.size() - 1]))
+        << "Dense: shape of weight is inconsistent with input data.";
+      CHECK(reporter->AssertEQ(weight->shape[1], param->units))


Don't need to check weight !=nullptr, The Assign will run the check internally. Directly goto the assign logic is fine

tqchen · 2018-10-12T05:06:32Z

src/relay/op/nn/nn.cc

+        << "Dense: shape of weight is inconsistent with units.";
+    } else {
+      // Assign weight type
+      std::vector<IndexExpr> wshape({dshape[dshape.size() - 1], param->units});


Create an Array since you directly send it to make

tqchen · 2018-10-17T01:27:55Z

Thanks @siju-samuel @srkreddy1238 @yuruofeifei this is now merged

yuruofeifei reviewed Oct 5, 2018

View reviewed changes

tqchen requested changes Oct 6, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch 2 times, most recently from 418559f to 53dd84a Compare October 6, 2018 09:51

siju-samuel commented Oct 6, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch 2 times, most recently from b9c294e to 0ce5eef Compare October 7, 2018 06:43

tqchen requested changes Oct 8, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch 2 times, most recently from 29a0633 to c8da946 Compare October 8, 2018 05:00

yzhliu added the status: need update need update based on feedbacks label Oct 9, 2018

siju-samuel force-pushed the relay_ops_1 branch from c8da946 to 108f67f Compare October 9, 2018 04:52

tqchen requested changes Oct 9, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch 2 times, most recently from b0c0c23 to f437e2b Compare October 9, 2018 07:47

tqchen requested changes Oct 9, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch from f437e2b to 7a570b2 Compare October 10, 2018 04:08

tqchen requested changes Oct 11, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch 3 times, most recently from d82dd93 to 0793ace Compare October 12, 2018 01:41

tqchen requested changes Oct 12, 2018

View reviewed changes

siju-samuel force-pushed the relay_ops_1 branch from 0793ace to 5eee8f7 Compare October 12, 2018 05:30

siju-samuel added 5 commits October 15, 2018 10:45

[RELAY]Ops Dense, leaky_relu

8a4856d

Rebasing and fixed review comments

6f44ab2

Check added to validate units and weight shape

949552c

moved to nn.cc

af28349

Review comments updated

658fa66

siju-samuel added 3 commits October 15, 2018 10:46

Review comments fixed and rebased

bc1e6b5

weight inference

b92d438

Review comment fix

431a57d

siju-samuel force-pushed the relay_ops_1 branch from bcc2f20 to 431a57d Compare October 15, 2018 05:18

tqchen and others added 2 commits October 15, 2018 17:09

Merge branch 'master' into relay_ops_1

c972613

Merge branch 'master' into relay_ops_1

d8b8df4

tqchen approved these changes Oct 17, 2018

View reviewed changes

tqchen merged commit d5d1944 into apache:master Oct 17, 2018

tqchen added status: accepted and removed status: need update need update based on feedbacks labels Oct 17, 2018

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

[RELAY]Ops Dense, leaky_relu (apache#1828)

d10db1f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RELAY]Ops Dense, leaky_relu #1828

[RELAY]Ops Dense, leaky_relu #1828

siju-samuel commented Oct 5, 2018

siju-samuel commented Oct 5, 2018

yuruofeifei Oct 5, 2018

siju-samuel Oct 6, 2018

srkreddy1238 Oct 6, 2018

tqchen Oct 6, 2018

tqchen commented Oct 6, 2018

siju-samuel Oct 6, 2018

tqchen Oct 6, 2018

tqchen Oct 8, 2018

tqchen Oct 9, 2018

tqchen Oct 9, 2018

tqchen Oct 9, 2018

tqchen Oct 11, 2018

tqchen Oct 12, 2018

tqchen Oct 12, 2018

tqchen commented Oct 17, 2018

[RELAY]Ops Dense, leaky_relu #1828

[RELAY]Ops Dense, leaky_relu #1828

Conversation

siju-samuel commented Oct 5, 2018

siju-samuel commented Oct 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen commented Oct 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tqchen commented Oct 17, 2018