Skip to content

keepdims argument to argmin() and argmax() #109

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

Closed
asmeurer opened this issue Jan 13, 2021 · 5 comments
Closed

keepdims argument to argmin() and argmax() #109

asmeurer opened this issue Jan 13, 2021 · 5 comments

Comments

@asmeurer
Copy link
Member

The argmin and argmax functions require the keepdims keyword argument, same as min and max. However, in NumPy, min and max have this keyword argument, but argmin and argmax do not. We should confirm whether this keyword argument actually makes sense for these functions, or whether it was just added to these functions by mistake because they are also in the non-arg variants.

The description for keepdims is as follows:

keepdims : bool

If True , the reduced axes (dimensions) must be included in the result as singleton dimensions, and, accordingly, the result must be compatible with the input array (see Broadcasting ). Otherwise, if False , the reduced axes (dimensions) must not be included in the result. Default: False .

Perhaps the reason NumPy doesn't implement this for the arg* functions is that they return indices, so maintaining broadcastability is not important. The documentation for max says (emphasis added):

If this is set to True, the axes which are reduced are left in the result as dimensions with size one. With this option, the result will broadcast correctly against the input array.

@kgryte
Copy link
Contributor

kgryte commented Jan 13, 2021

Adding the keepdims keyword argument was intentional. Background can be found here. While NumPy does not currently support keepdims, other libraries do (e.g., PyTorch).

IIRC, this was discussed in a meeting and keepdims was affirmed to have consistency with statistical function counterparts.

@leofang
Copy link
Contributor

leofang commented Jan 13, 2021

I support to keep keepdims. In CuPy I raised exactly the same question before this standardization effort: cupy/cupy#2595, and our conclusion was that it's useful and worth keeping. Quoting myself from cupy/cupy#2595 (comment):

For this case I think NumPy should change -- naively I'd expect argmax/argmin's behavior should be the same as that of max/min, i.e. accepting a tuple axis and keepdims and so on. I wonder why they didn't do so.

@leofang
Copy link
Contributor

leofang commented Jan 13, 2021

FYI, the same issue was already raised in NumPy: numpy/numpy#8710.

@kgryte
Copy link
Contributor

kgryte commented Jan 13, 2021

@leofang Thanks for the links!

@rgommers
Copy link
Member

Looks like we're in agreement here, so I'll close this issue. Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants