Skip to content

add Gather implementation#21738

Merged
asmorkalov merged 1 commit intoopencv:4.xfrom
rogday:gather
Sep 19, 2022
Merged

add Gather implementation#21738
asmorkalov merged 1 commit intoopencv:4.xfrom
rogday:gather

Conversation

@rogday
Copy link
Copy Markdown
Member

@rogday rogday commented Mar 17, 2022

Merge with extra: opencv/opencv_extra#1007
Should resolve #20439, resolve #19957

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@rogday rogday added feature category: dnn (onnx) ONNX suport issues in DNN module labels Mar 17, 2022
@zihaomu
Copy link
Copy Markdown
Member

zihaomu commented Jun 27, 2022

Hi @rogday, How is the PR progress so far? Maybe I can do something if you want.

@rogday
Copy link
Copy Markdown
Member Author

rogday commented Jun 29, 2022

Hi @zihaomu, I think we need to wait for merge of @fengyuentau's extra blobs info so that we can reuse it for 1-d and 0-d support here. Otherwise I think it's pretty much done.

@fengyuentau
Copy link
Copy Markdown
Member

Hello @rogday , we came across the problem of lacking support for gather op with multiple indices. I wonder if you have time working on this pull request. If no, me or someone on my side can help you finish this pull request.

@fengyuentau fengyuentau self-assigned this Aug 27, 2022
@rogday
Copy link
Copy Markdown
Member Author

rogday commented Aug 27, 2022

Hello @rogday , we came across the problem of lacking support for gather op with multiple indices. I wonder if you have time working on this pull request. If no, me or someone on my side can help you finish this pull request.

Hi! Sorry for the delay. I'll give it a shot next week. Would you be okay with that?

@fengyuentau
Copy link
Copy Markdown
Member

fengyuentau commented Aug 27, 2022

I'll give it a shot next week. Would you be okay with that?

Sure, no problem. Thank you!

In case you need a ONNX model for testing, here is the one I used: gather_multiple_indices.onnx.zip. It is the same operator that dnn does support in the model as mentioned above, and is generated with my script.

@rogday rogday force-pushed the gather branch 2 times, most recently from 63c9d76 to 05cff0f Compare September 6, 2022 12:41
@rogday rogday marked this pull request as ready for review September 12, 2022 13:59
@rogday rogday requested a review from fengyuentau September 12, 2022 14:43
Copy link
Copy Markdown
Member

@fengyuentau fengyuentau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we turn on some onnx conformance tests of gather? Others look good to me

@rogday
Copy link
Copy Markdown
Member Author

rogday commented Sep 16, 2022

Could we turn on some onnx conformance tests of gather? Others look good to me

Unfortunately no - type(integer indices) and shape(less than 2d) issues in non-const context aren't handled by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: dnn (onnx) ONNX suport issues in DNN module feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't import ONNX model produced by keras2onnx DNN/ONNX: Gather layer with 4 indices is not supported

5 participants