Skip to content

Add additional element-wise mathematical functions #36

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

Merged
merged 14 commits into from
Sep 16, 2020
Merged

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Sep 10, 2020

This PR

  • adds specifications for additional element-wise mathematical functions.
  • is derived from comparing API signatures across array libraries.
  • the APIs are used by downstream libraries with at least some frequency.
  • The addition of these APIs furthers parity with corresponding C APIs (C99) and those present in IEEE 754-2008.

Notes

  • The list of APIs is as follows:

    • expm1: present in all analyzed array libraries
    • log1p: present in all analyzed array libraries
    • log2: not present in TensorFlow (except in experimental NumPy)
    • log10: not present in TensorFlow (except in experimental NumPy)
    • pow: not present in Dask array (or at least not in its docs)
    • sign: present in all analyzed array libraries
    • square: present in all analyzed array libraries

@rgommers
Copy link
Member

square: present in all analyzed array libraries

square is a specialization of power, which is also not present yet. The latter also has a stronger justification - it corresponds to the ** operator. Can you add power as well?

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @kgryte. Adding these functions makes sense to me. Some minor textual comments.

@kgryte
Copy link
Contributor Author

kgryte commented Sep 14, 2020

I've add pow (following C naming conventions as we've done elsewhere).

@kgryte
Copy link
Contributor Author

kgryte commented Sep 14, 2020

I've also added rules for floating-point arithmetic. This PR should be good to go.

@rgommers
Copy link
Member

pow: not present in Dask array (or at least not in its docs)

It's present, you can add it to array-api-comparison.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @kgryte. LGTM, modulo the "financial application" thing

@asmeurer
Copy link
Member

Since out has been removed, then presumably abs and pow are allowed to implemented only via the built-on and __abs__ and __pow__ (unfortunately, min and max cannot).

@rgommers
Copy link
Member

Since out has been removed, then presumably abs and pow are allowed to implemented only via the built-on and __abs__ and __pow__ (unfortunately, min and max cannot).

I believe there's a statement somewhere already that all the different ways of spelling this must behave the same. If not, then we should indeed add it.

We shouldn't say how it should be implemented though, we try hard to stay away from that and only spec syntax and semantics - if they need to be 2 or 3 independent implementations with the same behavior, that's fine.

@kgryte
Copy link
Contributor Author

kgryte commented Sep 15, 2020

@rgommers Updated. Should be good to go.

@kgryte kgryte mentioned this pull request Sep 16, 2020
@rgommers
Copy link
Member

@kgryte FYI I put in a PR to Dask to add power and ~8 other missing ufuncs. That's now deployed on https://docs.dask.org/en/latest/array-api.html, so you may want to update array-api-comparison.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM, in it goes. Thanks @kgryte

@rgommers rgommers merged commit 07711a0 into master Sep 16, 2020
@rgommers rgommers deleted the elementwise-math branch September 16, 2020 08:14
@kgryte
Copy link
Contributor Author

kgryte commented Sep 17, 2020

@rgommers Thanks for the update on Dask. I'll post an issue on the array-api-comparision repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants