refactor regression ops to nnvm interface by ZiyueHuang · Pull Request #9540 · apache/mxnet

ZiyueHuang · 2018-01-24T08:02:06Z

Description

refactor regression operators to nnvm interface (then I will add sparse support for these operators in a separate PR later).

Fix the docs. Please see #9528.

Add unittest for MAERegressionOutput.

cc @eric-haibin-lin @sxjscience

Checklist

Essentials

Passed code style checking (make lint)
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
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

refactor, existing unit tests for LinearRegressionOutput and LogisticRegressionOutput. Add unittest for MAERegressionOutput.

Comments

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

sxjscience · 2018-01-24T18:09:03Z

+  // inputs are in_label and out_data, outputs are data_grad and label_grad
+  return std::vector<std::pair<int, int> >{{1, 0}};
+})
+.set_attr<FCompute>("FCompute<cpu>", RegressionBackward<cpu, mshadow_op::minus>);


We can consider creating a macro (like in https://github.com/apache/incubator-mxnet/blob/master/src/operator/tensor/elemwise_binary_op.h#L554-L569) for defining regression OPs as most of the codes are similar.

eric-haibin-lin · 2018-01-24T21:07:51Z

+  mshadow::Stream<xpu> *s = ctx.get_stream<xpu>();
+  MSHADOW_REAL_TYPE_SWITCH(inputs[reg_enum::kData].type_flag_, DType, {
+    MXNET_ASSIGN_REQ_SWITCH(req[reg_enum::kOut], Req, {
+      DType* in_data = inputs[reg_enum::kData].dptr<DType>();


eric-haibin-lin · 2018-01-24T21:07:59Z

+  // outputs are data_grad, label_grad
+  MSHADOW_REAL_TYPE_SWITCH(inputs[1].type_flag_, DType, {
+    MXNET_ASSIGN_REQ_SWITCH(req[0], Req, {
+      DType* in_label = inputs[0].dptr<DType>();


eric-haibin-lin · 2018-01-24T21:10:57Z


-By default, gradients of this loss function are scaled by factor `1/n`, where n is the number of training examples.
-The parameter `grad_scale` can be used to change this scale to `grad_scale/n`.
+By default, gradients of this loss function are scaled by factor `1/m`, where m is the number of dimensions of a training example.


number of dimensions is a bit confusing to me. For shape (3,4), my first thought of num dimension is two (2-D shape). Is "number of features" clearer?

eric-haibin-lin · 2018-01-24T21:12:18Z

+  // inputs are in_label and out_data, outputs are data_grad and label_grad
+  return std::vector<std::pair<int, int> >{{1, 0}};
+})
+.set_attr<FCompute>("FCompute<cpu>", RegressionBackward<cpu, mshadow_op::minus>);


ZiyueHuang · 2018-01-25T12:53:12Z

Thanks for the comments. Updated.

sxjscience · 2018-01-25T17:30:47Z


-By default, gradients of this loss function are scaled by factor `1/n`, where n is the number of training examples.
-The parameter `grad_scale` can be used to change this scale to `grad_scale/n`.
+By default, gradients of this loss function are scaled by factor `1/m`, where m is the number of features of a training example.


I think the word features is not very appropriate. We can use regression outputs

sxjscience · 2018-01-25T17:31:54Z

Added some minor docstring comments. The PR looks good.

eric-haibin-lin

"Add unittest for MAERegressionOutput."
Where is the test?

ZiyueHuang · 2018-01-27T03:03:28Z

Seems that I mistakenly checkout that test file... Added back.

* refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test

* Bump 1.1 (#192) * bump * also update base.h * revert website changes * Update index.html * update news.md (#191) * Update NEWS.md * Update README.md * refactor regression ops to nnvm interface (#9540) * refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test * Update KEYS * Update NEWS.md * fixed links that were missng ndarray folder path (#9618) * Fixed 4 broken links (#9698) * Fixed 4 broken links * fixed pylint for long line disable and 1 broken link * Update NEWS.md * Update NOTICE (#9706) * revert acc changes (#9731) * Revert "avoid per-batch blocking in metric (#9636)" This reverts commit 3fe694e. * Revert "proper flatten in acc (#9619)" This reverts commit ed823b2. * Revert "use nd for accuracy calculation (#9583)" This reverts commit f5f1b91. * keep doc change * PGP keys add liuyizhi AT apache.org (#9728) * Add my key (#9736) * [REVIEW REQUIRED] Revert PR #9484 & add additional dependency licenses to LICENSE file (#9701) * Revert "[Review Required] Fixing Licenses: Cleaning up the Top Level LICENSE file (#9484)" This reverts commit 8930d96. * Some more LICENSE fixes * Adding some more packages to the LICENSE file * Adding dependencies of dependencies * update navbar model zoo link (#9749) * update navbar model zoo link * update * initial commit * clean up * refactor * fix test

* Bump 1.1 (apache#192) * bump * also update base.h * revert website changes * Update index.html * update news.md (apache#191) * Update NEWS.md * Update README.md * refactor regression ops to nnvm interface (apache#9540) * refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test * Update KEYS * Update NEWS.md * fixed links that were missng ndarray folder path (apache#9618) * Fixed 4 broken links (apache#9698) * Fixed 4 broken links * fixed pylint for long line disable and 1 broken link * Update NEWS.md * Update NOTICE (apache#9706) * revert acc changes (apache#9731) * Revert "avoid per-batch blocking in metric (apache#9636)" This reverts commit 3fe694e. * Revert "proper flatten in acc (apache#9619)" This reverts commit ed823b2. * Revert "use nd for accuracy calculation (apache#9583)" This reverts commit f5f1b91. * keep doc change * PGP keys add liuyizhi AT apache.org (apache#9728) * Add my key (apache#9736) * [REVIEW REQUIRED] Revert PR apache#9484 & add additional dependency licenses to LICENSE file (apache#9701) * Revert "[Review Required] Fixing Licenses: Cleaning up the Top Level LICENSE file (apache#9484)" This reverts commit 8930d96. * Some more LICENSE fixes * Adding some more packages to the LICENSE file * Adding dependencies of dependencies * update navbar model zoo link (apache#9749) * update navbar model zoo link * update * initial commit * clean up * refactor * fix test

* refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test

* Bump 1.1 (apache#192) * bump * also update base.h * revert website changes * Update index.html * update news.md (apache#191) * Update NEWS.md * Update README.md * refactor regression ops to nnvm interface (apache#9540) * refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test * Update KEYS * Update NEWS.md * fixed links that were missng ndarray folder path (apache#9618) * Fixed 4 broken links (apache#9698) * Fixed 4 broken links * fixed pylint for long line disable and 1 broken link * Update NEWS.md * Update NOTICE (apache#9706) * revert acc changes (apache#9731) * Revert "avoid per-batch blocking in metric (apache#9636)" This reverts commit 3fe694e. * Revert "proper flatten in acc (apache#9619)" This reverts commit ed823b2. * Revert "use nd for accuracy calculation (apache#9583)" This reverts commit f5f1b91. * keep doc change * PGP keys add liuyizhi AT apache.org (apache#9728) * Add my key (apache#9736) * [REVIEW REQUIRED] Revert PR apache#9484 & add additional dependency licenses to LICENSE file (apache#9701) * Revert "[Review Required] Fixing Licenses: Cleaning up the Top Level LICENSE file (apache#9484)" This reverts commit 8930d96. * Some more LICENSE fixes * Adding some more packages to the LICENSE file * Adding dependencies of dependencies * update navbar model zoo link (apache#9749) * update navbar model zoo link * update * initial commit * clean up * refactor * fix test

* refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test

* Bump 1.1 (apache#192) * bump * also update base.h * revert website changes * Update index.html * update news.md (apache#191) * Update NEWS.md * Update README.md * refactor regression ops to nnvm interface (apache#9540) * refactor regression ops * fix err for instantiation of minus_sign * remove useless header file init_op.h * replace with macro and address other comments * update * minor revise docs * add mae test * Update KEYS * Update NEWS.md * fixed links that were missng ndarray folder path (apache#9618) * Fixed 4 broken links (apache#9698) * Fixed 4 broken links * fixed pylint for long line disable and 1 broken link * Update NEWS.md * Update NOTICE (apache#9706) * revert acc changes (apache#9731) * Revert "avoid per-batch blocking in metric (apache#9636)" This reverts commit 3fe694e. * Revert "proper flatten in acc (apache#9619)" This reverts commit ed823b2. * Revert "use nd for accuracy calculation (apache#9583)" This reverts commit f5f1b91. * keep doc change * PGP keys add liuyizhi AT apache.org (apache#9728) * Add my key (apache#9736) * [REVIEW REQUIRED] Revert PR apache#9484 & add additional dependency licenses to LICENSE file (apache#9701) * Revert "[Review Required] Fixing Licenses: Cleaning up the Top Level LICENSE file (apache#9484)" This reverts commit 8930d96. * Some more LICENSE fixes * Adding some more packages to the LICENSE file * Adding dependencies of dependencies * update navbar model zoo link (apache#9749) * update navbar model zoo link * update * initial commit * clean up * refactor * fix test

ZiyueHuang added 3 commits January 23, 2018 16:14

refactor regression ops

68ed790

fix err for instantiation of minus_sign

4815e85

remove useless header file init_op.h

0312f87

sxjscience reviewed Jan 24, 2018

View reviewed changes

eric-haibin-lin reviewed Jan 24, 2018

View reviewed changes

ZiyueHuang added 2 commits January 25, 2018 07:16

replace with macro and address other comments

3cd1e0d

update

08e4b0d

sxjscience reviewed Jan 25, 2018

View reviewed changes

minor revise docs

81dd5cc

eric-haibin-lin reviewed Jan 26, 2018

View reviewed changes

add mae test

4cd096e

eric-haibin-lin merged commit c6d97d6 into apache:master Jan 27, 2018

ZiyueHuang deleted the sparse_reg branch January 30, 2018 06:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor regression ops to nnvm interface#9540

refactor regression ops to nnvm interface#9540
eric-haibin-lin merged 7 commits intoapache:masterfrom
ZiyueHuang:sparse_reg

ZiyueHuang commented Jan 24, 2018 •

edited

Loading

Uh oh!

sxjscience Jan 24, 2018

Uh oh!

eric-haibin-lin Jan 24, 2018

Uh oh!

eric-haibin-lin Jan 24, 2018

Uh oh!

eric-haibin-lin Jan 24, 2018

Uh oh!

eric-haibin-lin Jan 24, 2018

Uh oh!

eric-haibin-lin Jan 24, 2018

Uh oh!

ZiyueHuang commented Jan 25, 2018

Uh oh!

sxjscience Jan 25, 2018

Uh oh!

ZiyueHuang Jan 26, 2018

Uh oh!

sxjscience commented Jan 25, 2018

Uh oh!

eric-haibin-lin left a comment

Uh oh!

ZiyueHuang commented Jan 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ZiyueHuang commented Jan 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Essentials

Changes

Comments

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!

ZiyueHuang commented Jan 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sxjscience commented Jan 25, 2018

Uh oh!

eric-haibin-lin left a comment

Choose a reason for hiding this comment

Uh oh!

ZiyueHuang commented Jan 27, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZiyueHuang commented Jan 24, 2018 •

edited

Loading