[ONNX] Extend ONNX Frontend with BitwiseAnd-18 operator #21585

rghvsh · 2023-12-11T16:42:24Z

Details:

Implemented ONNX frontend BitwiseAnd operator and corresponding tests.

Tickets:

Closes [Good First Issue]: Extend ONNX Frontend with BitwiseAnd-18 operator #20790

Added bitwise_and.prototxt and bitwise_and_broadcast_condition.prototxt.

mitruska

Thank you for the contribution, great work! 🚀
I've left one suggestion to the conversion code and hint to fix the CI checks related to enablement of ONNX BitwiseAnd tests marked as xfail.

src/frontends/onnx/frontend/src/op/bitwise_and.cpp

src/frontends/onnx/tests/onnx_import.in.cpp

Co-authored-by: Katarzyna Mitrus <[email protected]>

Removes the unblocked ONNX BitwiseAnd tests from the xfail list.

Removes the unblocked ONNX BitwiseAnd tests from the xfail list for compatibility api as well.

Corrects code style

src/frontends/onnx/frontend/src/op/bitwise_and.cpp

comply clang format code style

mitruska

LGTM, thank you for your quick response for all of the comments 🥇

If you enjoy contributing to OpenVINO, you are welcome to take a look at the other Good First Issue tasks. Especially you can be interested in the next ONNX Bitwise ops waiting for enablement, that should follow the changes from this PR.

rghvsh · 2023-12-14T15:31:55Z

LGTM, thank you for your quick response for all of the comments 🥇

If you enjoy contributing to OpenVINO, you are welcome to take a look at the other Good First Issue tasks. Especially you can be interested in the next ONNX Bitwise ops waiting for enablement, that should follow the changes from this PR.

[Good First Issue]: Extend ONNX Frontend with BitwiseOr-18 operator #20791

[Good First Issue]: Extend ONNX Frontend with BitwiseXor-18 operator #20792

[Good First Issue]: Extend ONNX Frontend with BitwiseNot-18 operator #20793

Thank You, this was fun. Would love to contribute more

gkrivor · 2023-12-15T09:11:45Z

build_jenkins

rghvsh · 2023-12-15T11:30:35Z

Hello! @mitruska @gkrivor what do the following tests do, I am unable to access more details about them. How do I fix these ?

Thanks
Raghav

@openvino-ci-bot
wheels_macos_release
Details
@openvino-ci-bot
wheels_manylinux2014_release
Details
@openvino-ci-bot
wheels_windows_vs2019_release
Details

gkrivor · 2023-12-15T12:02:12Z

Hi, ie_tests... - ignore please, it is a common issue at the moment.
Wheels failes with:

________ OnnxBackendNodeModelTest.test_bitwise_and_ui64_bcast_3v1d_cpu _________
AssertionError: 
Not equal to tolerance rtol=0.001, atol=1e-07
Mismatched elements: 3 / 60 (5%)
Max absolute difference: 2147483648
Max relative difference: 1.16415322e-10
 x: array([[[         0,          0,          0,          2,          0],
        [         0,          0,          0,          0,          0],
        [         0,          0,          0,          0,          0],...
 y: array([[[                   0,                    0,
                            0,                    2,
                            0],...

Could you try to reproduce it locally?

mitruska · 2023-12-15T12:29:28Z

Hello! @mitruska @gkrivor what do the following tests do, I am unable to access more details about them. How do I fix these ?

Hello, thank you for pointing this out. I can see one failing test within mentioned pipelines: tests_python/test_backend.py::OnnxBackendNodeModelTest::test_bitwise_and_ui64_bcast_3v1d_cpu.

I suppose it's related to the ui64 type of the test inputs, what can be tricky for the specific configurations.
As this test passed within openvino-ngraph-onnx-lin (OpenVINO_ONNX_CI Release) pipeline, it seems to be more internal issue for further investigation, not related to the ONNX BitwiseAnd conversion code.

If you are not able to reproduce it, I suggest to put this single test back on the xfail list for now, sorry for the inconvenience.

rghvsh · 2023-12-15T15:38:27Z

Was not able to reproduce this error I'll put this back on tests_python/test_backend.py and compatibility api.

…o xfail list

…_ui64_bcast_3v1d_cpu", back to the xfail list

mitruska · 2023-12-15T17:19:51Z

Was not able to reproduce this error I'll put this back on tests_python/test_backend.py and compatibility api.

Actually it requires to be marked as skip instead xfail, as the checks where the test pass will fail with xpass 🙃
So it should have dedicated label, please take a look at the example commit skipping test (7243274)
You can name it skip_bitwise_ui64.

rghvsh · 2023-12-17T11:37:01Z

Hello @mitruska @gkrivor ! Is this better?

gkrivor · 2023-12-17T12:59:10Z

build_jenkins

gkrivor · 2023-12-17T15:03:21Z

Looks like we have some issues in the CI. Please, do nothing, I'll merge master to you branch tomorrow and try again.

gkrivor · 2023-12-18T05:08:11Z

Huh, looks like I've broke your pr by refactoring onnx_import.in.cpp.

gkrivor · 2023-12-18T06:29:14Z

build_jenkins

rghvsh added 10 commits December 11, 2023 13:13

Create bitwise_and.hpp

9edeb5e

Create bitwise_and.cpp

d1689ea

Update bitwise_and.cpp

de75fb5

Update bitwise_and.hpp

5c29c1a

Update bitwise_and.hpp

2e86284

Update ops_bridge.cpp

12962b2

Added protobuf(.prototxt) files via upload.

d1d8867

Added bitwise_and.prototxt and bitwise_and_broadcast_condition.prototxt.

Update bitwise_and_broadcast_condition.prototxt

284f7a5

Update onnx_import.in.cpp

b24c17d

Merge branch 'openvinotoolkit:master' into onnxfe

f1686d8

rghvsh requested a review from a team as a code owner December 11, 2023 16:42

github-actions bot added the category: ONNX FE OpenVINO ONNX FrontEnd label Dec 11, 2023

rghvsh mentioned this pull request Dec 11, 2023

[Good First Issue]: Extend ONNX Frontend with BitwiseAnd-18 operator #20790

Closed

ilya-lavrenov added the ExternalPR External contributor label Dec 11, 2023

ilya-lavrenov assigned gkrivor Dec 11, 2023

mitruska reviewed Dec 11, 2023

View reviewed changes

src/frontends/onnx/frontend/src/op/bitwise_and.cpp Outdated Show resolved Hide resolved

src/frontends/onnx/tests/onnx_import.in.cpp Outdated Show resolved Hide resolved

rghvsh and others added 4 commits December 12, 2023 08:30

Update src/frontends/onnx/frontend/src/op/bitwise_and.cpp

ed0d9ee

Co-authored-by: Katarzyna Mitrus <[email protected]>

Update bitwise_and.cpp

c1898b2

Update test_backend.py

2d025c6

Removes the unblocked ONNX BitwiseAnd tests from the xfail list.

Update test_backend.py

b0dbb97

Removes the unblocked ONNX BitwiseAnd tests from the xfail list for compatibility api as well.

rghvsh requested a review from a team as a code owner December 12, 2023 14:50

github-actions bot added the category: Python API OpenVINO Python bindings label Dec 12, 2023

mitruska self-assigned this Dec 12, 2023

Update bitwise_and.cpp

9458460

Corrects code style

gkrivor reviewed Dec 13, 2023

View reviewed changes

src/frontends/onnx/frontend/src/op/bitwise_and.cpp Outdated Show resolved Hide resolved

mitruska reviewed Dec 13, 2023

View reviewed changes

src/frontends/onnx/frontend/src/op/bitwise_and.cpp Outdated Show resolved Hide resolved

rghvsh added 4 commits December 13, 2023 19:04

Update bitwise_and.cpp

376fce9

comply clang format code style

Merge branch 'master' into onnxfe

304be4b

Update bitwise_and.cpp

ac0faf1

Update onnx_import.in.cpp

94e32fc

rghvsh added 2 commits December 13, 2023 20:54

Update bitwise_and.cpp

bbce226

Update bitwise_and.cpp

1a87177

mitruska approved these changes Dec 14, 2023

View reviewed changes

mitruska requested a review from gkrivor December 14, 2023 15:30

rghvsh added 3 commits December 15, 2023 21:10

add "OnnxBackendNodeModelTest.test_bitwise_and_ui64_bcast_3v1d_cpu" t…

815d9d2

…o xfail list

Update test_backend.py add "OnnxBackendNodeModelTest.test_bitwise_and…

b20b667

…_ui64_bcast_3v1d_cpu", back to the xfail list

Merge branch 'master' into onnxfe

b2e9f85

rghvsh added 4 commits December 17, 2023 12:37

Add skip_bitwise_ui64 to __init__.py

ea5488d

Add skip_bitwise_ui64 test_backend.py

1d572b4

Add skip_bitwise_ui64 to __init__.py

85adb7b

add skip_bitwise_ui64 to test_backend.py

f8aab63

Merge branch 'master' into onnxfe

f78ed46

Update onnx_import.in.cpp

15f6344

gkrivor merged commit a4e8f9d into openvinotoolkit:master Dec 19, 2023
101 checks passed

mlukasze added this to the 2023.3 milestone Dec 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] Extend ONNX Frontend with BitwiseAnd-18 operator #21585

[ONNX] Extend ONNX Frontend with BitwiseAnd-18 operator #21585

rghvsh commented Dec 11, 2023 •

edited by mitruska

Loading

mitruska left a comment

mitruska left a comment

rghvsh commented Dec 14, 2023

gkrivor commented Dec 15, 2023

rghvsh commented Dec 15, 2023

gkrivor commented Dec 15, 2023

mitruska commented Dec 15, 2023

rghvsh commented Dec 15, 2023

mitruska commented Dec 15, 2023

rghvsh commented Dec 17, 2023

gkrivor commented Dec 17, 2023

gkrivor commented Dec 17, 2023

gkrivor commented Dec 18, 2023 •

edited

Loading

gkrivor commented Dec 18, 2023

[ONNX] Extend ONNX Frontend with BitwiseAnd-18 operator #21585

[ONNX] Extend ONNX Frontend with BitwiseAnd-18 operator #21585

Conversation

rghvsh commented Dec 11, 2023 • edited by mitruska Loading

Details:

Tickets:

mitruska left a comment

Choose a reason for hiding this comment

mitruska left a comment

Choose a reason for hiding this comment

rghvsh commented Dec 14, 2023

gkrivor commented Dec 15, 2023

rghvsh commented Dec 15, 2023

gkrivor commented Dec 15, 2023

mitruska commented Dec 15, 2023

rghvsh commented Dec 15, 2023

mitruska commented Dec 15, 2023

rghvsh commented Dec 17, 2023

gkrivor commented Dec 17, 2023

gkrivor commented Dec 17, 2023

gkrivor commented Dec 18, 2023 • edited Loading

gkrivor commented Dec 18, 2023

rghvsh commented Dec 11, 2023 •

edited by mitruska

Loading

gkrivor commented Dec 18, 2023 •

edited

Loading