[MXNET-978] n-th order gradient test support. #15611

kshitij12345 · 2019-07-19T18:34:50Z

Description

n-th order gradient test support function.

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

Support test for n-th order.

kshitij12345 · 2019-07-19T18:37:20Z

@apeforest @larroy

While working on a PR (I don't exactly remember which one), test for incorrect implementation of second order gradient was passing due to the very small values. However as sanity check, when I checked for its third order gradient, the assertion failed allowing to catch the issue.

So I think it would be better to check for the order being implemented and one more (computed by autograd) as a sanity check.

Would like to know your thoughts about the same.

Thank You.

larroy

Nice approach.

larroy · 2019-07-22T20:42:47Z

tests/python/unittest/test_higher_order_grad.py

+            head_grad = nd.random.normal(shape=x.shape)
+            y = autograd.grad(heads=y, variables=x, head_grads=head_grad,
+                              create_graph=True, retain_graph=True)[0]
+            if current_order in orders:


If current_order is not in orders we might have problem zipping? Is there a case where you wou want 1st and 3rd order but not second?

I don't think there would be an issue in that case (will confirm it later though).
If current_order is not in orders case is checked here,
https://github.com/apache/incubator-mxnet/blob/1f74614391e182e299e2fdcce1036515c4e5fb4f/tests/python/unittest/test_higher_order_grad.py#L41-L42
where first order is not asserted (as per the arguments).

The main thing is that elements in orders should be monotonically increasing and they should correspond to elements of the grad_ops.
We can also use a dictionary {order: corresponding grad op, ... } which removes the above requirement.

Have confirmed, that

check_nth_order_unary(array, sin, [grad_op, grad_grad_grad_op], [1, 3])

following works.

larroy · 2019-07-23T05:07:01Z

tests/python/unittest/test_higher_order_grad.py

-    assert_almost_equal(expected_grad_grad, x.grad.asnumpy())
+        for current_order in range(1, order+1):
+            head_grad = nd.random.normal(shape=x.shape)
+            y = autograd.grad(heads=y, variables=x, head_grads=head_grad,


better name instead of mutating y?

One option I can think of is, at the start of for loop we'll have computed_grad = y (which is deceiving) and replace y by computed_grad in the for loop.
Up for suggestions.

larroy · 2019-07-23T05:07:52Z

tests/python/unittest/test_higher_order_grad.py

+    check_nth_order_unary(x, op, grad_grad_op, 2)
+
+
+def check_nth_order_unary(x, op, grad_ops, orders):


add docstring?

Sure thing.

I have added the docstring. Could you please take a look to see if it needs more polishing.
Thank You.

larroy

LGTM

…to develop/higher-order-grad/test-support

apeforest

LGTM!

apeforest · 2019-09-14T07:33:56Z

tests/python/unittest/test_higher_order_grad.py

-    # Manual head_grads.
-    y_grad = nd.random.normal(shape=x.shape)
-    head_grad_grads = nd.random.normal(shape=x.shape)
+    order = max(orders)


If orders is monotonically increasing, should this just be orders[-1]?

Yes. That will work as well. But I felt max(orders) stated the intent more clearly.

apeforest · 2019-09-14T07:37:33Z

Ping @sxjscience for review.

kshitij12345 · 2019-09-28T08:28:59Z

@sxjscience Gentle ping for review:)

* n-th order grad test support * use check_nth_order_unary for second order check * add docstring to check_nth_order_unary * retrigger CI * add assertions for requirements of orders * fix assert condition * retrigger CI

n-th order grad test support

1b5d96a

use check_nth_order_unary for second order check

1f74614

marcoabreu added the pr-work-in-progress PR is still work in progress label Jul 19, 2019

larroy reviewed Jul 22, 2019

View reviewed changes

larroy reviewed Jul 23, 2019

View reviewed changes

add docstring to check_nth_order_unary

4e5c4cc

larroy approved these changes Jul 23, 2019

View reviewed changes

kshitij12345 changed the title ~~[MXNET-978] n-th order gradient test support~~ [MXNET-978] n-th order gradient test support. Jul 26, 2019

kshitij12345 added 3 commits August 21, 2019 22:24

Merge branch 'master' of https://github.com/apache/incubator-mxnet in…

5523d4a

…to develop/higher-order-grad/test-support

Merge latest 'master' into develop/higher-order-grad/test-support

daa77f1

retrigger CI

f7cd885

apeforest approved these changes Sep 14, 2019

View reviewed changes

apeforest reviewed Sep 14, 2019

View reviewed changes

apeforest added pr-awaiting-review PR is waiting for code review and removed pr-work-in-progress PR is still work in progress labels Sep 14, 2019

kshitij12345 added 3 commits September 14, 2019 20:30

add assertions for requirements of orders

dccc2e8

fix assert condition

c1c14d2

retrigger CI

82174a4

apeforest merged commit 3950a47 into apache:master Sep 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MXNET-978] n-th order gradient test support. #15611

[MXNET-978] n-th order gradient test support. #15611

kshitij12345 commented Jul 19, 2019 •

edited

Loading

kshitij12345 commented Jul 19, 2019 •

edited

Loading

larroy left a comment

larroy Jul 22, 2019

kshitij12345 Jul 23, 2019 •

edited

Loading

kshitij12345 Jul 23, 2019

larroy Jul 23, 2019

kshitij12345 Jul 23, 2019

larroy Jul 23, 2019

kshitij12345 Jul 23, 2019

kshitij12345 Jul 23, 2019

larroy left a comment

apeforest left a comment

apeforest Sep 14, 2019

kshitij12345 Sep 14, 2019

apeforest commented Sep 14, 2019

kshitij12345 commented Sep 28, 2019

		check_nth_order_unary(x, op, grad_grad_op, 2)


		def check_nth_order_unary(x, op, grad_ops, orders):

[MXNET-978] n-th order gradient test support. #15611

[MXNET-978] n-th order gradient test support. #15611

Conversation

kshitij12345 commented Jul 19, 2019 • edited Loading

Description

Checklist

Essentials

Changes

kshitij12345 commented Jul 19, 2019 • edited Loading

larroy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshitij12345 Jul 23, 2019 • edited Loading

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

larroy left a comment

Choose a reason for hiding this comment

apeforest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apeforest commented Sep 14, 2019

kshitij12345 commented Sep 28, 2019

kshitij12345 commented Jul 19, 2019 •

edited

Loading

kshitij12345 commented Jul 19, 2019 •

edited

Loading

kshitij12345 Jul 23, 2019 •

edited

Loading