Skip to content

reduceat cornercase (Trac #236) #834

Open

Description

@numpy-gitbot

Original ticket http://projects.scipy.org/numpy/ticket/236 on 2006-08-07 by trac user martin_wiechert, assigned to unknown.

.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])

Activity

  1. numpy-gitbot commented on Oct 23, 2012

    @numpy-gitbot
    Author

    @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.

  2. numpy-gitbot commented on Oct 23, 2012

    @numpy-gitbot
    Author

    trac user martin_wiechert wrote on 2006-08-08

    also see ticket #835

  3. numpy-gitbot commented on Oct 23, 2012

    @numpy-gitbot
    Author

    Milestone changed to 1.1 by @alberts on 2007-05-12

  4. numpy-gitbot commented on Oct 23, 2012

    @numpy-gitbot
    Author

    Milestone changed to Unscheduled by @cournape on 2009-03-02

  5. jnothman commented on Jan 29, 2014

    @jnothman
    Member

    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.
  6. jayvius commented on Jul 2, 2015

    @jayvius
    Contributor

    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.

  7. divenex commented on Nov 25, 2015

    @divenex

    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]]).

    See also HERE a similar comment by Pandas creator Wes McKinney.

  8. njsmith commented on Nov 25, 2015

    @njsmith
    Member

    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?

  9. divenex commented on Nov 26, 2015

    @divenex

    Thanks for the supportive response. I can start a discussion if this helps, but unfortunately am not up to patching the C code.

  10. jnothman commented on Nov 26, 2015

    @jnothman
    Member

    What do you intend for ufuncs without an identity, such as np.maximum?

  11. njsmith commented on Nov 26, 2015

    @njsmith
    Member

    For such functions, an empty reduction should be an error, as it already is
    when you use .reduce() instead of .reduceat().

  12. divenex commented on Nov 26, 2015

    @divenex

    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.

  13. divenex commented on Dec 7, 2015

    @divenex

    @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...

  14. njsmith commented on Dec 7, 2015

    @njsmith
    Member

    @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.

  15. WarrenWeckesser commented on Oct 4, 2016

    @WarrenWeckesser
    Member

    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.

  16. 31 remaining items

  17. shoyer commented on Apr 19, 2017

    @shoyer
    Member

    @eric-wieser I would be OK with required start 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).

  18. eric-wieser commented on Apr 19, 2017

    @eric-wieser
    Member

    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.

  19. shoyer commented on Apr 19, 2017

    @shoyer
    Member

    Note that there's also a bug here - the upper bound should be inclusive, since these are slices.

    Fixed, thanks.

  20. eric-wieser commented on Apr 19, 2017

    @eric-wieser
    Member

    Not in the reduceat function itself, you haven't ;)

  21. eric-wieser commented on Jan 2, 2018

    @eric-wieser
    Member

    Turns out that :\doc\neps\groupby_additions.rst contains an (IMO inferior) proposal for a reduceby function.

  22. martinling commented on Dec 22, 2023

    @martinling

    Is this something that could be fixed in the upcoming 2.0 release?

  23. mhvk commented on Dec 22, 2023

    @mhvk
    Contributor

    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!

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions