-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix: Defer axis validation in reduce operations to compile/runtime #33450
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
base: master
Are you sure you want to change the base?
Fix: Defer axis validation in reduce operations to compile/runtime #33450
Conversation
- Modify reduce_shape_infer() to check axis validity before normalizing - When invalid axes are detected, return dynamic output shape instead of throwing - Validation still occurs at runtime via try_get_normalized_axis_set() in evaluate() - Affects all reduce operations: ReduceMean, ReduceMax, ReduceMin, ReduceProd, etc. - Update tests to reflect new behavior where invalid axes result in dynamic shapes Fixes openvinotoolkit#33261
|
|
||
| if (data_rank.is_static() && axes_val) { | ||
| ov::util::try_normalize_axes(*axes_val, data_rank, *op); | ||
| // Check if all axes are valid before normalizing - if not, defer validation to runtime |
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.
This is not correct.
In OpenVINO if output shape cannot be calculated correctly because of invalid inputs the exception must be thrown.
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.
"Understood. I attempted to defer the validation to improve the user experience as requested in the issue, but I see now that core shape inference requires strict validation when inputs are known. I will revert this change.
I will instead update the PR to add a documentation warning (docstring) and a unit test to formalize this 'fail-fast' behavior, which addresses the original issue's request for better clarity."
Per reviewer feedback - OpenVINO requires immediate validation when output shape cannot be calculated correctly due to invalid inputs. Reverted changes to reduce_shape_inference.hpp and reduce_ops.hpp to restore original behavior where invalid axes throw immediately.
|
@praasz I have reverted the changes to reduce_shape_inference.hpp. Instead, I am converting this PR to a Documentation & Test fix: |
- Add docstring documentation to Python reduce operations (reduce_mean, reduce_max, reduce_min, reduce_sum, reduce_prod, reduce_logical_and, reduce_logical_or) explaining: - Valid axis range: [-rank, rank-1] - Validation occurs at node creation time when axes are constant - RuntimeError is raised immediately for out-of-range axes - Add Python unit tests documenting the expected 'fail-fast' behavior: - test_reduce_invalid_axis_raises_immediately - test_reduce_logical_invalid_axis_raises_immediately This addresses issue openvinotoolkit#33261 by improving documentation clarity about when axis validation occurs, as requested by the issue author. Fixes openvinotoolkit#33261
The unit test can be added to show there is exception on invalid inputs. Docstring update is not required as this is default action for OV nodes. |
Summary
This PR addresses issue #33261 by deferring axis validation for reduce operations from node creation time to compile/runtime, improving the user experience when building models with the Python opset API.
Problem
Previously, when creating reduce operations (ReduceMean, ReduceMax, ReduceMin, etc.) with out-of-range axes via the Python opset API, validation would fail immediately during node construction with a
RuntimeError. This was surprising behavior since users typically expect validation to occur atcore.compile_model()time, not during model building.Before (problematic behavior):