-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Request: argmax2d #9283
Comments
Why not use e.g. np.unravel_index(X.argmax(), X.shape) As a bonus, it works when |
Not a fan of |
@eric-wieser how would you say that should behave? Currently This issue aside, it would be a bit weird to use Then again, how would this shape-aware result option work together with the existing |
What if one of the rows in your 2D array don't have a maximum (it's simply constant array) - how argmax can report that? # setup the problem
import numpy as np
x=np.arange(10).reshape([2,5])
x[1,3]=2
# x is this:
# [[0, 1, 2, 3, 4],
# [5, 6, 7, 2, 9]]
# this will behave normally
np.argmax(x==2, axis=1)
# Out: [2,3]
# but this one returns 0 instead of NaN
np.argmax(x==3, axis=1)
# Out: [3,0]
# Expected: [3, nan] It would be nice to have an extra argument for example to let user control what to do in the case with no max available: |
@thoth291 isn't this a more generic question regarding the behaviour of
And this is the customary behaviour in case of ties: choose the first among tied values. Similarly how the |
@adeak, now you said that - I'm starting thinking that this indeed a more general question. That all being said, I would rather avoid such a decision in the first place and return |
This comment has been minimized.
This comment has been minimized.
@adeak: Sorry for never replying
I think there's one obvious way to deal with this. As an example: >>> ret = argmax(np.empty((A, B, C, D, E)), axes=(0, 2))
>>> type(ret)
tuple
>>> len(ret) # == len(axes)
2
>>> ret[0].shape
(B, D, E)
>>> ret[1].shape
(B, D, e) With that and keepdims, you'd get In pseudocode, I'd expect: def argmax(arr, axes, keepdims)
was_tuple = isinstance(axes, tuple):
if not was_tuple:
axes = (axes,)
shape = np.array(arr.shape)
axis_mask = np.array([i in axes for i in range(arr.ndim)])
shape[axis_mask] = 1
ret = tuple(np.empty(shape) for _ in axes)
# do the actual work
if not keepdims:
ret = tuple(r.reshape(shape[~axis_mask]) for r in ret)
if not was_tuple:
return ret[0] |
@eric-wieser - nice catch - I typed it faster than was able to translate properly. Will be careful later. Updated the comment to "should happen" instead of "happens". Hope that helps, otherwise - open for suggestions for reformulation. |
@thoth291: since what you're discussing isn't specific to 2D argmax, I suggest you create a new issue |
@eric-wieser no worries, thanks for getting back to me. I think I understand your suggestion, and it does seem unambiguous and practical. The fact that the type of the |
Both corrected, good catch
Note that the actual array is the same. I think this hits both practicality and purity - the rule is that |
I never would've thought to special-case length-1 tuples, that sounds wrong. I was wondering about the scalar vs tuple case. It did vaguely occur to me that the scalar and length-1 tuple cases correspond to each other in the way you mentioned, but now that you spelled it out it's obvious that scalar -> 1-tuple -> general tuple cases are straightforward generalizations. So thanks, I fully support your suggestion. |
It's an old issue to be sure, but this came up for me today. Given that 1.7 (released after this discussion) added the ability to pass a |
It would be really nice if there was a convenience function to perform an argmax over a 2D array and return the row and column indexes of the maximum.
I'm often finding myself reusing the following code
While it is simple, its opaque enough where I have to consistently look this function up.
It would be nice if this was already available in numpy.
Would there be any objections to opening up a PR in numpy to have this functionality readily available?
The text was updated successfully, but these errors were encountered: