Skip to content

Add support for numpy.flip in OpenVINO backend #21132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add support for numpy.flip in OpenVINO backend #21132

wants to merge 2 commits into from

Conversation

PraroopChanda
Copy link

@PraroopChanda PraroopChanda commented Apr 4, 2025 •
edited
Loading

Summary

Fixes openvinotoolkit/openvino#29359
This PR adds OpenVINO backend support for numpy.flip in Keras 3.

Please Review the PR @rkazants

@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 68.42105% with 6 lines in your changes missing coverage. Please review.

Project coverage is 82.69%. Comparing base (7ed8edb) to head (3c98d30).

Files with missing lines Patch % Lines
keras/src/backend/openvino/numpy.py 68.42% 3 Missing and 3 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #21132      +/-   ##
==========================================
- Coverage   82.70%   82.69%   -0.01%     
==========================================
  Files         564      564              
  Lines       54159    54178      +19     
  Branches     8414     8418       +4     
==========================================
+ Hits        44792    44805      +13     
- Misses       7294     7297       +3     
- Partials     2073     2076       +3
Flag Coverage Δ
keras 82.50% <68.42%> (-0.01%) ⬇️
keras-jax 64.04% <0.00%> (-0.03%) ⬇️
keras-numpy 59.06% <0.00%> (-0.03%) ⬇️
keras-openvino 32.93% <68.42%> (+0.01%) ⬆️
keras-tensorflow 64.32% <0.00%> (-0.03%) ⬇️
keras-torch 64.08% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PraroopChanda PraroopChanda mentioned this pull request Apr 4, 2025
Open
@gbaned gbaned requested a review from mattdangerw April 8, 2025 10:21
@gbaned gbaned added this to PR Queue Apr 8, 2025
@github-project-automation github-project-automation bot moved this to Assigned Reviewer in PR Queue Apr 8, 2025
@gbaned
Copy link
Collaborator

gbaned commented Apr 8, 2025

Hi @PraroopChanda Can you please resolve the conflicts? Thank you!

@PraroopChanda
Copy link
Author

PraroopChanda commented Apr 8, 2025 •
edited
Loading

Hi @PraroopChanda Can you please resolve the conflicts? Thank you!

Done.

@rkazants
Copy link
Contributor

rkazants commented Apr 8, 2025

DO NOT merge! Need review.

rkazants
rkazants reviewed Apr 8, 2025
View reviewed changes
keras/src/backend/openvino/excluded_concrete_tests.txt
@@ -88,6 +87,7 @@ NumpyOneInputOpsCorrectnessTest::test_cumprod
NumpyOneInputOpsCorrectnessTest::test_diag
NumpyOneInputOpsCorrectnessTest::test_diagonal
NumpyOneInputOpsCorrectnessTest::test_exp2
NumpyOneInputOpsCorrectnessTest::test_expm1
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert this change that is not related to PR

rkazants
rkazants reviewed Apr 8, 2025
View reviewed changes
keras/src/backend/openvino/excluded_concrete_tests.txt
@@ -88,6 +87,7 @@ NumpyOneInputOpsCorrectnessTest::test_cumprod
NumpyOneInputOpsCorrectnessTest::test_diag
NumpyOneInputOpsCorrectnessTest::test_diagonal
NumpyOneInputOpsCorrectnessTest::test_exp2
NumpyOneInputOpsCorrectnessTest::test_expm1
NumpyOneInputOpsCorrectnessTest::test_flip
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove the line with NumpyOneInputOpsCorrectnessTest::test_flip

rkazants
rkazants reviewed Apr 8, 2025
View reviewed changes
keras/src/backend/openvino/numpy.py
Comment on lines +743 to +746
reversed_indices = ov_opset.range(start, stop, step, Type.i64).output(0)
x = ov_opset.gather(
x, reversed_indices, ov_opset.constant(ax, Type.i64).output(0)
).output(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks not optimal. We have special op for reversing a tensor along required axis. Check
https://docs.openvino.ai/2025/api/ie_python_api/_autosummary/openvino.runtime.opset1.reverse_sequence.html#openvino.runtime.opset1.reverse_sequence

Copy link
Author

Choose a reason for hiding this comment

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

Hi @rkazants ,
I have a few prior deadlines this week, so I might need a bit of time to incorporate the requested changes.
I’ll get to it as soon as possible.

Thank you for your patience!

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

@rkazants rkazants rkazants left review comments

@mattdangerw mattdangerw Awaiting requested review from mattdangerw

Assignees

@gbaned gbaned

Labels
size:S
Projects
PR Queue
Status: Assigned Reviewer
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Good First Issue][Keras 3 OpenVINO Backend]: Support numpy.flip operation
4 participants
@PraroopChanda @codecov-commenter @gbaned @rkazants