You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/06/23 20:06:47 UTC
[GitHub] [tvm] altanh opened a new pull request #8318: [Relay][Training] fix first-order AD tuple/projection expr duplication
altanh opened a new pull request #8318:
URL: https://github.com/apache/tvm/pull/8318
Tuples constructions and projections were not handled correctly (in particular, they were not reconstructed using the let bindings of their inputs) which led to expression duplication. Often the CSE pass is able to eliminate this erroneous duplication when first-order AD was paired with ToGNF but sometimes it can't (which we observed in BERT training, leading to many duplicated matmuls).
cc @MarisaKirisame @jroesch
--
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:
users@infra.apache.org
[GitHub] [tvm] jroesch commented on a change in pull request #8318: [Relay][Training] fix first-order AD tuple/projection expr duplication
Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #8318:
URL: https://github.com/apache/tvm/pull/8318#discussion_r657524179
##########
File path: src/relay/transforms/first_order_gradient.cc
##########
@@ -174,11 +174,14 @@ struct FirstOrderReverseAD : ExprFunctor<ADValue(const Expr&)> {
}
ADValue VisitExpr_(const TupleGetItemNode* op) final {
- Expr e = GetRef<Expr>(op);
ADValue tup = VisitExpr(op->tuple);
- auto tt = op->tuple->checked_type().as<TupleTypeNode>();
+ TupleType tt = Downcast<TupleType>(op->tuple->checked_type());
size_t idx = op->index;
- auto ret = std::make_shared<ADTensor>(ll, e, diag_ctx);
+ // reconstruct projection using let-bound variable to avoid duplicating input tuple
+ TupleGetItem orig = TupleGetItem(tup->get<ADTensor>().forward, idx);
+ orig->checked_type_ = op->checked_type();
+ auto ret = std::make_shared<ADTensor>(ll, orig, diag_ctx);
+ // for orig = pi(tup, i), pi_grad(tup, i, g) = G where pi(G, i) = g and pi(G, j) = 0 for j != i
Review comment:
Explain more here or delete
--
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:
users@infra.apache.org
[GitHub] [tvm] altanh commented on a change in pull request #8318: [Relay][Training] fix first-order AD tuple/projection expr duplication
Posted by GitBox <gi...@apache.org>.
altanh commented on a change in pull request #8318:
URL: https://github.com/apache/tvm/pull/8318#discussion_r657526529
##########
File path: src/relay/transforms/first_order_gradient.cc
##########
@@ -174,11 +174,14 @@ struct FirstOrderReverseAD : ExprFunctor<ADValue(const Expr&)> {
}
ADValue VisitExpr_(const TupleGetItemNode* op) final {
- Expr e = GetRef<Expr>(op);
ADValue tup = VisitExpr(op->tuple);
- auto tt = op->tuple->checked_type().as<TupleTypeNode>();
+ TupleType tt = Downcast<TupleType>(op->tuple->checked_type());
size_t idx = op->index;
- auto ret = std::make_shared<ADTensor>(ll, e, diag_ctx);
+ // reconstruct projection using let-bound variable to avoid duplicating input tuple
+ TupleGetItem orig = TupleGetItem(tup->get<ADTensor>().forward, idx);
+ orig->checked_type_ = op->checked_type();
+ auto ret = std::make_shared<ADTensor>(ll, orig, diag_ctx);
+ // for orig = pi(tup, i), pi_grad(tup, i, g) = G where pi(G, i) = g and pi(G, j) = 0 for j != i
Review comment:
sure, this is just describing how the gradient for a projection is propagated back to the original tuple
--
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:
users@infra.apache.org
[GitHub] [tvm] jroesch merged pull request #8318: [Relay][Training] fix first-order AD tuple/projection expr duplication
Posted by GitBox <gi...@apache.org>.
jroesch merged pull request #8318:
URL: https://github.com/apache/tvm/pull/8318
--
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:
users@infra.apache.org