-
Notifications
You must be signed in to change notification settings - Fork 19.6k
[OpenVINO backend] fix numpy conversions #21498
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.
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
[OpenVINO backend] fix numpy conversions #21498
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @Mohamed-Ashraf273, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a critical compatibility issue with the OpenVINO backend where direct conversion of Keras tensors to NumPy arrays was not functioning as expected, unlike other Keras backends. My changes enable seamless NumPy conversion for concrete tensors and provide clearer error messages for symbolic tensors, while also improving the internal data type handling for casting operations.
Highlights
- NumPy Conversion for OpenVINO Tensors: I've added
__array__
andnumpy()
methods to theOpenVINOKerasTensor
class. This allows concrete OpenVINO tensors to be directly converted to NumPy arrays usingnp.array(tensor)
ortensor.numpy()
, aligning its behavior with other Keras backends. Symbolic tensors will now raise a clearRuntimeError
explaining that they cannot be converted to NumPy. - Improved Error Handling: The
convert_to_numpy
utility function now catches general exceptions during the conversion process and re-raises a more informativeRuntimeError
, making debugging easier when a tensor fails to convert. - Standardized Data Type Casting: I've introduced a call to
standardize_dtype(dtype)
within thecast
function. This ensures that the inputdtype
is consistently processed before being mapped to an OpenVINO type, improving robustness.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses NumPy conversion issues for the OpenVINO backend. The main change is the implementation of __array__
and numpy
methods for OpenVINOKerasTensor
, which now correctly handle dtype
conversions. The error handling for converting symbolic tensors to NumPy arrays is also significantly improved, with better exception handling and more informative, user-facing error messages. The changes are well-implemented and enhance both the correctness and usability of the OpenVINO backend.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21498 +/- ##
==========================================
- Coverage 82.85% 82.84% -0.01%
==========================================
Files 565 565
Lines 55694 55703 +9
Branches 8689 8689
==========================================
+ Hits 46144 46147 +3
- Misses 7434 7440 +6
Partials 2116 2116
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@rkazants SymbolicTensor:https://github.com/tensorflow/tensorflow/blob/v2.16.1/tensorflow/python/framework/tensor.py#L625-L630 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fchollet, looks good to me. Recommend to merge.
@rkazants
@fchollet
I've noticed that this test (and several similar ones) pass on all backends except OpenVINO:
https://github.com/keras-team/keras-hub/blob/master/keras_hub/src/layers/preprocessing/masked_lm_mask_generator_test.py#L46-L59
After investigating, I found that the following minimal example works correctly on
TensorFlow
,JAX
, andTorch
backends, but fails withOpenVINO
:To confirm the fix, I also created a reproducer that explicitly simulates the failure when an
OpenVINOKerasTensor
is used as a symbolic placeholder and.numpy()
conversion is attempted: