Reference implementation for Tile op#2641
Reference implementation for Tile op#2641postrational merged 32 commits intoopenvinotoolkit:masterfrom
Conversation
72bda1b to
1744af5
Compare
ngraph/core/reference/include/ngraph/runtime/reference/tile.hpp
Outdated
Show resolved
Hide resolved
…Change input_rank to be constant int
| const char* arg, char* out, const Shape& in_shape, const Shape& out_shape, size_t elem_size) | ||
| namespace | ||
| { | ||
| bool is_axis_upper_bound(const Shape& shape, |
There was a problem hiding this comment.
Can you remove the run param from this function and check axis-- == 0 in the main loop?
There was a problem hiding this comment.
Decrement of axis should happen in every iteration of inner while loop. If I move this check to the main loop it will decrease only once.
There was a problem hiding this comment.
Using struct that keep track of indices, axis and run may be the good solution. Additionally this structure can have is axis upper_bound method.
| memcpy(out + output_transform.index(output_coord) * elem_size, | ||
| arg + input_transform.index(input_coord) * elem_size, | ||
| elem_size); | ||
| while (is_axis_upper_bound(indices, axis, in_shape_expanded)) |
There was a problem hiding this comment.
Based on the name is_axis_upper_bound user doesn't know that input params are changed which can be confused.
I think that more clear will be changing it to
calculate_next_axis()
postrational
left a comment
There was a problem hiding this comment.
At this point code LGTM.
Can we add a few lines of comments to describe what each loop should be doing?
|
@postrational Please take a look one more time at this PR. |
Change tile reference implementation to make it work faster (remove CoordinateTransform). Also remove 2 tests from interpreter manifest