[MXNET-1227] Adding CornerPooling operator #13401

BigDeviltjj · 2018-11-25T10:33:09Z

Description

Add operator:

CornerPooling operator

link to JIRA issue 1227

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

CornerPooling
docs
unittest

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

stu1130 · 2018-11-25T22:32:03Z

@mxnet-label-bot add [pr-awaiting-review]
Thanks for doing this @BigDeviltjj

Roshrini · 2018-12-18T00:04:47Z

@apeforest @eric-haibin-lin Can you please review this PR?

anirudhacharya · 2018-12-18T00:18:10Z

tests/python/unittest/test_operator.py

@@ -7231,6 +7231,18 @@ def f_sm_ce(data, label):
    np_one_hot_label[np.arange(batch_size), np_label] = 1.
    check_symbolic_forward(sym, {'data' : np_data, 'label' : np_label}, [np.array([f_sm_ce(np_sm, np_one_hot_label)])], rtol=1e-3, atol=1e-5)

+@with_seed()
+def test_corner_pooling():
+    data = mx.sym.Variable(name='data')


also call check_symbolic_forward and check_symbolic_backward - https://mxnet.incubator.apache.org/faq/add_op_in_backend.html#unit-test

No problem, thank you!

I have added these two functions, check_symbolic_forward and check_symbolic_backward .

anirudh2290

Thanks for the contribution! Don't forget to add your name to CONTRIBUTORS.md

anirudh2290 · 2018-12-21T02:15:35Z

src/operator/contrib/corner_pooling-inl.h

+  CHECK_EQ(req_type, kWriteTo)
+    << "Only support req=kWriteTo in corner pooling operations";
+  using mshadow::red::limits::MinValue;
+// const TShape &oshape = ishape;


remove unused code

anirudh2290 · 2018-12-21T02:17:27Z

src/operator/contrib/corner_pooling-inl.h

+      h_end = height;
+    }
+    const index_t data_offset = width * height;
+    for (index_t b{0}; b < ishape[0]; ++b)


can you make it consistent with other mxnet code: for (index_t b = 0; b < ishape[0]; ++b) here and at other places.

anirudh2290 · 2018-12-21T02:26:26Z

src/operator/contrib/corner_pooling-inl.h

+    const index_t data_offset = width * height;
+    for (index_t b{0}; b < ishape[0]; ++b)
+      for (index_t c{0}; c < ishape[1]; ++c) {
+        for (index_t w{0}; w < width; ++w) {


can you add TODOs for optimization with Kernel::Launch

anirudh2290 · 2018-12-21T02:32:02Z

src/operator/contrib/corner_pooling.cc

+static bool CornerPoolingType(const nnvm::NodeAttrs &attrs,
+                              std::vector<int> *in_attrs,
+                              std::vector<int> *out_attrs) {
+  out_attrs->at(0) = in_attrs->at(0);


use TYPE_ASSIGN_CHECK

I have modified all the code you mentioned above. Please check my code, Thank you so much and Happy Christmas Eve!

Roshrini · 2019-01-02T22:02:52Z

@anirudh2290 @anirudhacharya Can you take a look if your comments are addressed? Thanks!

anirudhacharya · 2019-01-14T22:57:12Z

My comments are addressed.

@BigDeviltjj can you please rebase push.

BigDeviltjj · 2019-01-16T05:02:38Z

@anirudhacharya I have rebased my push.

sandeep-krishnamurthy · 2019-01-28T18:11:25Z

@anirudhacharya - Can you please take a look back at this PR?

anirudhacharya · 2019-01-29T08:43:12Z

@sandeep-krishnamurthy As mentioned above my comments have already been addressed. If @anirudh2290's comments are addressed, this PR can be merged.

vandanavk · 2019-02-05T19:15:39Z

@BigDeviltjj could you resolve the branch conflict?

@anirudh2290 for review/merge

anirudhacharya · 2019-03-02T21:10:59Z

@anirudh2290 I see that your comments are addressed, can you please take another look at this PR.

anirudh2290 · 2019-03-05T00:40:03Z

src/operator/contrib/corner_pooling.cu

+    mshadow::cuda::CheckLaunchParam(dimGrid,
+            dimBlock, "Corner Pooling Forward");
+    cudaStream_t stream = mshadow::Stream<gpu>::GetStream(s);
+    CornerPoolingForwardTBKernel<DType><<<  dimGrid, dimBlock, 0, stream>>>


can we add launch bound guards similar to #13188

karan6181 · 2019-03-19T00:32:31Z

@BigDeviltjj Can you please address the @anirudh2290 comments?
Also, please resolve the branch conflicts. Thanks!

abhinavs95 · 2019-03-28T22:39:51Z

@BigDeviltjj ping again! Could you please address the new comments and rebase?

@mxnet-label-bot update [pr-awaiting-response]

piyushghai · 2019-04-09T00:42:43Z

@BigDeviltjj Gentle ping...

Roshrini · 2019-04-17T15:41:00Z

@BigDeviltjj Can you address few comments and resolve conflicts so that we can move forward with this PR?

roywei · 2019-04-30T17:34:42Z

@BigDeviltjj gentle ping to resolve the conflicts. thanks!

vandanavk · 2019-05-10T05:45:33Z

@mxnet-label-bot add [Operator]

karan6181 · 2019-05-21T20:48:08Z

@BigDeviltjj Could you please provide an update on this PR? It's being more than 3 months and the PR is idle. If you are unable to work on this PR, you may want to consider closing it, and resubmitting once you have the bandwidth to work on it. Thank You!

BigDeviltjj requested review from anirudh2290 and szha as code owners November 25, 2018 10:33

marcoabreu added the pr-awaiting-review PR is waiting for code review label Nov 25, 2018

BigDeviltjj force-pushed the corner_pooling branch from e44fd5f to 76c1e1c Compare November 26, 2018 06:59

anirudhacharya reviewed Dec 18, 2018

View reviewed changes

anirudh2290 reviewed Dec 21, 2018

View reviewed changes

jiajie.tang and others added 13 commits January 15, 2019 13:17

add corner pooling

6cb1502

update

6035b40

update

5ee63a4

update

3519bcc

update

6262fc6

update

bfc64e9

update

1f8ee47

update

87502d9

update

1d53223

update

7a9fbdc

add_unit_test

6615505

check_crash

5a33e05

update

fda9a2a

BigDeviltjj force-pushed the corner_pooling branch from bc377b6 to fda9a2a Compare January 15, 2019 05:31

update

db22bc9

Merge branch 'master' into corner_pooling

25dccdc

anirudh2290 reviewed Mar 5, 2019

View reviewed changes

marcoabreu added pr-awaiting-response PR is reviewed and waiting for contributor to respond and removed pr-awaiting-review PR is waiting for code review labels Mar 28, 2019

marcoabreu added the Operator label May 10, 2019

BigDeviltjj closed this May 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-1227] Adding CornerPooling operator #13401

[MXNET-1227] Adding CornerPooling operator #13401

BigDeviltjj commented Nov 25, 2018 •

edited

Loading

stu1130 commented Nov 25, 2018

Roshrini commented Dec 18, 2018

anirudhacharya Dec 18, 2018

BigDeviltjj Dec 18, 2018

BigDeviltjj Dec 18, 2018

anirudh2290 left a comment

anirudh2290 Dec 21, 2018

anirudh2290 Dec 21, 2018

anirudh2290 Dec 21, 2018

anirudh2290 Dec 21, 2018

BigDeviltjj Dec 24, 2018

Roshrini commented Jan 2, 2019

anirudhacharya commented Jan 14, 2019

BigDeviltjj commented Jan 16, 2019

sandeep-krishnamurthy commented Jan 28, 2019

anirudhacharya commented Jan 29, 2019

vandanavk commented Feb 5, 2019

anirudhacharya commented Mar 2, 2019

anirudh2290 Mar 5, 2019

karan6181 commented Mar 19, 2019

abhinavs95 commented Mar 28, 2019

piyushghai commented Apr 9, 2019

Roshrini commented Apr 17, 2019

roywei commented Apr 30, 2019

vandanavk commented May 10, 2019

karan6181 commented May 21, 2019

[MXNET-1227] Adding CornerPooling operator #13401

[MXNET-1227] Adding CornerPooling operator #13401

Conversation

BigDeviltjj commented Nov 25, 2018 • edited Loading

Description

Checklist

Essentials

Changes

Comments

stu1130 commented Nov 25, 2018

Roshrini commented Dec 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anirudh2290 left a 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

Roshrini commented Jan 2, 2019

anirudhacharya commented Jan 14, 2019

BigDeviltjj commented Jan 16, 2019

sandeep-krishnamurthy commented Jan 28, 2019

anirudhacharya commented Jan 29, 2019

vandanavk commented Feb 5, 2019

anirudhacharya commented Mar 2, 2019

Choose a reason for hiding this comment

karan6181 commented Mar 19, 2019

abhinavs95 commented Mar 28, 2019

piyushghai commented Apr 9, 2019

Roshrini commented Apr 17, 2019

roywei commented Apr 30, 2019

vandanavk commented May 10, 2019

karan6181 commented May 21, 2019

BigDeviltjj commented Nov 25, 2018 •

edited

Loading