Skip to content

Add numpy.logaddexp to openvino backend #21043

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
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add numpy.logaddexp to openvino backend #21043

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd4bd4f
Add numpy.logaddexp to openvino backend
DeepC004 Mar 15, 2025
789947c
Adapt to compute stable expression calculation
DeepC004 Mar 17, 2025
ecf9645
Fix linting issue
DeepC004 Mar 17, 2025
271fb9a
Run format.sh to fix formatting error
DeepC004 Mar 17, 2025
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
20 changes: 17 additions & 3 deletions keras/src/backend/openvino/numpy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,9 +848,23 @@ def log2(x):

Copy link
Contributor

Choose a reason for hiding this comment

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

please switch on tests for this functionality.

Copy link
Author

Choose a reason for hiding this comment

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

In issue you mentioned ./keras/src/ops/numpy_test.py for testing. I initially thought that tests automatically invoke the numpy from correct backend. But on tracking knp imports in numpy_test file they seem to come from ./keras/src/backend/numpy module instead of openvino. I verify this by executing tests without implementing logaddexp() and they PASSED (a number of other unimplemented functions inside ./keras/src/backend/openvino/numpy.py also pass). What should I change exactly to test the function in current test setup?

Copy link
Contributor

Choose a reason for hiding this comment

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

please rebase to latest master and you will see corresponding lines for logaddexp


def logaddexp(x1, x2):
raise NotImplementedError(
"`logaddexp` is not supported with openvino backend"
)
element_type = None
if isinstance(x1, OpenVINOKerasTensor):
element_type = x1.output.get_element_type()
if isinstance(x2, OpenVINOKerasTensor):
element_type = x2.output.get_element_type()
x1 = get_ov_output(x1, element_type)
x2 = get_ov_output(x2, element_type)
x1, x2 = _align_operand_types(x1, x2, "logaddexp()")
x_type = x1.get_element_type()
if x_type.is_integral():
ov_type = OPENVINO_DTYPES[config.floatx()]
x1 = ov_opset.convert(x1, ov_type)
x2 = ov_opset.convert(x2, ov_type)
exp_x1 = ov_opset.exp(x1).output(0)
exp_x2 = ov_opset.exp(x2).output(0)
sum_exp = ov_opset.add(exp_x1, exp_x2).output(0)
Copy link
Contributor

@rkazants rkazants Mar 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I propose to make it more numerical stable for big x1 and x2 numbers:

ln(e^x1 + e^x2) = ln (e^k(e^(x1-k) + e^(x2-k))) = k ln (e^(x1-k) + e^(x2-k)), where k is maximum from x1 and x2. pay attention that by this trick x1-k and x2-k will be less than one that provides more compute stable for exponent that outputs non-negative value less than one that is better represented for floating point.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch on compute stability. I have made the changes

return OpenVINOKerasTensor(ov_opset.log(sum_exp).output(0))


def logical_and(x1, x2):
Expand Down