Reference implementation for ScatterUpdate #1678
Conversation
| // | ||
| // for i_coord in indices[m, n, ..., p]: | ||
| // i_idx = index(i_coord) | ||
| // for d_coord in slice data[..., i_idx, ...] && |
There was a problem hiding this comment.
This comment makes understanding the algorithm harder. What does "slice data" mean? And how should we understand two for-loops joined with &&?
There was a problem hiding this comment.
Slice here has NumPy meaning so, data[..., i_idx, ....] is a shortcut for a slice of n-dimensional tensor of input data. The ellipsis stands for : symbol, which gets all values from respective axis, for all dimensions up to i_idx (the axis at which we have update indexes) and following it till the end.
|
Could you measure the impact on binary size? Also I see a lot of template code. Can we reduce it and use template code only when it is needed? |
|
@postrational Please feel free to assign this PR to anyone if more work would be needed I think my part here is done. |
ngraph/core/include/ngraph/runtime/reference/scatter_update.hpp
Outdated
Show resolved
Hide resolved
ngraph/core/include/ngraph/runtime/reference/scatter_update.hpp
Outdated
Show resolved
Hide resolved
I see that in the meantime other reference implementation for ScatterUpdate #1494 was merged into master but with a template approach and without the As @ilyachur requested, I removed the template code (diff) from the reference implementation proposed in this PR what decreased Please rereview these changes. |
ngraph/core/reference/include/ngraph/runtime/reference/scatter_update.hpp
Outdated
Show resolved
Hide resolved
ngraph/core/reference/include/ngraph/runtime/reference/scatter_update.hpp
Outdated
Show resolved
Hide resolved
Thanks, I will take a look. At the current moment we use ScatterUpdate implementation only for tests and it don't affect the size of nGraph library. If we add evaluate methods and start to use reference implementations we need to think about binary size. |
* Reference implementation for ScatterUpdate and use of it in evaluate. * Review comments. Clarify comments. * Update file directory. * Replace scatter_update reference implementation in ngraph/core/reference/ * Remove template code from ScatterUpdate reference implementation * Apply review requests Co-authored-by: mitruska <[email protected]>
* Reference implementation for ScatterUpdate and use of it in evaluate. * Review comments. Clarify comments. * Update file directory. * Replace scatter_update reference implementation in ngraph/core/reference/ * Remove template code from ScatterUpdate reference implementation * Apply review requests Co-authored-by: mitruska <[email protected]>
No description provided.