Skip to content

ENH: add an axis argument to np.bincount - #4330

Closed
jaimefrio wants to merge 2 commits into
numpy:masterfrom
jaimefrio:ndbincount
Closed

jaimefrio wants to merge 2 commits into
numpy:masterfrom
jaimefrio:ndbincount

Conversation

@jaimefrio

Copy link
Copy Markdown
Member

This adds an axis argument to np.bincount, which can now take multidimensional
arrays and do the counting over an arbitrary number of axes. axis defaults
to None, which does the counting over all the axes, i.e. over the flattened
array.

The shape of the output is computed by removing from list (the input array)
all dimensions in axis, and appending a dimension of length
max(max(list) + 1, minlength) to the end. out[..., n] will hold the number
of occurrences of n at the given position over the selected axes.

If a weights argument is provided, its shape is broadcasted with list
before removing the axes. In this case, axis refers to the axes of list
before broadcasting, and out[..., n] will hold the sum of weights over
the selected axes, at all positions where list takes the value n.

The general case is handled with nested iterators, but shortcuts without
having to set up an iterator are provided for 1D cases, with no performance
loss against the previous version.

As a plus, this PR also solves #823, by providing specialized functions for
all integer types to find the max. There are also specialized functions for
all integer types for counting and doing weighted counting.

This adds an axis argument to np.bincount, which can now take multidimensional
arrays and do the counting over an arbitrary number of axes. `axis` defaults
to `None`, which does the counting over all the axes, i.e. over the flattened
array.

The shape of the output is computed by removing from `list` (the input array)
all dimensions in `axis`, and appending a dimension of length
`max(max(list) + 1, minlength)` to the end. `out[..., n]` will hold the number
of occurrences of `n` at the given position over the selected axes.

If a `weights` argument is provided, its shape is broadcasted with `list`
before removing the axes. In this case, `axis` refers to the axes of `list`
*before* broadcasting, and `out[..., n]` will hold the sum of `weights` over
the selected axes, at all positions where `list` takes the value `n`.

The general case is handled with nested iterators, but shortcuts without
having to set up an iterator are provided for 1D cases, with no performance
loss against the previous version.

As a plus, this PR also solves numpy#823, by providing specialized functions for
all integer types to find the max. There are also specialized functions for
all integer types for counting and doing weighted counting.
Comment thread numpy/add_newdocs.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think list is an unfortunate choice for a variable name, x was fine, but maybe arr or a instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was basically trying to make explicit the following current behavior:

>>> np.bincount(list=[3,4,5])
array([0, 0, 0, 1, 1, 1], dtype=int64)

Internally, the first argument is a kwarg with label list. I agree that it is an unfortunate choice, and will change it both internally and in the docstring to arr. Since it is an undocumented feature, I understand this change can be made with out worries, right?

@charris

charris commented Feb 22, 2014

Copy link
Copy Markdown
Member

I bailed early here ;) looking at the code size here relative to the original function, I prefer the original. The use of type specific code complicates things enormously. A simple, if not so fast, approach, would be to do this in the python call, just calling bincount in a loop and using max to set up the output array shape.

I guess I'm saying this is a premature optimization ;)

This does suggest we could use a standard function dispatcher.

@charris

charris commented Feb 22, 2014

Copy link
Copy Markdown
Member

To avoid arr_bincount doing extra work, we could add a argument to set the length and discard any bins greater than that. Maybe clip, meaning minlength is also a maxlength.

@jaimefrio

Copy link
Copy Markdown
Member Author

Yes, I probably got a little carried away optimizing performance once I got it running. The type specific code, is there to keep the performance for small arrays vs the old function. There was also the thing of handling uint64 inputs, but there have to be other options if that is all you want done. If you think this is bad, you should have seen it with specialized loops for the weights types, to allow complex numbers there also! ;-)

It is very easy to not do any of this and simply have the iterator buffer and cast everything to npy_intp/npy_double and have a single inlined loop. There is a performance hit for small arrays in setting up the iterator, which then run about 2x slower, but making contiguous copies of 1D arrays if they are not behaved/of the right type will probably take care of most of that.

Let me try to rewrite with a leaner approach: I promise you want have to read over so much code!

@charris

charris commented Feb 22, 2014

Copy link
Copy Markdown
Member

I think a clip keyword might be useful in any case.

@jaimefrio

Copy link
Copy Markdown
Member Author

I have just pushed a slimmed down version of ndbincount. As is, it has all the bells and whistles of broadcasting using the axis parameter, but is comparably as fast as the original version for 1D arrays. It could be simplified a little by always using the iterator, but then it would be 2x slower than the original code for small arrays. It's still several hundred more lines of code...

@josef-pkt

Copy link
Copy Markdown

Just as cheerleader for this since another issue reminded me of it:

statsmodels would have quite a lot of use for a vectorized version of bincount. I always missed that.
I never hit the "send" button for my reply almost a year ago. The main questions were about broadcasting which I don't know whether they are still relevant.
In any case, I and we can adjust to whatever the pattern numpy has now for partial reduction, and we will use any vectorized version if it's more efficient than the plain loops.

@jaimefrio

Copy link
Copy Markdown
Member Author

I am closing this for now. There are several functions out there that could use an axis argument, and bincount is one of them. But rather than bloating them all with code, the proper approach seems to me to build some intermediate layer over nditer that abstracts the complexity away. It would probably make for a nice GSoC project...

@jaimefrio jaimefrio closed this Mar 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants