You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@singa.apache.org by GitBox <gi...@apache.org> on 2020/07/03 10:41:02 UTC
[GitHub] [singa] Shashankwer opened a new pull request #760: Scatter
Shashankwer opened a new pull request #760:
URL: https://github.com/apache/singa/pull/760
Adding ScatterElements. The implementation is same as pytorch and onnx implementation. The implementation follows the numpy manipulation due to the use of advanced indexing. The use of advance indexing is to reduce the time complexity from O(n^dim) to O(n) for the operation.
----------------------------------------------------------------
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] [singa] joddiy edited a comment on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
joddiy edited a comment on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657281203
Please check my PR:
https://github.com/Shashankwer/singa/pull/1
For the arguments of forward function, we should only contain the argument which has a gradient. for example, in this operator, only x have the gradient, so we move other arguments(indices and updates) to `__init__` function.
By the way, I think the advanced indexing idea is fine, could we implement it without numpy? since we will move this implement to c++ end later. Any function using the dependency will be a problem then.
----------------------------------------------------------------
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] [singa] joddiy commented on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
joddiy commented on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657281203
Please check my PR:
https://github.com/Shashankwer/singa/pull/1
There are two points you should pay attention:
1. the implement should be as clear as possible.
2. for the arguments of forward function, we should only contain the argument which has a gradient. for example, in this operator, only x have the gradient, so we move other arguments(indices and updates) to `__init__` function.
----------------------------------------------------------------
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] [singa] joddiy edited a comment on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
joddiy edited a comment on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657401578
> Hi @joddiy,
>
> Would we be considering only 2 dimensions for the scatter_elements operation?
> For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
> Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
>
> Thanks and Regards,
> Shashank Nigam
Hi, Shashank
> Would we be considering only 2 dimensions for the scatter_elements operation?
Yes, according to the onnx [schemar](https://github.com/onnx/onnx/blob/master/docs/Changelog.md#ScatterElements-11), the input should be a 2d tensor.
> For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
Sure, of course, however, even we store the indexes, we still iterate through all these indexes when we use them, right? it cannot reduce the time complexity.
> Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
Let us see this code, actually, for elements in the `self.updates` tensor, we use each of them for only one time, right? even we stores these indexes as a list of tuple, we still use each of time for one time, right? how can we save complexity of iterating?
```
u_shape = self.updates.shape
y = _x.copy()
for i in range(u_shape[0]):
for j in range(u_shape[1]):
idx = int(self.indices[i][j])
if self.axis == 0:
y[idx][j] = self.updates[i][j]
else:
y[i][idx] = self.updates[i][j]
```
----------------------------------------------------------------
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] [singa] nudles merged pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
nudles merged pull request #760:
URL: https://github.com/apache/singa/pull/760
----------------------------------------------------------------
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] [singa] joddiy edited a comment on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
joddiy edited a comment on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657401578
> Hi @joddiy,
>
> Would we be considering only 2 dimensions for the scatter_elements operation?
> For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
> Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
>
> Thanks and Regards,
> Shashank Nigam
Hi, Shashank
> Would we be considering only 2 dimensions for the scatter_elements operation?
For now, all test cases are 2-d inputs, so we may only need to support the 2d inputs.
> For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
Sure, of course, however, even we store the indexes, we still iterate through all these indexes when we use them, right? it cannot reduce the time complexity.
> Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
Let us see this code, actually, for elements in the `self.updates` tensor, we use each of them for only one time, right? even we stores these indexes as a list of tuple, we still use each of time for one time, right? how can we save complexity of iterating?
```
u_shape = self.updates.shape
y = _x.copy()
for i in range(u_shape[0]):
for j in range(u_shape[1]):
idx = int(self.indices[i][j])
if self.axis == 0:
y[idx][j] = self.updates[i][j]
else:
y[i][idx] = self.updates[i][j]
```
----------------------------------------------------------------
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] [singa] Shashankwer commented on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
Shashankwer commented on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657501233
Hi @joddiy,
Thanks for the clarification, I did over consider the use of more than 2 dimensions so the questions were posted and also fancy indexing was used. Thanks again.
Thanks and Regards,
Shashank Nigam
----------------------------------------------------------------
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] [singa] Shashankwer commented on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
Shashankwer commented on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657324170
Hi @joddiy,
Would we be considering only 2 dimensions for the scatter_elements operation?
For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
Thanks and Regards,
Shashank Nigam
----------------------------------------------------------------
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] [singa] Shashankwer commented on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
Shashankwer commented on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-653482455
Following are the results of test_operation.py
```
.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s.s..s.s.s.s.s.s.s.s.s.s.s.sEsEs.s.s.s.s.s.s.s.s.s.s.s.s.ss.s.s.s.s.s.s.s.s.s.s.s.s.s
======================================================================
ERROR: test_numerical_gradients_check_for_lstm_cpu (__main__.TestPythonOperation)
----------------------------------------------------------------------
Traceback (most recent call last):
File "test_operation.py", line 491, in test_numerical_gradients_check_for_lstm_cpu
self._numerical_gradients_check_for_lstm_helper(cpu_dev)
File "test_operation.py", line 482, in _numerical_gradients_check_for_lstm_helper
auto_grads = autograd.gradients(loss1)
File "/usr/local/lib/python3.6/dist-packages/singa-3.0.0-py3.6.egg/singa/autograd.py", line 120, in gradients
grads[p] = dp
TypeError: unhashable type: 'Tensor'
======================================================================
ERROR: test_numerical_gradients_check_for_vallina_rnn_cpu (__main__.TestPythonOperation)
----------------------------------------------------------------------
Traceback (most recent call last):
File "test_operation.py", line 459, in test_numerical_gradients_check_for_vallina_rnn_cpu
self._numerical_gradients_check_for_vallina_rnn_helper(cpu_dev)
File "test_operation.py", line 450, in _numerical_gradients_check_for_vallina_rnn_helper
auto_grads = autograd.gradients(loss1)
File "/usr/local/lib/python3.6/dist-packages/singa-3.0.0-py3.6.egg/singa/autograd.py", line 120, in gradients
grads[p] = dp
TypeError: unhashable type: 'Tensor'
----------------------------------------------------------------------
Ran 186 tests in 0.613s
FAILED (errors=2, skipped=93)
```
Following is the results of test_onnx_backend.py
```
ss.s.s.s.sssssssss.s.s.s.s.s.s.s.s.s.sssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss.s.s.s.s.s.s.s.sssss.sss.sss.s.s.s.s.s.sssssssssss/usr/local/lib/python3.6/dist-packages/singa-3.0.0-py3.6.egg/singa/sonnx.py:1879: UserWarning: The imported opertor set verion 12 is larger than the supported version 11.
.format(imp.version, cls._opset_version))
/usr/local/lib/python3.6/dist-packages/singa-3.0.0-py3.6.egg/singa/sonnx.py:1886: UserWarning: The imported ir verion 6 is larger than the supported version 12.
.format(cls._ir_version, imp.version))
.sssssssssssssssss.s.s.s.s.s.sssss.s.sssss.s.s.s.sFs.s.s.s.s.s.sssssssss.s.s.s.s.s.s.s.s.s.s.s.sssss.sFloating point exception (core dumped)
```
Thanks and Regards,
Shashank
----------------------------------------------------------------
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] [singa] joddiy commented on pull request #760: Scatter
Posted by GitBox <gi...@apache.org>.
joddiy commented on pull request #760:
URL: https://github.com/apache/singa/pull/760#issuecomment-657401578
> Hi @joddiy,
>
> Would we be considering only 2 dimensions for the scatter_elements operation?
> For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
> Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
>
> Thanks and Regards,
> Shashank Nigam
Hi, Shashank
> Would we be considering only 2 dimensions for the scatter_elements operation?
Yes, according to the onnx [schemar](https://github.com/onnx/onnx/blob/master/docs/Changelog.md#ScatterElements-11), the input should be a 2d tensor.
> For backwards operation with scatter_elements can we store the indexes we process during the forward pass, this can help us reducing the time complexity for the operation?
Sure, of course, however, even we store the indexes, we still iterate through all these indexes when we use them, right? it cannot reduce the time complexity.
> Also for implementing the advanced indexing operation, we can implement it as a suboptimal operation for processing the index as a list of tuples and further indexing only based on those list of tuples? This can save complexity of iterating over dimension elements multiple times
Let us see this code, actually, for elements in the `self.updates` tensor, we use each of them for only one time, right? even we stores these indexes as a list of tuple, we still use each of time for one time, right? how can we save complexity of iterating?
```
u_shape = self.updates.shape
y = _x.copy()
for i in range(u_shape[0]):
for j in range(u_shape[1]):
idx = int(self.indices[i][j])
if self.axis == 0:
y[idx][j] = self.updates[i][j]
else:
y[i][idx] = self.updates[i][j]
```
----------------------------------------------------------------
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