Skip to content

Implements logaddexp and hypot #1272

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

Jump to bottom
Merged
merged 8 commits into from
Jul 24, 2023
Merged

Implements logaddexp and hypot #1272

merged 8 commits into from
Jul 24, 2023

Conversation

ndgrigorian
Copy link
Collaborator

@ndgrigorian ndgrigorian commented Jul 4, 2023 •
edited
Loading

This PR implements logaddexp and hypot.

To implement these functions, an issue with type promotion for binary functions has been addressed:

logaddexp and hypot both act like compositions of unary functions with binary functions and type promotion logic for binary functions has assumed pure binary functions, resulting in behaviors such as:

In [1]: import dpctl.tensor as dpt, numpy as np

In [2]: x = dpt.ones(1, dtype="i2")

In [3]: dpt.log(dpt.add(dpt.exp(x), dpt.exp(x)))
Out[3]: usm_ndarray([1.6931472], dtype=float32)

In [4]: np.logaddexp(dpt.asnumpy(x), dpt.asnumpy(x))
Out[4]: array([1.6931472], dtype=float32)

In [5]: dpt.logaddexp(x, x)
Out[5]: usm_ndarray([1.6931472])

where the dtype of the last array is float64.

  • Have you provided a meaningful PR description?
  • Have you added a test, reproducer or referred to an issue with a reproducer?
  • Have you tested your changes locally for CPU and GPU devices?
  • Have you made sure that new changes do not introduce compiler warnings?
  • Have you checked performance impact of proposed changes?
  • If this PR is a work in progress, are you opening the PR as a draft?

@ndgrigorian ndgrigorian force-pushed the impl-logaddexp-hypot branch from 16a1d58 to 0626792 Compare July 4, 2023 10:55
@ndgrigorian ndgrigorian marked this pull request as draft July 4, 2023 11:10
@github-actions
Copy link

github-actions bot commented Jul 4, 2023

View rendered docs @ https://intelpython.github.io/dpctl/pulls/1272/index.html

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Array API standard conformance tests for dpctl=0.14.5dev0=py310h7bf5fec_9 ran successfully.
Passed: 405
Failed: 595
Skipped: 119

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Array API standard conformance tests for dpctl=0.14.5dev0=py310h7bf5fec_9 ran successfully.
Passed: 406
Failed: 594
Skipped: 119

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev0=py310h7bf5fec_24 ran successfully.
Passed: 481
Failed: 519
Skipped: 119

@oleksandr-pavlyk
BinaryElementwiseFunc get acceptance_fn parameter, set individually f…
a173b9c 
…or tensor.divide

The _find_buf_dtype2 must stay consistent with _find_buf_dtype in that for
binary_fn that can be expressed via
    unary_fn(other_binary_fn(unary_fn(in1), unary_fn(in2))),
e.g. logaddexp(x1, x2) == log(exp(x1) + exp(x2))
the promotion of integral types to fp type must be consistence with both
evaluation.

The special-casing, necessary for proper working of dpt.divide, where
dpt.divide(dpt.asarray(1, dtype="i1"), dpt.asarray(1, dtype="u1")) should
give default fp type resulted in
 logaddexp(dpt.asarray(1, dtype="i1"), dpt.asarray(1, dtype="u1"))
returning array of different data-type than if evaluated alternatively,
since dpt.exp(dpt.asarray(1, dtype="i1")) and dpt.exp(dpt.asarray(1, dtype="u1"))
both gave float16 arrays, and the result was of float16 data type.

Now, both behaviors are fixed:

```
In [5]: dpt.log(dpt.add(dpt.exp(dpt.asarray(1, dtype="u1")), dpt.exp(dpt.asarray(1, dtype="i1"))))
Out[5]: usm_ndarray(1.693, dtype=float16)

In [6]: dpt.logaddexp(dpt.asarray(1, dtype="u1"), dpt.asarray(1, dtype="i1"))
Out[6]: usm_ndarray(1.693, dtype=float16)

In [7]: dpt.divide(dpt.asarray(1, dtype="u1"), dpt.asarray(1, dtype="i1"))
Out[7]: usm_ndarray(1., dtype=float32)
```
@oleksandr-pavlyk
Only try calling _inplace if inplace function is defined
626dd4a
@oleksandr-pavlyk
Since logaddexp is not defined for complex types, do not try complex …
22081fb 
…Python scalar
@oleksandr-pavlyk
Implement logaddexp in numerically stable way
d8437e6
@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev0=py310h7bf5fec_28 ran successfully.
Passed: 478
Failed: 522
Skipped: 119

@oleksandr-pavlyk
Added missing argument
c43caee
@oleksandr-pavlyk
Fixed computing allclose testing tolerance in test_dtype_matrix test
3788569 
When NumPy's result has coarser dtype than that of DPCTL we must use
the largest resolution from each dtype.
@coveralls
Copy link
Collaborator

coveralls commented Jul 24, 2023 •
edited
Loading

Coverage Status

coverage: 83.236% (+0.02%) from 83.218% when pulling 67c8195 on impl-logaddexp-hypot into 73a2b68 on master.

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev0=py310h7bf5fec_30 ran successfully.
Passed: 478
Failed: 522
Skipped: 119

@oleksandr-pavlyk oleksandr-pavlyk marked this pull request as ready for review July 24, 2023 12:53
@oleksandr-pavlyk
Copy link
Contributor

@ndgrigorian The issue with type promotion has already been addressed in one of my commits to this branch.

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev0=py310h7bf5fec_31 ran successfully.
Passed: 478
Failed: 522
Skipped: 119

oleksandr-pavlyk
oleksandr-pavlyk approved these changes Jul 24, 2023
View reviewed changes
Copy link
Contributor

@oleksandr-pavlyk oleksandr-pavlyk left a comment

Choose a reason for hiding this comment

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

I think this PR is ready to go in. Thank you @ndgrigorian

@ndgrigorian ndgrigorian merged commit 616c21e into master Jul 24, 2023
@github-actions
Copy link

Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞

@github-actions
Copy link

Array API standard conformance tests for dpctl=0.14.6dev0=py310h7bf5fec_26 ran successfully.
Passed: 478
Failed: 522
Skipped: 119

@ndgrigorian ndgrigorian deleted the impl-logaddexp-hypot branch August 8, 2023 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleksandr-pavlyk oleksandr-pavlyk oleksandr-pavlyk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ndgrigorian @coveralls @oleksandr-pavlyk