【PaddlePaddle Hackathon 3】Add Paddle gather_nd operator #12355
Conversation
|
@liubo-intel Could you please take a review? Thanks! |
liubo-intel
added
category: PDPD FE
Hi @AndPuQing could you upload the a screen shoot for your uni-test ? |
Sorry, maybe i forgot. Now, uploaded. |
There was a problem hiding this comment.
Hi, @AndPuQing : Thanks for contribution. In case that OpenVINO 'GatherND' OP not support Paddle 'gather_nd' OP in some special cases, Could you please add some more additional test cases based on the comment that I left in 'generate_gather_nd.py' file? If such kind of un-support really exists, we may need to leave some 'TODO' notification in related codes.
Besides this, this PR looks good to me.
src/core/tests/frontend/paddle/test_models/gen_scripts/generate_gather_nd.py
Show resolved
Hide resolved
src/core/tests/frontend/paddle/test_models/gen_scripts/generate_gather_nd.py
Outdated
Show resolved
Hide resolved
|
@AndPuQing : looks like there is some issue on CI, could you please rebase this PR to the latest master branch? thanks. |
|
Are there any other issues with this PR? |
|
Hi, @ceciliapeng2011 : do you have any other comments about this PR? if none, could you please help merge it? tks. |
| auto shape = index_node.get_partial_shape(); | ||
| for (const auto& dim : shape) { | ||
| if (dim.is_static() && dim.get_length() == 0) | ||
| PADDLE_OP_CHECK(node, false, "zero dimension is not allowed for gather_nd Index"); |
There was a problem hiding this comment.
As I see from the specification zero index is supported by gather ND op:
https://docs.openvino.ai/latest/openvino_docs_ops_movement_GatherND_8.html#doxid-openvino-docs-ops-movement-gather-n-d-8
There was a problem hiding this comment.
Hi, @ilyachur : thanks for taking your time to help have a look at this PR. the 'indices' input value of OV 'gather_nd' can be '0', but as its specification describes 'A tensor of a rank not less than 1'. the 'indices' input rank should not be '0'(@AndPuQing has created an commented test case 'gather_nd_empty' to cover this).
Maybe we should change it to the following way?
if (shape.is_static() && shape.rank().get_length() == 0)
PADDLE_OP_CHECK(node, false, "zero 'indices' input rank is not allowed for gather_nd");
There was a problem hiding this comment.
Yes I think it will be more aligned with the specification
Details:
Reference