-
Notifications
You must be signed in to change notification settings - Fork 6.8k
[MXNET-978] Higher Order Gradient Support sinh
, cosh
.
#15412
[MXNET-978] Higher Order Gradient Support sinh
, cosh
.
#15412
Conversation
sinh
, cosh
.sinh
, cosh
.
@mxnet-label-bot add [pr-awaiting-review] |
…to develop/add-higher-order/sinh-cosh
…to develop/add-higher-order/sinh-cosh
@apeforest Could you please review the PR? Thanks! |
@@ -92,7 +92,7 @@ The storage type of ``cos`` output is always dense | |||
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU(_backward_cos, unary_bwd<mshadow_op::cos_grad>) | |||
.set_attr<nnvm::FGradient>("FGradient", | |||
[](const nnvm::NodePtr& n, const std::vector<nnvm::NodeEntry>& ograds) { | |||
// ograds[0]: head_grad_grads (dL/dx_grad) | |||
// ograds[0]: head_grad_grads (dL/dy_grad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this? I think the original dL/dx_grad is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have a look and let me know what you think.
#15331 (comment)
@@ -49,7 +49,7 @@ The storage type of ``sin`` output depends upon the input storage type: | |||
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU_DR(_backward_sin, unary_bwd<mshadow_op::sin_grad>) | |||
.set_attr<nnvm::FGradient>("FGradient", | |||
[](const nnvm::NodePtr& n, const std::vector<nnvm::NodeEntry>& ograds) { | |||
// ograds[0]: head_grad_grads (dL/dxgrad) | |||
// ograds[0]: head_grad_grads (dL/dy_grad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this? I think the original dL/dx_grad is correct.
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU_DR(_backward_sinh, unary_bwd<mshadow_op::sinh_grad>) | ||
.set_attr<nnvm::FGradient>("FGradient", | ||
[](const nnvm::NodePtr& n, const std::vector<nnvm::NodeEntry>& ograds) { | ||
// ograds[0]: dL/dy_grad |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this? I think the original dL/dx_grad is correct.
MXNET_OPERATOR_REGISTER_BINARY_WITH_SPARSE_CPU(_backward_cosh, unary_bwd<mshadow_op::cosh_grad>) | ||
.set_attr<nnvm::FGradient>("FGradient", | ||
[](const nnvm::NodePtr& n, const std::vector<nnvm::NodeEntry>& ograds) { | ||
// ograds[0]: dL/dy_grad |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this? I think the original dL/dx_grad is correct.
95cadc1
to
794eefd
Compare
…to develop/add-higher-order/sinh-cosh
@kshitij12345 is this PR good to go for merge? |
I guess we should wait till #15531 is in. |
@larroy @apeforest Gentle ping for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* add higher order support for sinh cosh * add relevant tests * update comments * retrigger CI
Description
PR intends to add support for higher order gradient for
sinh
,cosh
.Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
sinh
,cosh
.