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/05 04:10:16 UTC
[GitHub] [incubator-tvm] handar423 opened a new pull request #5695: fix small bug about dense_grad
handar423 opened a new pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695
To whom it may concern,
Hello, I am learning tvm and trying to write some training model with relay when I got an error in `dense_grad()` with the size of data(`5*4`) and weight(`3*4`). Furthermore, I found that it may be caused by a small bug in dense_grad():
the present dense_grad() is:
```python
@register_gradient("nn.dense")
def dense_grad(orig, grad):
"""Returns [grad' @ weight, data @ grad']"""
data, weight = orig.args
return [collapse_sum_like(transpose(grad) * weight, data),
collapse_sum_like(data * transpose(grad), weight)]
```
in a common situation, when we calculate the gradient of `dense(A(i * j), weight(k * j))`, we get grad matrix with size `i * k`, so in above `dense_grad()`, the first multiply operator get parameters with size `k * i` and `k * j`, the second one get paramenters with size `i * j` and `k * i`, so we can only avoid conflict when `i == j == k` or some of them are `1`.
To increase the robustness of the function, maybe we can modify it to:
```python
@register_gradient("nn.dense")
def dense_grad(orig, grad):
"""Returns [grad' @ weight, data @ grad']"""
data, weight = orig.args
return [collapse_sum_like(_nn.dense(grad, transpose(weight)), data),
collapse_sum_like(_nn.dense(transpose(grad), transpose(data)), weight)]
```
we change multiply(`*`) to `_nn.dense` so that it can handel matrix multiply as well. For above assumption, the first `_nn.dense()` get parameters with size `i * k` and `j * k` and give a result with size `i * j`, which is the same as `data`; the second one get parameters with size `k * i` and `j * i` and give a result with size `k * j`, which is the same as `weight`. We add an extra test case in test_dense_grad() to test its correctness.
I am just starting to learn about tvm, so I apologize if I miss some obvious things. Thank you very much!
----------------------------------------------------------------
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] srkreddy1238 commented on a change in pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on a change in pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695#discussion_r432812422
##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -472,8 +472,8 @@ def bias_add_grad(orig, grad):
def dense_grad(orig, grad):
"""Returns [grad' @ weight, data @ grad']"""
data, weight = orig.args
- return [collapse_sum_like(transpose(grad) * weight, data),
- collapse_sum_like(data * transpose(grad), weight)]
+ return [collapse_sum_like(_nn.dense(grad, transpose(weight)), data),
Review comment:
Please add units arg too to support batching.
``
return [collapse_sum_like(_nn.dense(grad, transpose(weight), units=weight.checked_type.shape[1]), data),
collapse_sum_like(_nn.dense(transpose(grad), transpose(data), units=data.checked_type.shape[1]), weight)]
``
----------------------------------------------------------------
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] handar423 commented on pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
handar423 commented on pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695#issuecomment-639246598
sorry for replying so late! So grateful if you are still here.
Thank you for the guidance, I misunderstood the definition of batching and units ago and everything actually works well with your code. (I treated `batch` as the first dimension of a 3D `data` by mistake at first thought.)
New version has been checked in, please help review that if there is anything else need modification, thank you very much!
----------------------------------------------------------------
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] handar423 closed pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
handar423 closed pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695
----------------------------------------------------------------
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] srkreddy1238 commented on a change in pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
srkreddy1238 commented on a change in pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695#discussion_r432831890
##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -472,8 +472,8 @@ def bias_add_grad(orig, grad):
def dense_grad(orig, grad):
"""Returns [grad' @ weight, data @ grad']"""
data, weight = orig.args
- return [collapse_sum_like(transpose(grad) * weight, data),
- collapse_sum_like(data * transpose(grad), weight)]
+ return [collapse_sum_like(_nn.dense(grad, transpose(weight)), data),
Review comment:
In above example of data (5, 4) and weight (3, 4) implies a dense with 4 inputs and yielding 3 outputs each for 5 units. Hence 5 here is the batch when we apply to a network.
We very well support units/batches. Can you share details on the error you got while units is added as arg ?
----------------------------------------------------------------
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] handar423 commented on a change in pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
handar423 commented on a change in pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695#discussion_r432826485
##########
File path: python/tvm/relay/op/_tensor_grad.py
##########
@@ -472,8 +472,8 @@ def bias_add_grad(orig, grad):
def dense_grad(orig, grad):
"""Returns [grad' @ weight, data @ grad']"""
data, weight = orig.args
- return [collapse_sum_like(transpose(grad) * weight, data),
- collapse_sum_like(data * transpose(grad), weight)]
+ return [collapse_sum_like(_nn.dense(grad, transpose(weight)), data),
Review comment:
Thank you for your response!
After correcting it, I found that both x86 and CUDA only support dense without batching, I tried testing with arm-cpu(mobile) but came into [python/tvm/relay/op/strategy/x86.py](https://github.com/apache/incubator-tvm/blob/master/python/tvm/relay/op/strategy/x86.py) and failed as well, so could you please tell me how to run dense with batching?
Thanks again!
----------------------------------------------------------------
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] tqchen commented on pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695#issuecomment-636141164
cc @junrushao1994 @MarisaKirisame
----------------------------------------------------------------
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] srkreddy1238 merged pull request #5695: fix small bug about dense_grad
Posted by GitBox <gi...@apache.org>.
srkreddy1238 merged pull request #5695:
URL: https://github.com/apache/incubator-tvm/pull/5695
----------------------------------------------------------------
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