Hey, the MapIter API has only been made public in master right? So it is no problem at all to change at least the mapiter struct, right? I got annoyed at all those special cases that make things difficult to get an idea where to put i.e. to fix the boolean array-like stuff. So actually started rewriting it (and I already got one big function that does all index preparation -- ok it is untested but its basically there). I would guess it is not really a big problem even if it was public for longer, since you shouldn't do those direct struct access probably? But just checking. Since I got the test which mimics complex indexes in the tests, I thinks it should actually be feasible to do bigger refactoring there without having to worry too much about breaking things. Regards, Sebastian
On Mon, Apr 15, 2013 at 10:29 AM, Sebastian Berg <sebastian@sipsolutions.net
wrote:
Hey,
the MapIter API has only been made public in master right? So it is no problem at all to change at least the mapiter struct, right?
I got annoyed at all those special cases that make things difficult to get an idea where to put i.e. to fix the boolean array-like stuff. So actually started rewriting it (and I already got one big function that does all index preparation -- ok it is untested but its basically there).
I would guess it is not really a big problem even if it was public for longer, since you shouldn't do those direct struct access probably? But just checking.
Since I got the test which mimics complex indexes in the tests, I thinks it should actually be feasible to do bigger refactoring there without having to worry too much about breaking things.
Looks like the public API went in last August but didn't make it into the 1.7.x release. What sort of schedule are you looking at? Chuck
On Mon, 2013-04-15 at 11:16 -0600, Charles R Harris wrote:
On Mon, Apr 15, 2013 at 10:29 AM, Sebastian Berg <sebastian@sipsolutions.net> wrote: Hey,
the MapIter API has only been made public in master right? So it is no problem at all to change at least the mapiter struct, right?
I got annoyed at all those special cases that make things difficult to get an idea where to put i.e. to fix the boolean array-like stuff. So actually started rewriting it (and I already got one big function that does all index preparation -- ok it is untested but its basically there).
I would guess it is not really a big problem even if it was public for longer, since you shouldn't do those direct struct access probably? But just checking.
Since I got the test which mimics complex indexes in the tests, I thinks it should actually be feasible to do bigger refactoring there without having to worry too much about breaking things.
Looks like the public API went in last August but didn't make it into the 1.7.x release. What sort of schedule are you looking at?
Not sure about a schedule, I somewhat think it is not even that hard, but of course it would still take a while (once I get a bit further, I will put it out there, hopefully someone else will be interested to help), but certainly not aiming to get anything done for 1.8. My first idea was to just do the parsing differently and keep the mapiter part identical (or with minor modifications). That seems actually impractical, since MapIter has a lot of stuff that it does not need. Plus it seems to me that it might be worth it to use the new nditer. One could try keep the fields somewhat identical (likely identical enough to be binary compatible with that ufunc.at pull request even), but I am not even sure that that is something to aim for, since the ufunc.at could be modified too (and might get good speed improvements out of that). - Sebastian
Chuck
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Mon, Apr 15, 2013 at 1:27 PM, Sebastian Berg <sebastian@sipsolutions.net>wrote:
On Mon, 2013-04-15 at 11:16 -0600, Charles R Harris wrote:
On Mon, Apr 15, 2013 at 10:29 AM, Sebastian Berg <sebastian@sipsolutions.net> wrote: Hey,
the MapIter API has only been made public in master right? So it is no problem at all to change at least the mapiter struct, right?
I got annoyed at all those special cases that make things difficult to get an idea where to put i.e. to fix the boolean array-like stuff. So actually started rewriting it (and I already got one big function that does all index preparation -- ok it is untested but its basically there).
I would guess it is not really a big problem even if it was public for longer, since you shouldn't do those direct struct access probably? But just checking.
Since I got the test which mimics complex indexes in the tests, I thinks it should actually be feasible to do bigger refactoring there without having to worry too much about breaking things.
Looks like the public API went in last August but didn't make it into the 1.7.x release. What sort of schedule are you looking at?
Not sure about a schedule, I somewhat think it is not even that hard, but of course it would still take a while (once I get a bit further, I will put it out there, hopefully someone else will be interested to help), but certainly not aiming to get anything done for 1.8.
My first idea was to just do the parsing differently and keep the mapiter part identical (or with minor modifications). That seems actually impractical, since MapIter has a lot of stuff that it does not need. Plus it seems to me that it might be worth it to use the new nditer. One could try keep the fields somewhat identical (likely identical enough to be binary compatible with that ufunc.at pull request even), but I am not even sure that that is something to aim for, since the ufunc.at could be modified too (and might get good speed improvements out of that).
- Sebastian
Makes me wonder if we should expose the API in 1.8 if you are thinking a change might be appropriate. Or am I missing something here? Chuck
On Mon, 2013-04-15 at 13:36 -0600, Charles R Harris wrote:
On Mon, Apr 15, 2013 at 1:27 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote: On Mon, 2013-04-15 at 11:16 -0600, Charles R Harris wrote: > > > On Mon, Apr 15, 2013 at 10:29 AM, Sebastian Berg > <sebastian@sipsolutions.net> wrote: > Hey, > > the MapIter API has only been made public in master right?
<snip>
> Looks like the public API went in last August but didn't make it into > the 1.7.x release. What sort of schedule are you looking at? >
Not sure about a schedule, I somewhat think it is not even that hard, but of course it would still take a while (once I get a bit further, I will put it out there, hopefully someone else will be interested to help), but certainly not aiming to get anything done for 1.8.
My first idea was to just do the parsing differently and keep the mapiter part identical (or with minor modifications). That seems actually impractical, since MapIter has a lot of stuff that it does not need. Plus it seems to me that it might be worth it to use the new nditer. One could try keep the fields somewhat identical (likely identical enough to be binary compatible with that ufunc.at pull request even), but I am not even sure that that is something to aim for, since the ufunc.at could be modified too (and might get good speed improvements out of that).
- Sebastian
Makes me wonder if we should expose the API in 1.8 if you are thinking a change might be appropriate. Or am I missing something here?
Yeah, I am wondering about that. But since I am not clear on exactly if and how one would reimplement it right now (certainly it would look very similar in the basic design), there is a bit time before deciding that maybe. And maybe someone else has an opinion one way or another? For example the MapIter currently does not expose the subspace as a separate iterator. You could access it, but you cannot optimize subspace iteration by handling it separately. I am thinking about something like the np.nestediters, but the user would maybe have to check if the inner iterator even exists (the subspace can easily be 0-d or have only one element). Also, I could imagine to tag a second array onto the fancy index iteration itself. That would be iterated together with the fancy indexes in one nditer and return pointers into its own subspace. That array would be the value array in assignment or the new array in subscription. - Sebastian
Chuck
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Mon, Apr 15, 2013 at 5:29 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote:
Hey,
the MapIter API has only been made public in master right? So it is no problem at all to change at least the mapiter struct, right?
I got annoyed at all those special cases that make things difficult to get an idea where to put i.e. to fix the boolean array-like stuff. So actually started rewriting it (and I already got one big function that does all index preparation -- ok it is untested but its basically there).
I would guess it is not really a big problem even if it was public for longer, since you shouldn't do those direct struct access probably? But just checking.
Why don't we just make the struct opaque, i.e., just declare it in the public header file and move the actual definition to an internal header file? If it's too annoying I guess we could even make it non-public, at least in 1.8 -- IIRC it's only there so we can use it in umath, and IIRC the patch to use it hasn't landed yet. Or we could just merge umath and multiarray into a single .so, that would save a *lot* of annoying fiddling with the public API that doesn't actually serve any purpose. -n
Hi, this is currently used in Theano! In fact, it is a John S. that implemented it in NumPy to allow fast gradient of the advanced indexing in Theano. It allow code like: matrix1[vector1, vector2] += matrix2 where there is duplicate indices in the vector In looking at the code, I saw it use at least those part of the interface. PyArrayMapIterObject PyArray_MapIterNext PyArray_ITER_NEXT PyArray_MapIterSwapAxes PyArray_BroadcastToShape I lost the end of this discussion, but I think this is not possible in NumPy as there was not an agreement to include that. But I remember a few other user on this list asking for this(and they where Theano user to my knowledge). So I would prefer that you don't remove the part that we use for the next 1.8 release. thanks Frédéric On Tue, Apr 16, 2013 at 9:54 AM, Nathaniel Smith <njs@pobox.com> wrote:
On Mon, Apr 15, 2013 at 5:29 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote:
Hey,
the MapIter API has only been made public in master right? So it is no problem at all to change at least the mapiter struct, right?
I got annoyed at all those special cases that make things difficult to get an idea where to put i.e. to fix the boolean array-like stuff. So actually started rewriting it (and I already got one big function that does all index preparation -- ok it is untested but its basically there).
I would guess it is not really a big problem even if it was public for longer, since you shouldn't do those direct struct access probably? But just checking.
Why don't we just make the struct opaque, i.e., just declare it in the public header file and move the actual definition to an internal header file?
If it's too annoying I guess we could even make it non-public, at least in 1.8 -- IIRC it's only there so we can use it in umath, and IIRC the patch to use it hasn't landed yet. Or we could just merge umath and multiarray into a single .so, that would save a *lot* of annoying fiddling with the public API that doesn't actually serve any purpose.
-n _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Tue, 2013-04-23 at 17:08 -0400, Frédéric Bastien wrote:
Hi,
this is currently used in Theano! In fact, it is a John S. that implemented it in NumPy to allow fast gradient of the advanced indexing in Theano. It allow code like:
matrix1[vector1, vector2] += matrix2
Yes, I had missed that and thought maybe nobody actually used it yet. I gave some points why I think there should be some changes in the original pull request [1]. Mostly I think it would make sense (also a lot for theano) to rewrite it with the new iterators and expose the subspace more directly. That would give vast speedups for mixed fancy/non-fancy indices. But if this is useful to you, I guess one can also just create a new one if someone finds time, leaving the old MapIter deprecated and unmaintained. [1] https://github.com/numpy/numpy/pull/377
where there is duplicate indices in the vector
In looking at the code, I saw it use at least those part of the interface.
PyArrayMapIterObject PyArray_MapIterNext PyArray_ITER_NEXT PyArray_MapIterSwapAxes PyArray_BroadcastToShape
There is likely no reason for changing these, but improving MapIter would likely break binary compatibility because of struct access. - Sebastian
I lost the end of this discussion, but I think this is not possible in NumPy as there was not an agreement to include that. But I remember a few other user on this list asking for this(and they where Theano user to my knowledge).
So I would prefer that you don't remove the part that we use for the next 1.8 release.
thanks
Frédéric
On Tue, Apr 16, 2013 at 9:54 AM, Nathaniel Smith <njs@pobox.com> wrote: On Mon, Apr 15, 2013 at 5:29 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote: > Hey, > > the MapIter API has only been made public in master right? So it is no > problem at all to change at least the mapiter struct, right? > > I got annoyed at all those special cases that make things difficult to > get an idea where to put i.e. to fix the boolean array-like stuff. So > actually started rewriting it (and I already got one big function that > does all index preparation -- ok it is untested but its basically > there). > > I would guess it is not really a big problem even if it was public for > longer, since you shouldn't do those direct struct access probably? But > just checking.
Why don't we just make the struct opaque, i.e., just declare it in the public header file and move the actual definition to an internal header file?
If it's too annoying I guess we could even make it non-public, at least in 1.8 -- IIRC it's only there so we can use it in umath, and IIRC the patch to use it hasn't landed yet. Or we could just merge umath and multiarray into a single .so, that would save a *lot* of annoying fiddling with the public API that doesn't actually serve any purpose.
-n _______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
_______________________________________________ NumPy-Discussion mailing list NumPy-Discussion@scipy.org http://mail.scipy.org/mailman/listinfo/numpy-discussion
On Tue, Apr 23, 2013 at 4:06 PM, Sebastian Berg <sebastian@sipsolutions.net>wrote:
On Tue, 2013-04-23 at 17:08 -0400, Frédéric Bastien wrote:
Hi,
this is currently used in Theano! In fact, it is a John S. that implemented it in NumPy to allow fast gradient of the advanced indexing in Theano. It allow code like:
matrix1[vector1, vector2] += matrix2
Yes, I had missed that and thought maybe nobody actually used it yet. I gave some points why I think there should be some changes in the original pull request [1]. Mostly I think it would make sense (also a lot for theano) to rewrite it with the new iterators and expose the subspace more directly. That would give vast speedups for mixed fancy/non-fancy indices.
But if this is useful to you, I guess one can also just create a new one if someone finds time, leaving the old MapIter deprecated and unmaintained.
[1] https://github.com/numpy/numpy/pull/377
where there is duplicate indices in the vector
In looking at the code, I saw it use at least those part of the interface.
PyArrayMapIterObject PyArray_MapIterNext PyArray_ITER_NEXT PyArray_MapIterSwapAxes PyArray_BroadcastToShape
There is likely no reason for changing these, but improving MapIter would likely break binary compatibility because of struct access.
- Sebastian
I lost the end of this discussion, but I think this is not possible in NumPy as there was not an agreement to include that. But I remember a few other user on this list asking for this(and they where Theano user to my knowledge).
So I would prefer that you don't remove the part that we use for the next 1.8 release.
thanks
Frédéric
On Tue, Apr 16, 2013 at 9:54 AM, Nathaniel Smith <njs@pobox.com> wrote: On Mon, Apr 15, 2013 at 5:29 PM, Sebastian Berg <sebastian@sipsolutions.net> wrote: > Hey, > > the MapIter API has only been made public in master right? So it is no > problem at all to change at least the mapiter struct, right? > > I got annoyed at all those special cases that make things difficult to > get an idea where to put i.e. to fix the boolean array-like stuff. So > actually started rewriting it (and I already got one big function that > does all index preparation -- ok it is untested but its basically > there). > > I would guess it is not really a big problem even if it was public for > longer, since you shouldn't do those direct struct access probably? But > just checking.
Why don't we just make the struct opaque, i.e., just declare it in the public header file and move the actual definition to an internal header file?
If it's too annoying I guess we could even make it non-public, at least in 1.8 -- IIRC it's only there so we can use it in umath, and IIRC the patch to use it hasn't landed yet. Or we could just merge umath and multiarray into a single .so, that would save a *lot* of annoying fiddling with the public API that doesn't actually serve any purpose.
Does this have any overlap with https://github.com/numpy/numpy/pull/2821 ? Chuck
participants (4)
-
Charles R Harris -
Frédéric Bastien -
Nathaniel Smith -
Sebastian Berg