Gradient multiplier (contrib) operator #13632

ifeherva · 2018-12-13T06:41:36Z

Description

Adds the gradient multiplier operator that is mostly used in unsupervised adversarial domain adaptation.
In short: on forward pass it acts as identity transform; on backwards it multiplies the gradients with a scalar constant (lambda).
See full description here: http:https://proceedings.mlr.press/v37/ganin15.pdf

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

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)
[x 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.
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Missing test for backwards pass

roywei · 2018-12-14T17:40:04Z

@mxnet-label-bot add[Operator, pr-awaiting-review]

ThomasDelteil · 2018-12-19T19:57:46Z

Shouldn't we have a more generic gradient multiplier operator? What d you think?

ifeherva · 2018-12-19T20:04:59Z

That is certainly possible, shall I rewrite it?

ThomasDelteil · 2018-12-20T07:57:29Z

@szha @zheng-da what do you think?

Roshrini · 2019-01-03T18:12:08Z

@szha @zheng-da Can you take a look at this PR? Thanks!

src/operator/contrib/gradient_reversal_op-inl.h

src/operator/contrib/gradient_reversal_op.cc

szha

Thanks for contributing the op. The forward and backward logic can utilize existing kernels such as those in identity and broadcast_scalar_mul.

ifeherva · 2019-01-11T00:24:52Z

@szha Thanks for the feedback, good points. However, I have a hard time finding those kernels, to me they seem to be deeply integrated into other operators. Could you please point me to the right functions?

szha · 2019-01-11T04:43:52Z

Identity: https://github.com/apache/incubator-mxnet/blob/59f43956e2b045458f2c10b5197a727b58849f57/src/operator/tensor/elemwise_unary_op_basic.cc#L283-L288

szha · 2019-01-11T04:46:00Z

Scalar mul:
https://github.com/apache/incubator-mxnet/blob/59f43956e2b045458f2c10b5197a727b58849f57/src/operator/tensor/elemwise_binary_scalar_op_basic.cc#L156-L160

ifeherva · 2019-01-14T02:31:40Z

@szha Dumped the header file and used forward and backward from identity / scalar_mul.

eric-haibin-lin · 2019-01-14T08:42:53Z

src/operator/contrib/gradient_reversal_op.cc

+.set_attr_parser([](NodeAttrs* attrs) {
+ attrs->parsed = std::stod(attrs->dict["scalar"]);
+ })
+.set_attr<FInferStorageType>("FInferStorageType", ElemwiseStorageType<1, 1, false, true, true>)


Do you also plan to support sparse inputs/outputs? If not, you don't have to register FInferStorageType and FComputeEx (by default it infers dense storage and uses FCompute).

Since the operator is very simple I thought it would be easy to support sparse data as well. What do I need to change to have full support?

ifeherva · 2019-01-14T17:17:12Z

Thinking to rename the operator to gradient multiplier. Any thoughts?

edisongustavo · 2019-01-14T18:33:14Z

src/operator/contrib/gradient_reversal_op.cc

+ DispatchMode* dispatch_mode,
+ std::vector<int> *in_attrs,
+ std::vector<int> *out_attrs) {
+ CHECK_EQ(in_attrs->size(), 1);


This method has no indentation. Is this expected?

It does, not sure why github shows it wrong

Retrigger flaky test

szha · 2019-01-16T05:33:29Z

src/operator/contrib/gradient_multiplier_op.cc

+ [](const NodeAttrs& attrs){
+ return std::vector<bool>{true};
+ })
+.add_argument("scalar", "float", "scalar input");


consider making this description more informative (e.g. X multiplier)

Good point, updated.

Improved the description of the scalar multiplier

lanking520 · 2019-01-24T01:33:24Z

@szha @ThomasDelteil merge?

* Added the gradient reversal contrib operator Missing test for backwards pass * Fixed linting errors * Fixed forward test * Added random forward / backward test for gradient reversal * Update test_contrib_operator.py * Fixed typo in gradient reversal op description * Replace forward code with the identitiy implementation * Fixed typos in function docs * Changed default behavior to identity * Replaced backward code with scalar_mul * Fixed backward operator and unit test * Renamed operator to gradient multiplier * Update test_contrib_operator.py Retrigger flaky test * Update gradient_multiplier_op.cc Improved the description of the scalar multiplier

Added the gradient reversal contrib operator

735438a

Missing test for backwards pass

ifeherva requested a review from anirudh2290 as a code owner December 13, 2018 06:41

ifeherva added 3 commits December 12, 2018 23:01

Fixed linting errors

44eda55

Fixed forward test

e9bf741

Added random forward / backward test for gradient reversal

dfd1906

ifeherva changed the title ~~[WIP] Gradient reversal (contrib) operator~~ Gradient reversal (contrib) operator Dec 13, 2018

marcoabreu added Operator pr-awaiting-review PR is waiting for code review labels Dec 14, 2018

Update test_contrib_operator.py

5c73533

ChaiBapchya reviewed Jan 10, 2019

View reviewed changes

src/operator/contrib/gradient_reversal_op-inl.h Outdated Show resolved Hide resolved

src/operator/contrib/gradient_reversal_op-inl.h Outdated Show resolved Hide resolved

src/operator/contrib/gradient_reversal_op.cc Outdated Show resolved Hide resolved

szha reviewed Jan 10, 2019

View reviewed changes

Fixed typo in gradient reversal op description

0bc7986

ifeherva added 5 commits January 12, 2019 16:25

Replace forward code with the identitiy implementation

ad72f41

Fixed typos in function docs

912f2a0

Changed default behavior to identity

f865e14

Replaced backward code with scalar_mul

0cd8416

Fixed backward operator and unit test

19194b0

eric-haibin-lin reviewed Jan 14, 2019

View reviewed changes

edisongustavo reviewed Jan 14, 2019

View reviewed changes

Renamed operator to gradient multiplier

d1fffac

ifeherva changed the title ~~Gradient reversal (contrib) operator~~ Gradient multiplier (contrib) operator Jan 14, 2019

Update test_contrib_operator.py

54ae4f0

Retrigger flaky test

ThomasDelteil approved these changes Jan 15, 2019

View reviewed changes

szha reviewed Jan 16, 2019

View reviewed changes

szha approved these changes Jan 16, 2019

View reviewed changes

Update gradient_multiplier_op.cc

3983458

Improved the description of the scalar multiplier

ThomasDelteil merged commit 183be8c into apache:master Jan 24, 2019

ifeherva deleted the gradient_reversal_operator branch February 10, 2019 04:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gradient multiplier (contrib) operator #13632

Gradient multiplier (contrib) operator #13632

ifeherva commented Dec 13, 2018 •

edited

Loading

roywei commented Dec 14, 2018

ThomasDelteil commented Dec 19, 2018

ifeherva commented Dec 19, 2018

ThomasDelteil commented Dec 20, 2018

Roshrini commented Jan 3, 2019

szha left a comment

ifeherva commented Jan 11, 2019

szha commented Jan 11, 2019

szha commented Jan 11, 2019

ifeherva commented Jan 14, 2019

eric-haibin-lin Jan 14, 2019

ifeherva Jan 14, 2019

ifeherva commented Jan 14, 2019

edisongustavo Jan 14, 2019

ifeherva Jan 14, 2019

szha Jan 16, 2019

ifeherva Jan 17, 2019

lanking520 commented Jan 24, 2019

Gradient multiplier (contrib) operator #13632

Gradient multiplier (contrib) operator #13632

Conversation

ifeherva commented Dec 13, 2018 • edited Loading

Description

Checklist

Essentials

roywei commented Dec 14, 2018

ThomasDelteil commented Dec 19, 2018

ifeherva commented Dec 19, 2018

ThomasDelteil commented Dec 20, 2018

Roshrini commented Jan 3, 2019

szha left a comment

Choose a reason for hiding this comment

ifeherva commented Jan 11, 2019

szha commented Jan 11, 2019

szha commented Jan 11, 2019

ifeherva commented Jan 14, 2019

eric-haibin-lin Jan 14, 2019

Choose a reason for hiding this comment

ifeherva Jan 14, 2019

Choose a reason for hiding this comment

ifeherva commented Jan 14, 2019

edisongustavo Jan 14, 2019

Choose a reason for hiding this comment

ifeherva Jan 14, 2019

Choose a reason for hiding this comment

szha Jan 16, 2019

Choose a reason for hiding this comment

ifeherva Jan 17, 2019

Choose a reason for hiding this comment

lanking520 commented Jan 24, 2019

ifeherva commented Dec 13, 2018 •

edited

Loading