MAINT: Align masked with normal ufunc loops #19259
Conversation
seberg
force-pushed
the
maint-ufunc-refactor-masked-not-duplicate
branch
from
d9d60a7
to
11ddb3f
Compare
seberg
force-pushed
the
maint-ufunc-refactor-masked-not-duplicate
branch
4 times, most recently
from
a9719ca
to
380b83f
Compare
| return -1; | ||
| } | ||
| char **dataptr = NpyIter_GetDataPtrArray(iter); | ||
| npy_intp *strides = NpyIter_GetInnerStrideArray(iter); | ||
| npy_intp *countptr = NpyIter_GetInnerLoopSizePtr(iter); | ||
| int needs_api = NpyIter_IterationNeedsAPI(iter); | ||
| needs_api |= NpyIter_IterationNeedsAPI(iter); |
There was a problem hiding this comment.
In case anyone wonders: Currently at this point needs_api is never set (passing it earlier is a well intentioned no-op). But the future can see that happening, the concept is good. (Except that NpyIter_IterationNeedsAPI is not perfect either right now.)
seberg
force-pushed
the
maint-ufunc-refactor-masked-not-duplicate
branch
from
380b83f
to
fa4a2cc
Compare
This removes the ability to specialize masked inner loops (for now) as was already noted in NEP 41 and NEP 43. The masked array is now passed in as the last argument to use the identical signature and avoid duplicating the code unnecessary. This is part of the longer process to refactor ufuncs to NEP 43 and split out, to keep the diff's shorter (or at least easier to read).
seberg
force-pushed
the
maint-ufunc-refactor-masked-not-duplicate
branch
from
fa4a2cc
to
9c6081e
Compare
|
I know I started pushing two things at once a bit right now (ufuncs and the result-type/ensure-nbo with voids thing). But @mattip and @mhvk if you have time: This PR and the ones listed in the ufunc project are actually more important, in the sense that pushing ufuncs will unblock/make other things easier to grok. |
| will be given in the unlikely event that it was customized. | ||
|
|
||
| We do not expect that any code uses this. If you do use it, | ||
| you must unset unset the selector on newer NumPy version. |
There was a problem hiding this comment.
Remove one of the duplicate "unset"
| * | ||
| * ufunc: The ufunc object. | ||
| * dtypes: An array which has been populated with dtypes, | ||
| * in most cases by the type resolution function | ||
| * for the same ufunc. | ||
| * fixed_strides: For each input/output, either the stride that |
There was a problem hiding this comment.
Interesting that this was there and not an argument...
| } while (loopsize > 0); | ||
| N -= subloopsize; | ||
|
|
||
| /* Process unmasked values */ |
There was a problem hiding this comment.
Gosh, never knew about this memchr stuff; should have used it for the reductions too -
numpy/numpy/core/src/umath/ufunc_object.c
Lines 2956 to 2959 in cbec2c8
Though if I look at source code of the glibc version, I have no idea why one would roll one's own - that is one heck of an optimized function! EDIT: though obviously we need to deal with strides...
There was a problem hiding this comment.
The followup deletes the whole chunk from the reductions to re-use this. It might make sense to re-add a broadcasted special case eventually though. Your reduce code has such a special case, the normal ufunc code does not.
There was a problem hiding this comment.
Nice! Quite happy to go for simpler first and then see whether speed can be improved for special cases.
|
Thanks @seberg |
This removes the ability to specialize masked inner loops (for now)
as was already noted in NEP 41 and NEP 43.
The masked array is now passed in as the last argument to use the
identical signature and avoid duplicating the code unnecessary.
This is part of the longer process to refactor ufuncs to NEP 43
and split out, to keep the diff's shorter (or at least easier
to read).
EDIT: In case anyone wonders about moving the place where the inner-loop is selected. This is necessary, because in the future we want to do what the masked version currently pretends: use the fixed-strides array, which is only available after setting up the iterator.
This is based of gh-19258 andwill have a conflict with gh-19257 (The masked+__array_prepare__limitation in the tests there is unnecessary, by merging the two loops the tests added there should fail and need simplification/updating).