Skip to content

[OpenVINO backend] Support for nanarmax and nanar min #22383

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

Open
andersendsa wants to merge 2 commits into
base: master
Choose a base branch
from
+126 −8
Open

[OpenVINO backend] Support for nanarmax and nanar min #22383

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e43bc0e
Implement nanar min and max in OpenVINO Backend
andersendsa Mar 8, 2026
8988755
format fix
andersendsa Mar 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions keras/src/backend/openvino/excluded_concrete_tests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,16 +490,12 @@ NNOpsDynamicShapeTest::test_glu
NumpyDtypeTest::test_array
NumpyDtypeTest::test_maximum_python_types
NumpyDtypeTest::test_minimum_python_types
NumpyDtypeTest::test_nanargmax
NumpyDtypeTest::test_nanargmin
NumpyDtypeTest::test_power
NumpyDtypeTest::test_view
NumpyOneInputOpsCorrectnessTest::test_array
NumpyOneInputOpsCorrectnessTest::test_conj
NumpyOneInputOpsCorrectnessTest::test_imag
NumpyOneInputOpsCorrectnessTest::test_isreal
NumpyOneInputOpsCorrectnessTest::test_nanargmax
NumpyOneInputOpsCorrectnessTest::test_nanargmin
NumpyOneInputOpsCorrectnessTest::test_real
NumpyOneInputOpsCorrectnessTest::test_reshape
NumpyOneInputOpsCorrectnessTest::test_slogdet
Expand Down
114 changes: 108 additions & 6 deletions keras/src/backend/openvino/numpy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,18 @@ def amin(x, axis=None, keepdims=False):


def _resolve_axis(x, axis):
if hasattr(axis, "__class__") and "Output" in axis.__class__.__name__:
axis_node = axis.get_node()
if hasattr(axis_node, "get_data"):
axis_val = axis_node.get_data()
if axis_val is not None:
if hasattr(axis_val, "ndim") and axis_val.ndim > 0 and axis_val.size > 0:
axis = int(axis_val[0])
elif hasattr(axis_val, "size") and axis_val.size == 1:
axis = int(axis_val.item())
else:
axis = int(axis_val)
Comment on lines +343 to +357
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 8, 2026

Choose a reason for hiding this comment

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

medium

This logic to resolve a static axis value from an OpenVINO tensor is duplicated in argmax (lines 537-551) and argmin (lines 584-598). To improve maintainability and reduce code redundancy, consider extracting this logic into a private helper function. This function could attempt to resolve the axis to an integer and be used in all three places.

Additionally, the type check hasattr(axis, "__class__") and "Output" in axis.__class__.__name__ is fragile. It's better to use isinstance(axis, ov.Output). The argmax and argmin functions use a redundant or condition with both checks. A single, robust isinstance check should be used in the new helper function.


Comment on lines +343 to +358
Copy link
Collaborator

@hertschuh hertschuh Mar 13, 2026

Choose a reason for hiding this comment

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

Why was this added? Tensor axes are not supported.

if axis == () or axis == []:
return x, None
if axis is None:
Expand Down Expand Up @@ -522,6 +534,21 @@ def argmax(x, axis=None, keepdims=False):
axis = 0
k = ov_opset.constant(1, Type.i32).output(0)
else:
if isinstance(axis, ov.Output) or (hasattr(axis, "__class__") and "Output" in axis.__class__.__name__):
axis_node = axis.get_node()
if hasattr(axis_node, "get_data"):
axis_val = axis_node.get_data()
if axis_val is not None:
if hasattr(axis_val, "ndim") and axis_val.ndim > 0 and axis_val.size > 0:
axis = int(axis_val[0])
elif hasattr(axis_val, "size") and axis_val.size == 1:
axis = int(axis_val.item())
else:
axis = int(axis_val)
else:
raise ValueError("axis must be static for argmax")
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 8, 2026

Choose a reason for hiding this comment

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

medium

According to the Keras API design guidelines (line 140), error messages should be contextual and actionable. This error message could be improved by providing more context, such as the type of axis received and what is expected. A similar improvement can be applied to the argmin function.

For example:

raise ValueError(
    f"For the OpenVINO backend, `argmax` requires a static `axis` value, "
    f"but received a dynamic tensor. Please provide a static integer "
    f"for the `axis` argument."
)
References
  1. Error messages should be contextual, informative, and actionable, explaining what happened, what was expected, and how the user can fix it. (link)

else:
raise ValueError("axis must be static for argmax")
Comment on lines +541 to +561
Copy link
Collaborator

@hertschuh hertschuh Mar 13, 2026

Choose a reason for hiding this comment

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

Why was this added? Tensor axes are not supported.

if axis < 0:
axis = rank + axis
k = ov_opset.constant(1, Type.i32).output(0)
Expand Down Expand Up @@ -554,6 +581,21 @@ def argmin(x, axis=None, keepdims=False):
axis = 0
k = ov_opset.constant(1, Type.i32).output(0)
else:
if isinstance(axis, ov.Output) or (hasattr(axis, "__class__") and "Output" in axis.__class__.__name__):
axis_node = axis.get_node()
if hasattr(axis_node, "get_data"):
axis_val = axis_node.get_data()
if axis_val is not None:
if hasattr(axis_val, "ndim") and axis_val.ndim > 0 and axis_val.size > 0:
axis = int(axis_val[0])
elif hasattr(axis_val, "size") and axis_val.size == 1:
axis = int(axis_val.item())
else:
axis = int(axis_val)
else:
raise ValueError("axis must be static for argmin")
else:
raise ValueError("axis must be static for argmin")
Comment on lines +594 to +614
Copy link
Collaborator

@hertschuh hertschuh Mar 13, 2026

Choose a reason for hiding this comment

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

Why was this added? Tensor axes are not supported.

if axis < 0:
axis = rank + axis
k = ov_opset.constant(1, Type.i32).output(0)
Expand Down Expand Up @@ -2848,15 +2890,75 @@ def moveaxis(x, source, destination):


def nanargmax(x, axis=None, keepdims=False):
raise NotImplementedError(
"`nanargmax` is not supported with openvino backend"
)
if isinstance(x, np.ndarray) and x.dtype == np.float64:
# conversion to f32 due to https://github.com/openvinotoolkit/openvino/issues/34138
x = x.astype(np.float32)
x = get_ov_output(x)
x_type = x.get_element_type()
if x_type == Type.f64:
# conversion to f32 due to https://github.com/openvinotoolkit/openvino/issues/34138
x = ov_opset.convert(x, Type.f32).output(0)
x_type = Type.f32

if x_type.is_integral() or x_type == Type.boolean:
return argmax(OpenVINOKerasTensor(x), axis=axis, keepdims=keepdims)

x, axis = _resolve_axis(x, axis)
if axis is None:
return OpenVINOKerasTensor(x)
Comment on lines +2923 to +2924
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 8, 2026

Choose a reason for hiding this comment

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

high

When axis is an empty tuple () or list [], _resolve_axis returns None for the axis. In this case, the function returns the input tensor x unmodified. This behavior is inconsistent with NumPy's nanargmax, which raises a TypeError for an empty tuple axis. Returning the input tensor is likely incorrect as no reduction is performed. Consider raising a TypeError to match NumPy's behavior for this edge case. The same issue exists in the nanargmin implementation.


nan_mask = ov_opset.is_nan(x)
neg_inf = ov_opset.constant(np.array(-np.inf, dtype=np.float32))
if x_type != Type.f32:
neg_inf = ov_opset.convert(neg_inf, x_type)
x_replaced = ov_opset.select(nan_mask, neg_inf, x).output(0)

result = argmax(OpenVINOKerasTensor(x_replaced), axis=axis, keepdims=keepdims)
result_ov = get_ov_output(result)

all_nan = ov_opset.reduce_logical_and(nan_mask, axis, keepdims).output(0)
nan_value = ov_opset.constant(-1, Type.i32).output(0)
if result_ov.get_element_type() != Type.i32:
nan_value = ov_opset.convert(nan_value, result_ov.get_element_type())
Comment on lines +2939 to +2940
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 8, 2026

Choose a reason for hiding this comment

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

medium

The argmax function is configured to return indices of type i32. Therefore, result_ov.get_element_type() should always be Type.i32. This conditional check appears to be redundant and can be removed for simplification. A similar redundant check exists in the nanargmin implementation (lines 2956-2957).

result_ov = ov_opset.select(all_nan, nan_value, result_ov).output(0)

return OpenVINOKerasTensor(result_ov)

def nanargmin(x, axis=None, keepdims=False):
raise NotImplementedError(
"`nanargmin` is not supported with openvino backend"
)
if isinstance(x, np.ndarray) and x.dtype == np.float64:
# conversion to f32 due to https://github.com/openvinotoolkit/openvino/issues/34138
x = x.astype(np.float32)
x = get_ov_output(x)
x_type = x.get_element_type()
if x_type == Type.f64:
# conversion to f32 due to https://github.com/openvinotoolkit/openvino/issues/34138
x = ov_opset.convert(x, Type.f32).output(0)
x_type = Type.f32

if x_type.is_integral() or x_type == Type.boolean:
return argmin(OpenVINOKerasTensor(x), axis=axis, keepdims=keepdims)

x, axis = _resolve_axis(x, axis)
if axis is None:
return OpenVINOKerasTensor(x)

nan_mask = ov_opset.is_nan(x)
pos_inf = ov_opset.constant(np.array(np.inf, dtype=np.float32))
if x_type != Type.f32:
pos_inf = ov_opset.convert(pos_inf, x_type)
x_replaced = ov_opset.select(nan_mask, pos_inf, x).output(0)

result = argmin(OpenVINOKerasTensor(x_replaced), axis=axis, keepdims=keepdims)
result_ov = get_ov_output(result)

all_nan = ov_opset.reduce_logical_and(nan_mask, axis, keepdims).output(0)
nan_value = ov_opset.constant(-1, Type.i32).output(0)
if result_ov.get_element_type() != Type.i32:
nan_value = ov_opset.convert(nan_value, result_ov.get_element_type())
result_ov = ov_opset.select(all_nan, nan_value, result_ov).output(0)

return OpenVINOKerasTensor(result_ov)



def nancumsum(x, axis=None, dtype=None):
Expand Down
Loading