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 2020/06/27 08:54:21 UTC
[GitHub] [incubator-tvm] t-vi opened a new pull request #5941: Amendments for gradients
t-vi opened a new pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941
- We fix the dtype handling of consts in generated gradients.
- We add a collapse_sum_to instruction mirroring the collapse_sum_like.
While for general definitions (potentially dynamic shapes),
collapse_sum_like is the first choice, when moving to static,
using collapse_sum_to will greatly simplify the graph.
(This simplification is not part of the PR.)
----------------------------------------------------------------
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] [incubator-tvm] t-vi commented on pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
t-vi commented on pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#issuecomment-650526467
@junrushao1994 too. :)
----------------------------------------------------------------
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] [incubator-tvm] MarisaKirisame commented on a change in pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on a change in pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#discussion_r447318960
##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1713,6 +1713,54 @@ RELAY_REGISTER_OP("collapse_sum_like")
.set_attr<FTVMCompute>("FTVMCompute", CollapseSumLikeCompute)
.set_attr<TOpPattern>("TOpPattern", kCommReduce);
+// CollapseSumTo: <A, B> -> B where CollapseSumTo(A, B) = B
Review comment:
BroadCast is a symmetric function that take two tensor type A, B, and return the broadcast type (I am confusing the type and term level a bit.). I am just listing constraint between the two elements with the equation after where.
what constraint does it exist between two argument of CollapseSumTo? If I get it I can 'collapse' A into B, which mean the type B can be broadcast to A right?
----------------------------------------------------------------
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] [incubator-tvm] t-vi commented on a change in pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
t-vi commented on a change in pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#discussion_r446799560
##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1713,6 +1713,54 @@ RELAY_REGISTER_OP("collapse_sum_like")
.set_attr<FTVMCompute>("FTVMCompute", CollapseSumLikeCompute)
.set_attr<TOpPattern>("TOpPattern", kCommReduce);
+// CollapseSumTo: <A, B> -> B where CollapseSumTo(A, B) = B
Review comment:
Oh, right. But Broadcast(A, B) = A right? Thanks for spotting this. I must admit that I'm not 100% sure I understand the notation.
----------------------------------------------------------------
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] [incubator-tvm] MarisaKirisame commented on a change in pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on a change in pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#discussion_r446755527
##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1713,6 +1713,54 @@ RELAY_REGISTER_OP("collapse_sum_like")
.set_attr<FTVMCompute>("FTVMCompute", CollapseSumLikeCompute)
.set_attr<TOpPattern>("TOpPattern", kCommReduce);
+// CollapseSumTo: <A, B> -> B where CollapseSumTo(A, B) = B
Review comment:
I am confused by this line. BroadCast(A, B) = B?
----------------------------------------------------------------
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] [incubator-tvm] t-vi commented on a change in pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
t-vi commented on a change in pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#discussion_r446799560
##########
File path: src/relay/op/tensor/transform.cc
##########
@@ -1713,6 +1713,54 @@ RELAY_REGISTER_OP("collapse_sum_like")
.set_attr<FTVMCompute>("FTVMCompute", CollapseSumLikeCompute)
.set_attr<TOpPattern>("TOpPattern", kCommReduce);
+// CollapseSumTo: <A, B> -> B where CollapseSumTo(A, B) = B
Review comment:
Oh, right. But Broadcast(A, B) = A right?
----------------------------------------------------------------
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] [incubator-tvm] MarisaKirisame commented on pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
MarisaKirisame commented on pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#issuecomment-651426510
> We add a collapse_sum_to instruction mirroring the collapse_sum_like.
While for general definitions (potentially dynamic shapes),
collapse_sum_like is the first choice, when moving to static,
using collapse_sum_to will greatly simplify the graph.
(This simplification is not part of the PR.)
Sounds cool. I assume you have plan for a PR that implement a pass to turn `like` into `to` function? There's a lot of them.
----------------------------------------------------------------
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] [incubator-tvm] MarisaKirisame merged pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
MarisaKirisame merged pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941
----------------------------------------------------------------
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] [incubator-tvm] t-vi commented on pull request #5941: Amendments for gradients
Posted by GitBox <gi...@apache.org>.
t-vi commented on pull request #5941:
URL: https://github.com/apache/incubator-tvm/pull/5941#issuecomment-650526408
@MarisaKirisame @tqchen If I can interest you in this.
I have more gradient work coming up.
----------------------------------------------------------------
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