New iterator API (nditer): Overlap detection in NumPy
Hi all, Pauli just opened a nice pull request [1] to add overlap detection to the new iterator, this means adding a new iterator flag: `NPY_ITER_COPY_IF_OVERLAP` If passed to the iterator (also exposed in python), the iterator will copy the operands such that reading and writing should only occur for identical operands. For now this is implemented by always copying the output/writable operand (this could be improved though, so I would not say its fixed API). Since adding this flag is new API, please feel free to suggest other names/approaches or even decline the change ;). This is basically a first step, which should be easily followed by adding overlap detection to ufuncs, removing traps such as the well (or not so well known) `a += a.T`. Other parts of numpy may follow one by one. The work is based on his older awesome new memory overlap detection implementation. If there are no comments, I will probably merge it very soon, so we can look at the follow up things. - Sebastian [1] https://github.com/numpy/numpy/pull/8026
On Sep 7, 2016 9:03 AM, "Sebastian Berg" <sebastian@sipsolutions.net> wrote:
I wonder if there is any way we can avoid the flag, and just make this happen automatically when appropriate? nditer has too many "unbreak-me" flags already. Are there any cases where we *don't* want the copy-if-overlap behavior? Traditionally overlap has triggered undefined behavior, so there's no backcompat issue, right? -n
On Mi, 2016-09-07 at 09:22 -0700, Nathaniel Smith wrote:
Puh, I remember weird abuses, that sometimes stopped working. Even just adding it to ufuncs might destroy some weird cases in someones script.... Whether or not we can just make it default, might be worth thinking about it. What do downstream projects that use the API think? My guess is that would be projects such as numexpr, numba, or I think theano? Maybe another approach is to think about some other way to make good defaults to the iterator easier/saner. Heck, I wonder if we would default to things like "zero size ok" and warned about it, anyone would notice unless as in: Oh I should make it zero size ok ;). - Sebastian
Wed, 07 Sep 2016 09:22:24 -0700, Nathaniel Smith kirjoitti: [clip]
I didn't put it on by default, because of backward compatibility and side effects that break things. On side effects: there are some bugs in ufunc code that need fixing if the flag is turned on (wheremask code breaks, and ufuncs write to wrong output arrays). Moreover, copying write operands with updateifcopy marks the original arrays as read-only, until the copied array is decrefed. There may also be other side effects that are not so obvious. The PR is not mergeable if the flag would be on by default --- that requires inspecting all the uses of the iterator in the numpy codebase and making sure there's no weird stuff done. I'm not sure how much 3rd party code is using the iterator, but I'm a bit worried also that copies break assumptions also there. It might be possible to turn it on by default for operands with COPY or UPDATEIFCOPY flags --- but I'm not sure if that's helpful (now you'd need to set the flags to all input operands). -- Pauli Virtanen
Hi, In the end some further API additions turn out to appear needed: * NPY_ITER_COPY_IF_OVERLAP, NPY_ITER_OVERLAP_NOT_SAME flags for NpyIter_New. * New API function PyArray_MapIterArrayCopyIfOverlap, as ufunc.at needs to check overlaps for index arrays before constructing iterators, and the parsing is done in multiarray. Continuation here: https://github.com/numpy/numpy/pull/8043 Wed, 07 Sep 2016 18:02:59 +0200, Sebastian Berg kirjoitti: pull/8026_______________________________________________
NumPy-Discussion mailing list NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
On So, 2016-09-11 at 21:21 +0000, Pauli Virtanen wrote:
Hi,
In the end some further API additions turn out to appear needed:
Very nice :).
I think here Nathaniels point might be right. It could be we can assume that copying is always fine, there is probably only one or two downstream projects using this function, plus it seems harder to create abusing structures that actually do something useful. It was only exposed for usage in `ufunc.at` if I remember right. I know theano uses it though, but not sure about anyone else, maybe numba. On the other hand.... It is not the worst API clutter in history.
Mon, 12 Sep 2016 11:31:07 +0200, Sebastian Berg kirjoitti:
Do you suggest that I break the PyArray_MapIterArray API? One issue here is that the function doesn't make distinction between read- only access and read-write access, so copying may give unnecessary slowdown. The second thing is that it will result to a bit uglier code, as I need to manage the overlap with the second operation in ufunc_at. For NpyIter, I'd still be wary about copying by default, because it's not needed everywhere (the may_share_memory checks are better done earlier), and since the semantic change can break things inside Numpy. Pauli
On Mo, 2016-09-12 at 20:22 +0000, Pauli Virtanen wrote:
Yeah, was only wondering about MapIterArray, because I might get away with the API break in the case that it works everywhere for our internal usage. But if its not quite straight forward, there is no point in thinking about it.
Yes, I tend to agree here about it. You can always wonder whether its still the most convenient place to do the checks (at least for a few places), but from glancing at the code, it still seems elegant to me. If we are concerned about making the iterator more and more complex, maybe we can really do something else about it as well. I am not sure whether I will manage to look at it very closely soon, so would invite anyone to take a look; this is definitely a highlight! - Sebastian
On Sep 7, 2016 9:03 AM, "Sebastian Berg" <sebastian@sipsolutions.net> wrote:
I wonder if there is any way we can avoid the flag, and just make this happen automatically when appropriate? nditer has too many "unbreak-me" flags already. Are there any cases where we *don't* want the copy-if-overlap behavior? Traditionally overlap has triggered undefined behavior, so there's no backcompat issue, right? -n
On Mi, 2016-09-07 at 09:22 -0700, Nathaniel Smith wrote:
Puh, I remember weird abuses, that sometimes stopped working. Even just adding it to ufuncs might destroy some weird cases in someones script.... Whether or not we can just make it default, might be worth thinking about it. What do downstream projects that use the API think? My guess is that would be projects such as numexpr, numba, or I think theano? Maybe another approach is to think about some other way to make good defaults to the iterator easier/saner. Heck, I wonder if we would default to things like "zero size ok" and warned about it, anyone would notice unless as in: Oh I should make it zero size ok ;). - Sebastian
Wed, 07 Sep 2016 09:22:24 -0700, Nathaniel Smith kirjoitti: [clip]
I didn't put it on by default, because of backward compatibility and side effects that break things. On side effects: there are some bugs in ufunc code that need fixing if the flag is turned on (wheremask code breaks, and ufuncs write to wrong output arrays). Moreover, copying write operands with updateifcopy marks the original arrays as read-only, until the copied array is decrefed. There may also be other side effects that are not so obvious. The PR is not mergeable if the flag would be on by default --- that requires inspecting all the uses of the iterator in the numpy codebase and making sure there's no weird stuff done. I'm not sure how much 3rd party code is using the iterator, but I'm a bit worried also that copies break assumptions also there. It might be possible to turn it on by default for operands with COPY or UPDATEIFCOPY flags --- but I'm not sure if that's helpful (now you'd need to set the flags to all input operands). -- Pauli Virtanen
Hi, In the end some further API additions turn out to appear needed: * NPY_ITER_COPY_IF_OVERLAP, NPY_ITER_OVERLAP_NOT_SAME flags for NpyIter_New. * New API function PyArray_MapIterArrayCopyIfOverlap, as ufunc.at needs to check overlaps for index arrays before constructing iterators, and the parsing is done in multiarray. Continuation here: https://github.com/numpy/numpy/pull/8043 Wed, 07 Sep 2016 18:02:59 +0200, Sebastian Berg kirjoitti: pull/8026_______________________________________________
NumPy-Discussion mailing list NumPy-Discussion@scipy.org https://mail.scipy.org/mailman/listinfo/numpy-discussion
On So, 2016-09-11 at 21:21 +0000, Pauli Virtanen wrote:
Hi,
In the end some further API additions turn out to appear needed:
Very nice :).
I think here Nathaniels point might be right. It could be we can assume that copying is always fine, there is probably only one or two downstream projects using this function, plus it seems harder to create abusing structures that actually do something useful. It was only exposed for usage in `ufunc.at` if I remember right. I know theano uses it though, but not sure about anyone else, maybe numba. On the other hand.... It is not the worst API clutter in history.
Mon, 12 Sep 2016 11:31:07 +0200, Sebastian Berg kirjoitti:
Do you suggest that I break the PyArray_MapIterArray API? One issue here is that the function doesn't make distinction between read- only access and read-write access, so copying may give unnecessary slowdown. The second thing is that it will result to a bit uglier code, as I need to manage the overlap with the second operation in ufunc_at. For NpyIter, I'd still be wary about copying by default, because it's not needed everywhere (the may_share_memory checks are better done earlier), and since the semantic change can break things inside Numpy. Pauli
On Mo, 2016-09-12 at 20:22 +0000, Pauli Virtanen wrote:
Yeah, was only wondering about MapIterArray, because I might get away with the API break in the case that it works everywhere for our internal usage. But if its not quite straight forward, there is no point in thinking about it.
Yes, I tend to agree here about it. You can always wonder whether its still the most convenient place to do the checks (at least for a few places), but from glancing at the code, it still seems elegant to me. If we are concerned about making the iterator more and more complex, maybe we can really do something else about it as well. I am not sure whether I will manage to look at it very closely soon, so would invite anyone to take a look; this is definitely a highlight! - Sebastian
participants (3)
-
Nathaniel Smith -
Pauli Virtanen -
Sebastian Berg