[OpenVINO] Fix ndim() to return Python int instead of graph tensor#22608
[OpenVINO] Fix ndim() to return Python int instead of graph tensor#22608hertschuh merged 3 commits intokeras-team:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes a test from the OpenVINO excluded tests list and refactors the ndim function in the OpenVINO backend to use get_partial_shape().rank.get_length(). While the refactor simplifies the code, the current implementation is fragile because get_length() will raise a RuntimeError if the tensor has a dynamic rank. It is recommended to handle dynamic ranks by checking is_static or returning None to ensure robustness.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the ndim implementation in the OpenVINO backend to use the tensor's partial shape rank and re-enables the test_normalization_kpl test. The review feedback suggests optimizing the ndim function by checking for the attribute directly on the input object to avoid unnecessary backend operations and recommends clarifying the error message regarding static rank requirements.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #22608 +/- ##
=======================================
Coverage 83.28% 83.29%
=======================================
Files 596 596
Lines 68102 68139 +37
Branches 10610 10614 +4
=======================================
+ Hits 56720 56753 +33
- Misses 8636 8639 +3
- Partials 2746 2747 +1
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:
|
ndim()in the OpenVINO backend was building an OV graph node to represent the rank, returning an OpenVINOKerasTensor. However, the number of dimensions is a static, compile-time property of a tensor's type, not a runtime value. It never changes during inference and must be a Python int for use in Python control flow.All other backends return an int.
This PR adds a fix which reads the rank directly from the OV partial shape,
get_partial_shape().rank.get_length()which is always known at graph-build time.