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/01/21 18:51:39 UTC

[GitHub] [tvm] altanh opened a new pull request #7323: [Relay][Training] Add more gradients

altanh opened a new pull request #7323:
URL: https://github.com/apache/tvm/pull/7323


   Added gradients for:
   - `concatenate`
   - `reshape_like`
   - `where`
   - `less_equal`
   
   cc @antinucleon @MarisaKirisame
   
   Somewhat tangential: the behavior for `split` in Python Relay is somewhat unintuitive as it actually returns a `TupleWrapper` and not a `Tuple`, perhaps this should be changed? (This caused an error when I was trying to use the result of `split` as a `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] slyubomirsky commented on pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
slyubomirsky commented on pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#issuecomment-765089486


   I've also been bitten by `TupleWrapper`s lol, maybe it shouldn't be the default, since it's only intended as a convenience when working with the Python APIs.


----------------------------------------------------------------
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 pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#issuecomment-767069386


   Looks fine to me


----------------------------------------------------------------
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 pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
altanh commented on pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#issuecomment-765102893


   > I've also been bitten by `TupleWrapper`s lol, maybe it shouldn't be the default, since it's only intended as a convenience when working with the Python APIs.
   
   Indeed, it also contradicts the documentation.


----------------------------------------------------------------
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] masahi merged pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #7323:
URL: https://github.com/apache/tvm/pull/7323


   


----------------------------------------------------------------
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 #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#discussion_r563997724



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -357,16 +357,24 @@ def global_avg_pool2d_grad(orig, grad):
     return [pool_grad]
 
 
-# not implemented, this is only for testing.
 @register_gradient("concatenate")
 def concatenate_grad(orig, grad):
+    """
+    Returns the gradient of concatenate, which is just the downstream gradient
+    split across the inputs.
+    """
     assert len(orig.args) == 1
     t = orig.args[0]
-    x = TupleGetItem(t, 0)
-    y = TupleGetItem(t, 1)
-    # Assume only two element in tuple rn.
-    # In the real implementation, concatenate_grad probably need to be implemented by an operator.
-    return [Tuple([zeros_like(x), zeros_like(y)])]
+
+    # calculate split indices. TODO(@altanh): support Any?

Review comment:
       Can we put a check and error here for Any?




----------------------------------------------------------------
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 #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#discussion_r563997724



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -357,16 +357,24 @@ def global_avg_pool2d_grad(orig, grad):
     return [pool_grad]
 
 
-# not implemented, this is only for testing.
 @register_gradient("concatenate")
 def concatenate_grad(orig, grad):
+    """
+    Returns the gradient of concatenate, which is just the downstream gradient
+    split across the inputs.
+    """
     assert len(orig.args) == 1
     t = orig.args[0]
-    x = TupleGetItem(t, 0)
-    y = TupleGetItem(t, 1)
-    # Assume only two element in tuple rn.
-    # In the real implementation, concatenate_grad probably need to be implemented by an operator.
-    return [Tuple([zeros_like(x), zeros_like(y)])]
+
+    # calculate split indices. TODO(@altanh): support Any?

Review comment:
       Can we put a check and error here for Any?

##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -357,16 +357,24 @@ def global_avg_pool2d_grad(orig, grad):
     return [pool_grad]
 
 
-# not implemented, this is only for testing.
 @register_gradient("concatenate")
 def concatenate_grad(orig, grad):
+    """
+    Returns the gradient of concatenate, which is just the downstream gradient
+    split across the inputs.
+    """
     assert len(orig.args) == 1
     t = orig.args[0]
-    x = TupleGetItem(t, 0)
-    y = TupleGetItem(t, 1)
-    # Assume only two element in tuple rn.
-    # In the real implementation, concatenate_grad probably need to be implemented by an operator.
-    return [Tuple([zeros_like(x), zeros_like(y)])]
+
+    # calculate split indices. TODO(@altanh): support Any?

Review comment:
       Can you either do it here, or open follow up issue and close this week?




----------------------------------------------------------------
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 #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
jroesch commented on a change in pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#discussion_r563998220



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -357,16 +357,24 @@ def global_avg_pool2d_grad(orig, grad):
     return [pool_grad]
 
 
-# not implemented, this is only for testing.
 @register_gradient("concatenate")
 def concatenate_grad(orig, grad):
+    """
+    Returns the gradient of concatenate, which is just the downstream gradient
+    split across the inputs.
+    """
     assert len(orig.args) == 1
     t = orig.args[0]
-    x = TupleGetItem(t, 0)
-    y = TupleGetItem(t, 1)
-    # Assume only two element in tuple rn.
-    # In the real implementation, concatenate_grad probably need to be implemented by an operator.
-    return [Tuple([zeros_like(x), zeros_like(y)])]
+
+    # calculate split indices. TODO(@altanh): support Any?

Review comment:
       Can you either do it here, or open follow up issue and close this week?




----------------------------------------------------------------
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 #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
altanh commented on a change in pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#discussion_r564000478



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -357,16 +357,24 @@ def global_avg_pool2d_grad(orig, grad):
     return [pool_grad]
 
 
-# not implemented, this is only for testing.
 @register_gradient("concatenate")
 def concatenate_grad(orig, grad):
+    """
+    Returns the gradient of concatenate, which is just the downstream gradient
+    split across the inputs.
+    """
     assert len(orig.args) == 1
     t = orig.args[0]
-    x = TupleGetItem(t, 0)
-    y = TupleGetItem(t, 1)
-    # Assume only two element in tuple rn.
-    # In the real implementation, concatenate_grad probably need to be implemented by an operator.
-    return [Tuple([zeros_like(x), zeros_like(y)])]
+
+    # calculate split indices. TODO(@altanh): support Any?

Review comment:
       I can open an issue for this but more generally a lot of the gradients (and even inference compute functions) assume concrete shapes. It may be worth creating an Any support tracking page on a per-operator basis if we want something centralized




----------------------------------------------------------------
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 #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
altanh commented on a change in pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#discussion_r564000478



##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -357,16 +357,24 @@ def global_avg_pool2d_grad(orig, grad):
     return [pool_grad]
 
 
-# not implemented, this is only for testing.
 @register_gradient("concatenate")
 def concatenate_grad(orig, grad):
+    """
+    Returns the gradient of concatenate, which is just the downstream gradient
+    split across the inputs.
+    """
     assert len(orig.args) == 1
     t = orig.args[0]
-    x = TupleGetItem(t, 0)
-    y = TupleGetItem(t, 1)
-    # Assume only two element in tuple rn.
-    # In the real implementation, concatenate_grad probably need to be implemented by an operator.
-    return [Tuple([zeros_like(x), zeros_like(y)])]
+
+    # calculate split indices. TODO(@altanh): support Any?

Review comment:
       I can open an issue for this but more generally a lot of the gradients (and even inference compute functions) assume concrete shapes. It may be worth creating an Any support tracking page on a per-operator basis if we want something centralized




----------------------------------------------------------------
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 pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#issuecomment-767069386


   Looks fine to me


----------------------------------------------------------------
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] slyubomirsky commented on pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
slyubomirsky commented on pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#issuecomment-765089486


   I've also been bitten by `TupleWrapper`s lol, maybe it shouldn't be the default, since it's only intended as a convenience when working with the Python APIs.


----------------------------------------------------------------
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 pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
altanh commented on pull request #7323:
URL: https://github.com/apache/tvm/pull/7323#issuecomment-765102893


   > I've also been bitten by `TupleWrapper`s lol, maybe it shouldn't be the default, since it's only intended as a convenience when working with the Python APIs.
   
   Indeed, it also contradicts the documentation.


----------------------------------------------------------------
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] masahi merged pull request #7323: [Relay][Training] Add more gradients

Posted by GitBox <gi...@apache.org>.
masahi merged pull request #7323:
URL: https://github.com/apache/tvm/pull/7323


   


----------------------------------------------------------------
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