Added openvino backend support for numpy.median (Issue #30115) #21379
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21379 +/- ##
==========================================
- Coverage 82.67% 77.19% -5.48%
==========================================
Files 565 565
Lines 55064 55559 +495
Branches 8569 8670 +101
==========================================
- Hits 45525 42891 -2634
- Misses 7441 10605 +3164
+ Partials 2098 2063 -35
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:
|
alungriffith
force-pushed
the
median_support_ov
branch
from
43ac109 to
d92b60a
Compare
|
@rkazants PR is ready for review. Apologies for all the commits, it took me a second to fix my local testing to run on openvino backend on macOS. |
alungriffith
changed the title
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for numpy.median to the OpenVINO backend. The implementation handles scenarios for the axis and keepdims parameters. A critical bug was identified in the logic for handling multiple reduction axes, making that code path unreachable. Additionally, high-severity issues were found where the code relies on static shape information, leading to failures with dynamic input shapes. Suggestions have been provided to address these points and improve robustness.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The code changes introduce OpenVINO backend support for numpy.median. There is a high severity issue in the logic for flattening axes within the median function that needs to be addressed, as well as a medium-severity suggestion to improve the code's correctness.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for numpy.median for the OpenVINO backend. The implementation is comprehensive, handling various cases for axis and keepdims. I've made a few suggestions to improve maintainability by adding comments to complex parts of the logic, and fixed a correctness issue in the ndim function.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request adds OpenVINO backend support for the numpy.median function. The implementation involves a complex series of OpenVINO operations to handle different input scenarios and calculate the median. I've suggested adding more comments to improve the code's readability and maintainability, especially around the tensor manipulations and specific OpenVINO operations used.
|
@mattdangerw PR is ready for review. Gemini's raised concerns have been addressed, I believe it might be going over old ground with the latest suggestions but I'm happy to add more comments if needed. The torch test is failing in today's commits but I don't think this is due to my median function updates. |
pytest.ini
Outdated
| [pytest] | ||
| env = | ||
| KERAS_BACKEND=openvino |
| if np.isscalar(x): | ||
| x = get_ov_output(x) | ||
| return OpenVINOKerasTensor(x) | ||
|
|
There was a problem hiding this comment.
please add comments to explain the logic of the algorithm you implemented
There was a problem hiding this comment.
Comments added and some variables renamed to better explain the logic.
|
@rkazants updated with comments and better clarity, ready for review. |
Added decomposition for numpy.median using OpenVINO backend.