You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
.reduceat does not handle repeated indices correctly. When an index is repeated the neutral element of the operation should be returned. In the example below [0, 10], not [1, 10], is expected.
In [1]:import numpy
In [2]:numpy.version.version
Out[2]:'1.0b1'
In [3]:a = numpy.arange (5)
In [4]:numpy.add.reduceat (a, (1,1))
Out[4]:array([ 1, 10])
Unfortunately, perhaps, the reduceat method of NumPy follows the behavior of the reduceat method of Numeric for this corner case.
There is no facility for returning the "identity" element of the operation in cases of index-equality. The defined behavior is to return the element given by the first index if the slice returns an empty sequence. Therefore, the documented and actual behavior of reduceat in this case is to construct
I think this is closely connected to #835: If one of the indices is len(a), reduceat cannot output the element at that index, which is needed if the index len(a) appears or is repeated at the end of the indices.
Some solutions:
an option to reduceat to not set any value in the output where end - start == 0
an option to set the output to a given fixed value where end - start == 0
a where parameter, like in ufunc(), which masks which outputs should be calculated at all.
Has there been any more thought on this issue? I would be interested in having the option to set the output to the identity value (if it exists) where end - start == 0.
I strongly support the change of the reduceat behaviour as suggested in this long-standing open issue. It looks like a clear bug or obvious design mistake which hinders the usefulness of this great Numpy construct.
reduceat should behave consistently for all indices. Namely, for every index i, ufunc.reduceat(a, indices) should return ufunc.reduce(a[indices[i]:indices[i+1]]).
This should also be true for the case indices[i] == indices[i+1]. I cannot see any sensible reason why, in this case, reduceat should return a[indices[i]] instead of ufunc.reduce(a[indices[i]:indices[i+1]]).
Wow, this is indeed terrible and broken.
.
We'd need some discussion on the mailing list, but I at least would be
totally in favor of making that issue a FutureWarning in the next release
and fixing the behavior a few releases later. We'd need someone to take the
lead on starting that discussion and writing the patch. Perhaps that's you?
Indeed, the behaviour should be driven by the consistency with ufunc.reduce(a[indices[i]:indices[i+1]]), which is what every user would expect. So this does not require new design decisions. It really looks just like a long standing bug fix to me. Unless anybody can justify the current inconsistent behaviour.
@divenex: did you check your spam folder? (I always forget to do that...) Otherwise I'm not sure what could be going wrong. There definitely shouldn't be any special requirements to subscribe beyond "has an email address". If you still can't get it to work then speak up and we'll try to track down the relevant sysadmin... We definitely want to know if it's broken.
A version of reduceat that is consistent with ufunc.reduce(a[indices[i]:indices[i+1]]) would be really, really nice. It would be so much more useful! Either an argument to select the behavior or a new function (reduce_intervals? reduce_segments? ...?) would avoid breaking backwards incompatibility.
@eric-wieser I would be OK with requiredstart and stop arguments, but I do not like making one of them optional. It is not obvious that providing only start means out[i] = func.reduce(arr[start[i]:start[i+1]]) rather than out[i] = func.reduce(arr[start[i]:]), which is what I would have guessed.
My preferred API for reducebins is like reduceat but without the confusing "exceptions" noted in the docstring. Namely, just:
For i in range(len(indices)), reduceat computes ufunc.reduce(a[indices[i]:indices[i+1]]), which becomes the i-th generalized “row” parallel to axis in the final result (i.e., in a 2-D array, for example, if axis = 0, it becomes the i-th row, but if axis = 1, it becomes the i-th column).
I could go either way on the third "exception" which requires non-negative indices (0 <= indices[i] <= a.shape[axis]), which I view as more of a sanity check rather than an exception. But possibly that one could go, too -- I can see how negative indices might be useful to someone, and it's not hard to do the math to normalize such indices.
Not automatically adding an index at the end does imply that the result should have length len(a)-1, like the result of np.histogram.
@jni Can you please give an example of what you actually want to calculate from arrays found in sparse matrices? Preferably with a concrete example with non-random numbers, and self contained (without depending on scipy.sparse).
It is not obvious that providing only start means out[i] = func.reduce(arr[start[i]:start[i+1]]) rather than out[i] = func.reduce(arr[start[i]:]), which is what I would have guessed.
The reading I was going for is that "Each bin starts at these positions", with the implication that all bins are contiguous unless explicitly specified otherwise. Perhaps I should try and draft a more complete docstring. I think I can see a strong argument for forbidding passing neither argument, so I'll remove that from my propose function.
which requires non-negative indices (0 <= indices[i] < a.shape[axis])
Note that there's also a bug here (#835) - the upper bound should be inclusive, since these are slices.
Triggered by this issue coming alive again, I wrote #25476 to see how hard it would be to allow passing in a 2-D array with start, stop values. Not too hard, it turns out. But API to be decided -- best discussed at #25476, probably!
Activity
numpy-gitbot commentedon Oct 23, 2012
@teoliphant wrote on 2006-08-08
Unfortunately, perhaps, the reduceat method of NumPy follows the behavior of the reduceat method of Numeric for this corner case.
There is no facility for returning the "identity" element of the operation in cases of index-equality. The defined behavior is to return the element given by the first index if the slice returns an empty sequence. Therefore, the documented and actual behavior of reduceat in this case is to construct
[a[1], add.reduce(a[1:])]
This is a feature request.
numpy-gitbot commentedon Oct 23, 2012
trac user martin_wiechert wrote on 2006-08-08
also see ticket #835
numpy-gitbot commentedon Oct 23, 2012
Milestone changed to
1.1by @alberts on 2007-05-12numpy-gitbot commentedon Oct 23, 2012
Milestone changed to
Unscheduledby @cournape on 2009-03-02jnothman commentedon Jan 29, 2014
I think this is closely connected to #835: If one of the indices is
len(a),reduceatcannot output the element at that index, which is needed if the indexlen(a)appears or is repeated at the end of the indices.Some solutions:
reduceatto not set any value in the output whereend - start == 0end - start == 0whereparameter, like inufunc(), which masks which outputs should be calculated at all.jayvius commentedon Jul 2, 2015
Has there been any more thought on this issue? I would be interested in having the option to set the output to the identity value (if it exists) where end - start == 0.
divenex commentedon Nov 25, 2015
I strongly support the change of the
reduceatbehaviour as suggested in this long-standing open issue. It looks like a clear bug or obvious design mistake which hinders the usefulness of this great Numpy construct.reduceatshould behave consistently for all indices. Namely, for every index i,ufunc.reduceat(a, indices)should returnufunc.reduce(a[indices[i]:indices[i+1]]).This should also be true for the case
indices[i] == indices[i+1]. I cannot see any sensible reason why, in this case, reduceat should returna[indices[i]]instead ofufunc.reduce(a[indices[i]:indices[i+1]]).See also HERE a similar comment by Pandas creator Wes McKinney.
njsmith commentedon Nov 25, 2015
Wow, this is indeed terrible and broken.
.
We'd need some discussion on the mailing list, but I at least would be
totally in favor of making that issue a FutureWarning in the next release
and fixing the behavior a few releases later. We'd need someone to take the
lead on starting that discussion and writing the patch. Perhaps that's you?
divenex commentedon Nov 26, 2015
Thanks for the supportive response. I can start a discussion if this helps, but unfortunately am not up to patching the C code.
jnothman commentedon Nov 26, 2015
What do you intend for ufuncs without an identity, such as np.maximum?
njsmith commentedon Nov 26, 2015
For such functions, an empty reduction should be an error, as it already is
when you use .reduce() instead of .reduceat().
divenex commentedon Nov 26, 2015
Indeed, the behaviour should be driven by the consistency with
ufunc.reduce(a[indices[i]:indices[i+1]]), which is what every user would expect. So this does not require new design decisions. It really looks just like a long standing bug fix to me. Unless anybody can justify the current inconsistent behaviour.divenex commentedon Dec 7, 2015
@njsmith I am unable to sign up to the Numpy list. I sent my address here https://mail.scipy.org/mailman/listinfo/numpy-discussion but I never get any "email requesting confirmation". Not sure whether one need special requirements to subscribe...
njsmith commentedon Dec 7, 2015
@divenex: did you check your spam folder? (I always forget to do that...) Otherwise I'm not sure what could be going wrong. There definitely shouldn't be any special requirements to subscribe beyond "has an email address". If you still can't get it to work then speak up and we'll try to track down the relevant sysadmin... We definitely want to know if it's broken.
WarrenWeckesser commentedon Oct 4, 2016
A version of
reduceatthat is consistent withufunc.reduce(a[indices[i]:indices[i+1]])would be really, really nice. It would be so much more useful! Either an argument to select the behavior or a new function (reduce_intervals?reduce_segments? ...?) would avoid breaking backwards incompatibility.31 remaining items
shoyer commentedon Apr 19, 2017
@eric-wieser I would be OK with required
startandstoparguments, but I do not like making one of them optional. It is not obvious that providing onlystartmeansout[i] = func.reduce(arr[start[i]:start[i+1]])rather thanout[i] = func.reduce(arr[start[i]:]), which is what I would have guessed.My preferred API for
reducebinsis likereduceatbut without the confusing "exceptions" noted in the docstring. Namely, just:I could go either way on the third "exception" which requires non-negative indices (
0 <= indices[i] <= a.shape[axis]), which I view as more of a sanity check rather than an exception. But possibly that one could go, too -- I can see how negative indices might be useful to someone, and it's not hard to do the math to normalize such indices.Not automatically adding an index at the end does imply that the result should have length
len(a)-1, like the result ofnp.histogram.@jni Can you please give an example of what you actually want to calculate from arrays found in sparse matrices? Preferably with a concrete example with non-random numbers, and self contained (without depending on scipy.sparse).
eric-wieser commentedon Apr 19, 2017
The reading I was going for is that "Each bin starts at these positions", with the implication that all bins are contiguous unless explicitly specified otherwise. Perhaps I should try and draft a more complete docstring. I think I can see a strong argument for forbidding passing neither argument, so I'll remove that from my propose function.
Note that there's also a bug here (#835) - the upper bound should be inclusive, since these are slices.
shoyer commentedon Apr 19, 2017
Fixed, thanks.
eric-wieser commentedon Apr 19, 2017
Not in the
reduceatfunction itself, you haven't ;)eric-wieser commentedon Jan 2, 2018
Turns out that
:\doc\neps\groupby_additions.rstcontains an (IMO inferior) proposal for areducebyfunction.martinling commentedon Dec 22, 2023
Is this something that could be fixed in the upcoming 2.0 release?
mhvk commentedon Dec 22, 2023
Triggered by this issue coming alive again, I wrote #25476 to see how hard it would be to allow passing in a 2-D array with start, stop values. Not too hard, it turns out. But API to be decided -- best discussed at #25476, probably!