scikit-hep / awkward-0.x

Manipulate arrays of complex data structures as easily as Numpy.
BSD 3-Clause "New" or "Revised" License
215 stars 39 forks source link

JaggedArray.argmax() errors for zero-length entries even if max() works #158

Closed AndreasAlbert closed 5 years ago

AndreasAlbert commented 5 years ago

Hi, I encountered a problem when trying to get the argmax of an array I apply some selection to. If the selection happens to kill all entries, argmax will fail even though max still works. I first suspected this was a duplicate of #100, but since max seems to work I am not so sure. Anyway, here's a snippet of what I mean:

from awkward import JaggedArray
import numpy as np
counts = np.array([ 1, 0, 3 ])
values = np.array( [1, 2, 3, 4])

j = JaggedArray.fromcounts(counts, values)

j.max() # works fine
j.argmax() # works fine

# Apply a selection that results in zero 
# passing entries
j2 = j[j < 0]

j2.max() # works fine
j2.argmax() # Error!

The error message is:

ValueError: zero-size array to reduction operation maximum which has no identity

I would have expected to get

j2.argmax() -> [[][][]]

which would be consistent with

j.argmax() -> [[0] [] [2]]

Is this expected?

Thanks

jpivarski commented 5 years ago

Thank you for the explicit example! I had heard rumors of a bug like this, affecting argmax but not max because max is explicitly protected. I had been looking at the wrong part of the argmax code, but now I've found it.

The fix is going through continuous integration. When it's done, it will be version 0.12.0rc2. Since that's a release candidate (you caught me in the middle of working on a semimajor version), you have to ask for it as

pip install "awkward>=0.12.0rc2"

(or similarly for conda) in order to get a release from the 0.12 series, since there are no release-releases in this series yet.

jpivarski commented 5 years ago

When the third-to-last is green, it's ready for pip/conda-install.

https://travis-ci.org/scikit-hep/awkward-array/builds/556511902

AndreasAlbert commented 5 years ago

Thanks for being super efficient about this! Will test as soon as possible.

jpivarski commented 5 years ago

The release candidate is deployed: https://pypi.org/project/awkward/#history but remember that you have to explicitly request it from pip or conda. (Or just use GitHub master.)

You caught me at a good time. :)