Skip to content

G-API: ONNX. Adding INT64-32 conversion for output.#19752

Merged
alalek merged 17 commits intoopencv:masterfrom
mpashchenkov:mp/onnx-int64-to-32
Mar 30, 2021
Merged

G-API: ONNX. Adding INT64-32 conversion for output.#19752
alalek merged 17 commits intoopencv:masterfrom
mpashchenkov:mp/onnx-int64-to-32

Conversation

@mpashchenkov
Copy link
Copy Markdown
Contributor

@mpashchenkov mpashchenkov commented Mar 19, 2021

Adding INT64-32 conversion for output.

ONNX's RCNN contains output (labels) with INT64 type.
INT64 type isn't supported for cv::Mat. Conversion to INT32 is used for this output.

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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

Custom build:

force_builders=Custom,Custom Win,Custom Mac
build_gapi_standalone:Linux x64=ade-0.1.1f
build_gapi_standalone:Win64=ade-0.1.1f
build_gapi_standalone:Mac=ade-0.1.1f
build_gapi_standalone:Linux x64 Debug=ade-0.1.1f

Xbuild_image:Custom=centos:7
Xbuildworker:Custom=linux-1
Xbuild_gapi_standalone:Custom=ade-0.1.1f

Xbuild_image:Custom=ubuntu-openvino-2021.3.0:20.04
build_image:Custom Win=openvino-2021.2.0
build_image:Custom Mac=openvino-2021.2.0

test_modules:Custom=gapi,python2,python3,java
test_modules:Custom Win=gapi,python2,python3,java
test_modules:Custom Mac=gapi,python2,python3,java

buildworker:Custom=linux-1
# disabled due high memory usage: test_opencl:Custom=ON
Xtest_opencl:Custom=OFF
Xtest_bigdata:Custom=1

build_image:Custom=ubuntu-onnx:20.04

@TolyaTalamanov
Copy link
Copy Markdown
Contributor

@mpashchenkov pay attention that Mac and Win build not queued

@mpashchenkov mpashchenkov requested a review from OrestChura March 26, 2021 08:41
Copy link
Copy Markdown
Contributor

@dmatveev dmatveev left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +206 to +207
std::transform(ptr, ptr + total, mt_ptr,
[](int64_t el) { return static_cast<int>(el); });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hmm what about strides? I'd put an assert here expecting a Mat is continuous.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It makes sense to place checks before related statements.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed. But i don't see where gap can appear in Mat for current implementation.

@mpashchenkov
Copy link
Copy Markdown
Contributor Author

@TolyaTalamanov, please check last update before merge.

Copy link
Copy Markdown
Contributor

@TolyaTalamanov TolyaTalamanov left a comment

Choose a reason for hiding this comment

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

Great! 🔥

LGTM 👍

@alalek I believe it can be merged!

@TolyaTalamanov TolyaTalamanov requested a review from alalek March 30, 2021 14:09
@alalek
Copy link
Copy Markdown
Member

alalek commented Mar 30, 2021

Why is build configuration not enabled to check ONNX code?

@alalek alalek merged commit 69fc0ac into opencv:master Mar 30, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
a-sajjad72 pushed a commit to a-sajjad72/opencv that referenced this pull request Mar 30, 2023
G-API: ONNX. Adding INT64-32 conversion for output.

* Added int64 to 32 conversion

* Added warning

* Added type checks for all toCV

* Added type checks for tests

* Small fixes

* Const for fixture in test

* std::tuple if retutn value for toCV

* Mistake

* Changed toCV for tests

* Added Assert

* Fix for comments

* One conversion for ONNX and IE

* Clean up

* One more fix

* Added copyFromONNX

* Removed warning

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants