[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad
log
URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r295504687
##
File path: src/operator/tensor/elemwise_unary_op_basic.cc
##
@@ -1090,68 +1090,84 @@
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU_DR(_backward_log,
unary_bwd)
.set_attr("FGradient",
[](const nnvm::NodePtr& n, const std::vector& ograds) {
-// For f(x) -> f = log
+// ograds[0]: dL/dxgrad
+// inputs[0]: dL/dy
+// inputs[1]: x
+// f(x) = y = log(x)
+// f'(x) = 1/x
// f''(x) = -1 * (f'(x) * f'(x))
-auto gx = nnvm::NodeEntry{n};
-auto ggx_mid = MakeNode("elemwise_mul", n->attrs.name +
"_backward_mid_grad_grad",
-{gx, gx}, nullptr, );
-auto ggx = MakeNode("negative", n->attrs.name + "_backward_grad_grad",
-{nnvm::NodeEntry{ggx_mid}}, nullptr, );
+auto dydx_mul_dldy = nnvm::NodeEntry{n}; // f'(x) * head_grads
+auto dlogx = MakeNode("reciprocal", n->attrs.name + "_dlogx",
+{n->inputs[1]}, nullptr, );
+auto d2ydx2_mid = MakeNode("elemwise_mul", n->attrs.name + "_d2ydx2_mid",
+{dydx_mul_dldy, nnvm::NodeEntry{dlogx}}, nullptr,
);
+auto d2ydx2 = MakeNode("negative", n->attrs.name + "_d2ydx2",
+{nnvm::NodeEntry{d2ydx2_mid}}, nullptr, );
std::vector ret;
ret.emplace_back(MakeNode("elemwise_mul", n->attrs.name +
"_backward_grad_grad",
- {ograds[0], gx}, nullptr, ));
+ {ograds[0], nnvm::NodeEntry{dlogx}}, nullptr,
));
ret.emplace_back(MakeNode("elemwise_mul", n->attrs.name +
"_backward_grad_grad_inp",
- {ograds[0], nnvm::NodeEntry{ggx}}, nullptr, ));
+ {ograds[0], nnvm::NodeEntry{d2ydx2}}, nullptr,
));
return ret;
});
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU_DR(_backward_log10,
unary_bwd)
.set_attr("FGradient",
[](const nnvm::NodePtr& n, const std::vector& ograds) {
-// For f(x) -> f = log10
+// ograds[0]: dL/dxgrad
+// inputs[0]: dL/dy
+// inputs[1]: x
+// f(x) = y = log10(x)
// f'(x) = 1 / (log(10) * x)
// f''(x) = -1 * (f'(x) * 1/x)
-auto gx = nnvm::NodeEntry{n, 0, 0};
-auto g_lx = MakeNode("reciprocal", n->attrs.name + "_backward_log_grad",
+auto dydx_mul_dldy = nnvm::NodeEntry{n}; // f'(x) * head_grads
+auto dydx = MakeNode("elemwise_div", n->attrs.name + "_dydx",
+{n->inputs[0]}, nullptr, );
+auto dlogx = MakeNode("reciprocal", n->attrs.name + "_dlogx",
{n->inputs[1]}, nullptr, );
-auto ggx_mid = MakeNode("elemwise_mul", n->attrs.name +
"_backward_mid_grad_grad",
-{gx, nnvm::NodeEntry{g_lx}}, nullptr, );
-auto ggx = MakeNode("negative", n->attrs.name + "_backward_grad_grad",
-{nnvm::NodeEntry{ggx_mid}}, nullptr, );
+auto d2ydx2_mid = MakeNode("elemwise_mul", n->attrs.name + "_d2ydx2_mid",
+{dydx_mul_dldy, nnvm::NodeEntry{dlogx}}, nullptr,
);
+auto d2ydx2 = MakeNode("negative", n->attrs.name + "_d2ydx2",
+{nnvm::NodeEntry{d2ydx2_mid}}, nullptr, );
std::vector ret;
ret.emplace_back(MakeNode("elemwise_mul", n->attrs.name +
"_backward_grad_grad",
- {ograds[0], gx}, nullptr, ));
+ {ograds[0], nnvm::NodeEntry{dydx}}, nullptr, ));
ret.emplace_back(MakeNode("elemwise_mul", n->attrs.name +
"_backward_grad_grad_inp",
- {ograds[0], nnvm::NodeEntry{ggx}}, nullptr, ));
+ {ograds[0], nnvm::NodeEntry{d2ydx2}}, nullptr,
));
return ret;
});
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU_DR(_backward_log2,
unary_bwd)
.set_attr("FGradient",
[](const nnvm::NodePtr& n, const std::vector& ograds) {
-// For f(x) -> f = log2
+// ograds[0]: dL/dxgrad
+// inputs[0]: dL/dy
+// inputs[1]: x
+// f(x) = y = log10(x)
Review comment:
```suggestion
// f(x) = y = log2(x)
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad log URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r293485580 ## File path: tests/python/unittest/test_higher_order_grad.py ## @@ -66,13 +66,26 @@ def grad_grad_op(x): def check_second_order_unary(x, op, grad_grad_op): x = nd.array(x) -expect_grad_grad = grad_grad_op(x) +grad_grad_x = grad_grad_op(x) x.attach_grad() + +# Manual head_grads. +y_grad = nd.random.normal(shape=x.shape) +head_grad_grads = nd.random.normal(shape=x.shape) + +# Perform compute. with autograd.record(): y = op(x) -y_grad = autograd.grad(y, x, create_graph=True, retain_graph=True)[0] -y_grad.backward() -assert_almost_equal(expect_grad_grad.asnumpy(), x.grad.asnumpy()) +x_grad = autograd.grad(y, x, head_grads=y_grad, Review comment: Can you explicitly specify the argument as ```suggestion x_grad = autograd.grad(heads=y, variables=x, head_grads=y_grad, create_graph=True, retain_graph=True) ``` I think this will make it easier to understand. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad log URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r293484925 ## File path: tests/python/unittest/test_higher_order_grad.py ## @@ -27,52 +27,79 @@ def test_log(): def log(x): return nd.log(x) +def grad_op(x): +return 1/x + def grad_grad_op(x): return -1/(x**2) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log, grad_grad_op) +check_second_order_unary(array, log, grad_op, grad_grad_op) @with_seed() def test_log2(): def log2(x): return nd.log2(x) +def grad_op(x): +return 1/(x * math.log(2)) + def grad_grad_op(x): return -1/((x**2) * math.log(2)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log2, grad_grad_op) +check_second_order_unary(array, log2, grad_op, grad_grad_op) @with_seed() def test_log10(): def log10(x): return nd.log10(x) +def grad_op(x): +return 1/(x * math.log(10)) + def grad_grad_op(x): return -1/((x**2) * math.log(10)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log10, grad_grad_op) +check_second_order_unary(array, log10, grad_op, grad_grad_op) -def check_second_order_unary(x, op, grad_grad_op): +def check_second_order_unary(x, op, grad_op, grad_grad_op): x = nd.array(x) -expect_grad_grad = grad_grad_op(x) +grad_x = grad_op(x) +grad_grad_x = grad_grad_op(x) x.attach_grad() + +# Manual head_grads. +head_grads = nd.random.normal(shape=x.shape) +head_grad_grads = nd.random.normal(shape=x.shape) Review comment: Thanks for the clarification. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad log URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r290902233 ## File path: tests/python/unittest/test_higher_order_grad.py ## @@ -27,52 +27,79 @@ def test_log(): def log(x): return nd.log(x) +def grad_op(x): +return 1/x + def grad_grad_op(x): return -1/(x**2) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log, grad_grad_op) +check_second_order_unary(array, log, grad_op, grad_grad_op) @with_seed() def test_log2(): def log2(x): return nd.log2(x) +def grad_op(x): +return 1/(x * math.log(2)) + def grad_grad_op(x): return -1/((x**2) * math.log(2)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log2, grad_grad_op) +check_second_order_unary(array, log2, grad_op, grad_grad_op) @with_seed() def test_log10(): def log10(x): return nd.log10(x) +def grad_op(x): +return 1/(x * math.log(10)) + def grad_grad_op(x): return -1/((x**2) * math.log(10)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log10, grad_grad_op) +check_second_order_unary(array, log10, grad_op, grad_grad_op) -def check_second_order_unary(x, op, grad_grad_op): +def check_second_order_unary(x, op, grad_op, grad_grad_op): x = nd.array(x) -expect_grad_grad = grad_grad_op(x) +grad_x = grad_op(x) +grad_grad_x = grad_grad_op(x) x.attach_grad() + +# Manual head_grads. +head_grads = nd.random.normal(shape=x.shape) +head_grad_grads = nd.random.normal(shape=x.shape) +head_grads.attach_grad() + +# Perform compute. with autograd.record(): y = op(x) -y_grad = autograd.grad(y, x, create_graph=True, retain_graph=True)[0] -y_grad.backward() -assert_almost_equal(expect_grad_grad.asnumpy(), x.grad.asnumpy()) +y_grad = autograd.grad(y, x, head_grads=head_grads, Review comment: This variable is actually dL/dx, maybe rename it to `x_grad` for better readability? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad log URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r290902895 ## File path: tests/python/unittest/test_higher_order_grad.py ## @@ -27,52 +27,79 @@ def test_log(): def log(x): return nd.log(x) +def grad_op(x): +return 1/x + def grad_grad_op(x): return -1/(x**2) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log, grad_grad_op) +check_second_order_unary(array, log, grad_op, grad_grad_op) @with_seed() def test_log2(): def log2(x): return nd.log2(x) +def grad_op(x): +return 1/(x * math.log(2)) + def grad_grad_op(x): return -1/((x**2) * math.log(2)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log2, grad_grad_op) +check_second_order_unary(array, log2, grad_op, grad_grad_op) @with_seed() def test_log10(): def log10(x): return nd.log10(x) +def grad_op(x): +return 1/(x * math.log(10)) + def grad_grad_op(x): return -1/((x**2) * math.log(10)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log10, grad_grad_op) +check_second_order_unary(array, log10, grad_op, grad_grad_op) -def check_second_order_unary(x, op, grad_grad_op): +def check_second_order_unary(x, op, grad_op, grad_grad_op): x = nd.array(x) -expect_grad_grad = grad_grad_op(x) +grad_x = grad_op(x) +grad_grad_x = grad_grad_op(x) x.attach_grad() + +# Manual head_grads. +head_grads = nd.random.normal(shape=x.shape) +head_grad_grads = nd.random.normal(shape=x.shape) Review comment: I still don't understand what this variable is mathematically... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad log URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r290902765 ## File path: tests/python/unittest/test_higher_order_grad.py ## @@ -27,52 +27,79 @@ def test_log(): def log(x): return nd.log(x) +def grad_op(x): +return 1/x + def grad_grad_op(x): return -1/(x**2) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log, grad_grad_op) +check_second_order_unary(array, log, grad_op, grad_grad_op) @with_seed() def test_log2(): def log2(x): return nd.log2(x) +def grad_op(x): +return 1/(x * math.log(2)) + def grad_grad_op(x): return -1/((x**2) * math.log(2)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log2, grad_grad_op) +check_second_order_unary(array, log2, grad_op, grad_grad_op) @with_seed() def test_log10(): def log10(x): return nd.log10(x) +def grad_op(x): +return 1/(x * math.log(10)) + def grad_grad_op(x): return -1/((x**2) * math.log(10)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log10, grad_grad_op) +check_second_order_unary(array, log10, grad_op, grad_grad_op) -def check_second_order_unary(x, op, grad_grad_op): +def check_second_order_unary(x, op, grad_op, grad_grad_op): x = nd.array(x) -expect_grad_grad = grad_grad_op(x) +grad_x = grad_op(x) +grad_grad_x = grad_grad_op(x) x.attach_grad() + +# Manual head_grads. +head_grads = nd.random.normal(shape=x.shape) Review comment: Rename this to `y_grad` as it is dL/dy? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad log URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r290902233 ## File path: tests/python/unittest/test_higher_order_grad.py ## @@ -27,52 +27,79 @@ def test_log(): def log(x): return nd.log(x) +def grad_op(x): +return 1/x + def grad_grad_op(x): return -1/(x**2) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log, grad_grad_op) +check_second_order_unary(array, log, grad_op, grad_grad_op) @with_seed() def test_log2(): def log2(x): return nd.log2(x) +def grad_op(x): +return 1/(x * math.log(2)) + def grad_grad_op(x): return -1/((x**2) * math.log(2)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log2, grad_grad_op) +check_second_order_unary(array, log2, grad_op, grad_grad_op) @with_seed() def test_log10(): def log10(x): return nd.log10(x) +def grad_op(x): +return 1/(x * math.log(10)) + def grad_grad_op(x): return -1/((x**2) * math.log(10)) arrays = random_arrays((2, 2), (2, 3), (4, 5, 2), (3, 1, 4, 5)) for array in arrays: -check_second_order_unary(array, log10, grad_grad_op) +check_second_order_unary(array, log10, grad_op, grad_grad_op) -def check_second_order_unary(x, op, grad_grad_op): +def check_second_order_unary(x, op, grad_op, grad_grad_op): x = nd.array(x) -expect_grad_grad = grad_grad_op(x) +grad_x = grad_op(x) +grad_grad_x = grad_grad_op(x) x.attach_grad() + +# Manual head_grads. +head_grads = nd.random.normal(shape=x.shape) +head_grad_grads = nd.random.normal(shape=x.shape) +head_grads.attach_grad() + +# Perform compute. with autograd.record(): y = op(x) -y_grad = autograd.grad(y, x, create_graph=True, retain_graph=True)[0] -y_grad.backward() -assert_almost_equal(expect_grad_grad.asnumpy(), x.grad.asnumpy()) +y_grad = autograd.grad(y, x, head_grads=head_grads, Review comment: This variable is actually dL/dx, maybe rename it to `xgrad` for better readability? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-mxnet] apeforest commented on a change in pull request #15120: [bug] fix higher grad log
apeforest commented on a change in pull request #15120: [bug] fix higher grad
log
URL: https://github.com/apache/incubator-mxnet/pull/15120#discussion_r290538567
##
File path: src/operator/tensor/elemwise_unary_op_basic.cc
##
@@ -1074,16 +1074,19 @@
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU_DR(_backward_log,
[](const nnvm::NodePtr& n, const std::vector& ograds) {
// For f(x) -> f = log
// f''(x) = -1 * (f'(x) * f'(x))
-auto gx = nnvm::NodeEntry{n};
+auto gx_mul_head_grads = nnvm::NodeEntry{n}; // f'(x) * head_grads
+auto head_grads = nnvm::NodeEntry{n->inputs[0]};
+auto g_lx = MakeNode("reciprocal", n->attrs.name + "_backward_log_grad",
+{n->inputs[1]}, nullptr, );
auto ggx_mid = MakeNode("elemwise_mul", n->attrs.name +
"_backward_mid_grad_grad",
-{gx, gx}, nullptr, );
+{gx_mul_head_grads, nnvm::NodeEntry{g_lx}},
nullptr, );
auto ggx = MakeNode("negative", n->attrs.name + "_backward_grad_grad",
{nnvm::NodeEntry{ggx_mid}}, nullptr, );
std::vector ret;
ret.emplace_back(MakeNode("elemwise_mul", n->attrs.name +
"_backward_grad_grad",
- {ograds[0], gx}, nullptr, ));
+ {ograds[0], nnvm::NodeEntry{g_lx}}, nullptr, ));
Review comment:
Still looking into this. The first output should be the gradient of y_grad.
However, the `head_grads.grad` does not get the value. I suspect the returned
value from this function is dropped in the gradient calculation in
imperative.cc. I will look more into this. Stay tuned.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
