Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

NEP: Array function protocol#11189

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mattip merged 10 commits intonumpy:masterfrommrocklin:nep-array-function-protocol
Jun 1, 2018

Conversation

mrocklin
Copy link
Contributor

@mrocklinmrocklin commentedMay 29, 2018
edited by shoyer
Loading

Written with Stephan Hoyer
after discussion with Stefan Van der Walt, Charles Harris, Matti Picus, Jaime Frio and Nathaniel Smith

Written with Stephan Hoyer after discussion with Stefan Van der Walt,Charles Harris, Matti Picus, and Jaime Frio
@mrocklinmrocklin changed the titleAdd first draft of NEP 0016: array function protocolNEP: Array function protocolMay 29, 2018
Copy link
Member

@shoyershoyer left a comment
edited
Loading

Choose a reason for hiding this comment

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

Can we save NEP-16 for Nathaniel'sAn abstract base class for identifying "duck arrays" in#10706 ? That would make this NEP-17.

@mrocklin
Copy link
ContributorAuthor

mrocklin commentedMay 29, 2018 via email

Thanks. Done
On Tue, May 29, 2018 at 11:50 AM, Stephan Hoyer ***@***.***> wrote: ***@***.**** commented on this pull request. Can we save NEP-16 for Nathaniel's An abstract base class for identifying "duck arrays" in#10706 <#10706> ? ------------------------------ In doc/neps/nep-0016-array-function-protocol.rst <#11189 (comment)>: > + class MyArray: + def __array_function__(self, func, overload_types, *args, **kwargs): + if func not in HANDLED_FUNCTIONS: + return NotImplemented + if not all(issubclass(t, MyArray) for t in overload_types): + return NotImplemented + return HANDLED_FUNCTIONS[func](*args, **kwargs) + + HANDLED_FUNCTIONS = { + np.concatenate: my_concatenate, + np.broadcast_to: my_broadcast_to, + np.sum: my_sum, + ... + } + +*Note from MR: would it make sense instead for them to return the delete? :) ------------------------------ In doc/neps/nep-0016-array-function-protocol.rst <#11189 (comment)>: > @@ -0,0 +1,517 @@ +================================================== +NEP: Dispatch Mechanism for NumPy's high level API +================================================== + +:Author: Authors: Stephan Hoyer ***@***.***> and Matthew Rocklin ***@***.***> I think the standard is one line/:Author: entry per author — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11189 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszD_VkAgClzxAFP3kYRjvyF24dbRfks5t3W5FgaJpZM4URsKT> .


.. code-block:: python

def __array_function__(self, func, types, *args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

What if{'func', 'types'} & kwargs.keys() is non-empty?

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, I think we need to switch this to not use* and **` unpacking.

@mattip
Copy link
Member

This should be merged as nep-0018 once the initial comments are handled. Deeper discussion, as I understand the process, should take place on the mailing list

.. code:: python

def broadcast_to(array, shape, subok=False):
success, value = do_array_function_dance(
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to handle this as a decorator? Perhaps we could use

ba = inspect.signature(func).bind(*args, **kwargs)ba.apply_defaults()array_function = ...array_function(func, *ba.args, **bar.kwargs)

hameerabbasi reacted with thumbs up emoji
Copy link
Member

Choose a reason for hiding this comment

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

Yes, if we only supported Python 3. But in Python 2, we would lose introspection of function arguments.

Nonetheless this should indeed be mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

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

This example really does not look nice!! If this cannot be done more elegantly in python2, then I think we should aim for python3 only - this process is unlikely to be that fast and even if it were, skipping one release would be well worth having a much simpler interface. In particular, in python3 one could use a decorator that looks at annotations (which we'd like anyway!) - much more elegant.

Copy link
Contributor

@mhvkmhvk left a comment

Choose a reason for hiding this comment

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

The nature of types is really rather unclear. Should it perhaps be a list of types thendarray implementation does not know what to do with? (This would includendarray subclasses if the current function would doasarray)? More comments on mailing list.

2. Are all arguments of a type that we know how to handle?

If these conditions hold, ``__array_function__`` should return
implementation for ``func(*args, **kwargs)``. Otherwise, it should
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the example below I think you mean "the result from calling its implementation of ...".

``__array_function__`` attribute on those inputs, and call those
methods appropriately until one succeeds.

This is one additional function of moderate complexity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe comment here that speed will be of the essence.

Copy link
Contributor

Choose a reason for hiding this comment

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

In particular, the speed forregular arrays should not be impacted much even for small arrays.

.. code:: python

def broadcast_to(array, shape, subok=False):
success, value = do_array_function_dance(
Copy link
Contributor

Choose a reason for hiding this comment

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

This example really does not look nice!! If this cannot be done more elegantly in python2, then I think we should aim for python3 only - this process is unlikely to be that fast and even if it were, skipping one release would be well worth having a much simpler interface. In particular, in python3 one could use a decorator that looks at annotations (which we'd like anyway!) - much more elegant.

(Autograd, Tangent), higher order array factorizations (TensorLy), etc.
that add additional functionality on top of the Numpy API.

We would like to be able to use these libraries together, for example we
Copy link
Contributor

Choose a reason for hiding this comment

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

Add:

Finally, some ``ndarray`` subclasses add new behaviour (e.g., MaskedArray and astropy's Quantity),which gives different meaning to functions. Indeed, MaskedArray reimplements some of the corenumpy functions, and Quantity has long-standing issues about compatibility with stacking functions.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I'm not sure how best to weave this text into the NEP. To me this seems like orthogonal functionality.

Copy link
Contributor

Choose a reason for hiding this comment

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

My text may have been confusing, but I think__array_function__ is definitelyvery relevant for subclasses.

For instance, currentlyma.extras is full of re-definitions of functions likestack - i.e., it follows the make-your-own-namespace "solution" that this NEP would like to address. This could be completely replaced by the__array_function__ proposed here!

Similarly, forQuantity we are currently relying on overrides or hope that a function uses ufuncs and does not doasarray - and we have no good solution for functions likeconcatenate (not being willing to provide our own namespace...). With__array_function__, this would be solved. For instance, forconcatenate we can ensure that all inputs are converted to the same unit before doing the actual concatenation.

In other words, I probably should have written what looks much like this NEP forMaskedArray andQuantity alone - hence the request to mention them!

Aside: For both the above, I see possible implementations very much in terms of "1. prepare input arrays; 2. call original function on those; 3. set some properties on outputs" - step (2) is most logically implemented with asuper() call, which is why I strongly suggest there be anndarray.__array_function__.

@mhvk
Copy link
Contributor

I cannot find this mentioned on the mailing list, so then here:

  1. I'm rather unclear about the use oftypes. It can help me decide what to do, but I would still have to find the argument in question. If we pass anything at all, should it just be the argument itself that does not get immediately recognized byndarray? Or, better still, perhaps a tuple of all arguments that were inspected?
  2. For subclasses, it would be very handy to havendarray.__array_function__, so one can call super after changing arguments. (For__array_ufunc__, there was lots of question about whether this was useful, but it really is!!).
  3. Thisndarray.__array_function__ might also help solve the problem of cases where coercion is fine: it could have an extra keyword argument (saycoerce) that would call the function with coercion in place.
  4. Indeed, thendarray.__array_function__ could just be used inside the "dance" function, and then the actual implementation of a given function would just be a separate, private one.

@shoyer
Copy link
Member

shoyer commentedMay 30, 2018 via email

We still need to send this out to the mailing this. I'll do this thisafternoon unless Matt beats me to it.
On Wed, May 30, 2018 at 10:06 AM Marten van Kerkwijk < ***@***.***> wrote: I cannot find this mentioned on the mailing list, so then here: 1. I'm rather unclear about the use of types. It can help me decide what to do, but I would still have to find the argument in question. If we pass anything at all, should it just be the argument itself that does not get immediately recognized by ndarray? Or, better still, perhaps a tuple of all arguments that were inspected? 2. For subclasses, it would be very handy to have ndarray.__array_function__, so one can call super after changing arguments. (For __array_ufunc__, there was lots of question about whether this was useful, but it really is!!). 3. This ndarray.__array_function__ might also help solve the problem of cases where coercion is fine: it could have an extra keyword argument (say coerce) that would call the function with coercion in place. 4. Indeed, the ndarray.__array_function__ could just be used inside the "dance" function, and then the actual implementation of a given function would just be a separate, private one. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11189 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABKS1vVuNaGfNEdEe0nBiLiXpdFfnLyfks5t3tGsgaJpZM4URsKT> .

@mattip
Copy link
Member

The authors should send it out to the mailing list.

Still needed for merge (as perNep Workflow, the merge should be sooner rather than later):

  • rebase against master to remove the failing test
  • rename the file todoc/neps/nep-0018-array-function-protocol.rst

@mhvk
Copy link
Contributor

Another general comment, since this is still not on the mailing list, yet I'm thinking about it now: Since speed fornormal operation should be impacted as minimally as possible, there should be obvious ways to ensure no type checking dance is done. Some possible solutions (which I think should be in the NEP, even if as discounted options):

  • Two namespaces, one for the undecorated base functions, and one for the decorated ones. The idea would be that if one knows one is dealing with arrays only, one would doimport numpy.array_only as np. The latter namespace would be the one automatically used forndarray.__array_function__.
  • Decorator automatic insertion of aarray_only=np._NoValue (orcoerce and perhapssubok=... if not present) in the function signature, so that users who know that they have arrays only could passarray_only=True (name to be decided). This would be most useful if there were also some type of configuration parameter that could set the default ofarray_only.

@mrocklin
Copy link
ContributorAuthor

rebase against master to remove the failing test
rename the file to doc/neps/nep-0018-array-function-protocol.rst

Done! My apologies for the delays on this@mattip .

@mattipmattip merged commit1c50f24 intonumpy:masterJun 1, 2018
@mattip
Copy link
Member

Thanks@mrocklin

@shoyer
Copy link
Member

I've now posted the NEP to the mailing list for discussion
https://mail.python.org/pipermail/numpy-discussion/2018-June/078127.html

@mhvk with regards tondarray.__array_function__ in particular, I don't have strong feelings here and am happy to differ to your judgment (you know I don't like subclassing). Certainly there is virtue in using the same approach as__array_ufunc__. I'll add a brief note on this in the next revision.

With regards to namespaces, please note the section on "Separate namespace" that is already in the NEP text.

Copy link
Contributor

@mhvkmhvk left a comment

Choose a reason for hiding this comment

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

Two smaller comments; more general ones on the mailing list.

``__array_function__``.

This protocol is intended to be a catch-all for NumPy functionality that
is not covered by existing protocols, like reductions (like ``np.sum``)
Copy link
Contributor

Choose a reason for hiding this comment

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

This particular reduction is an example of aufunc - maybe stick to the more complicated example ofnp.mean?

would only include these as keyword arguments when they have changed
from default values. This is similar to `what NumPy already has
done <https://github.com/numpy/numpy/blob/v1.14.2/numpy/core/fromnumeric.py#L1865-L1867>`__,
e.g., for the optional ``keepdims`` argument in ``sum``:
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth mentioning that the dance decorator could do this part as well!

.. code:: python

def broadcast_to(array, shape, subok=False):
success, value = do_array_function_dance(
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a mini-thing, but one might as well have the dance return either a result orNotImplemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

I’d be in favor of this. No need to complicate things with multiple out args, remember the order and so on.

@mattip
Copy link
Member

Are we ready to start the approval process for NEP 18?

@shoyer
Copy link
Member

@mattip almost -- I'd like to add short paragraphs on class methods and dtypes to "Alternatives" to make it clear that we aren't aren't ruling out such options in the future.

@okuta
Copy link
Contributor

Hello, I am aCuPy developer.
This feature is important for NumPy compatible library.
I create a PR for CuPy in experimentalcupy/cupy#1650 . Please use it if you want.
Thanks.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@eric-wiesereric-wiesereric-wieser left review comments

@mattipmattipmattip left review comments

@shoyershoyershoyer left review comments

@hameerabbasihameerabbasihameerabbasi left review comments

@mhvkmhvkmhvk left review comments

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@mrocklin@mattip@mhvk@shoyer@okuta@eric-wieser@hameerabbasi

[8]ページ先頭

©2009-2025 Movatter.jp