8000 `torch.(min|max)(..., dim=...)` diverges from array API specification · Issue #58745 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

torch.(min|max)(..., dim=...) diverges from array API specification #58745

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.

Sign up for GitHub

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

Jump to bottom
Open
Tracked by #58743
pmeier opened this issue May 21, 2021 · 7 comments
Open
Tracked by #58743

torch.(min|max)(..., dim=...) diverges from array API specification #58745

pmeier opened this issue May 21, 2021 · 7 comments
Labels
module: deprecation module: python array api Issues related to the Python Array API module: reductions triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@pmeier
Copy link
Collaborator
pmeier commented May 21, 2021 •
edited by pytorch-probot bot
Loading

The array API specification stipulates that torch.(max|min) must always return a tensor. If called with dim=... torch.return_types.(max|min) is returned instead.

cc @mruberry @rgommers @pmeier @asmeurer @leofang @heitorschueroff @asi1024
@pmeier pmeier added the module: python array api Issues related to the Python Array API label May 21, 2021
@pmeier pmeier mentioned this issue May 21, 2021
Open
29 tasks
@asi1024
Copy link
Contributor
asi1024 commented May 24, 2021

These fixes break backward compatibility, so further discussion is needed.

@rgommers
Copy link
Collaborator

I was confused by what the issue was from reading the description. This is pretty odd:

>>> import torch
>>> t = torch.tensor([1, 2, 3])
>>> t.min()
tensor(1)
>>> torch.min(t)
tensor(1)
>>> t_min = torch.min(t, axis=0)
>>> t_min
torch.return_types.min(
values=tensor(1),
indices=tensor(0))

Would be nice to get some context first - why does this work this way right now?

@pmeier pmeier changed the title torch.(min|max) diverges from array API specification torch.(min|max)(..., axis=...) diverges from array API specification May 24, 2021
@zou3519 zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 24, 2021
@mruberry
Copy link
Collaborator

I was confused by what the issue was from reading the description. This is pretty odd:

>>> import torch
>>> t = torch.tensor([1, 2, 3])
>>> t.min()
tensor(1)
>>> torch.min(t)
tensor(1)
>>> t_min = torch.min(t, axis=0)
>>> t_min
torch.return_types.min(
values=tensor(1),
indices=tensor(0))

Would be nice to get some context first - why does this work this way right now?

Probably for no good reason. @gchanan do you recall?

8000
@pmeier pmeier changed the title torch.(min|max)(..., axis=...) diverges from array API specification torch.(min|max)(..., dim=...) diverges from array API specification May 30, 2021
@heitorschueroff
Copy link
Contributor

The Array

I was confused by what the issue was from reading the description. This is pretty odd:

>>> import torch
>>> t = torch.tensor([1, 2, 3])
>>> t.min()
tensor(1)
>>> torch.min(t)
tensor(1)
>>> t_min = torch.min(t, axis=0)
>>> t_min
torch.return_types.min(
values=tensor(1),
indices=tensor(0))

Would be nice to get some context first - why does this work this way right now?

@rgommers While NumPy and the Array API standard have two functions max and argmax, PyTorch's solution thus far has been to return both the values and indices, not only for this function but many others. Now that we have committed to following the Array API standard, I think it's a good time to consider breaking BC to break up these functions into one that returns the values and one that returns the indices.

@mruberry
Copy link
Collaborator

The Array

I was confused by what the issue was from reading the description. This is pretty odd:

>>> import torch
>>> t = torch.tensor([1, 2, 3])
>>> t.min()
tensor(1)
>>> torch.min(t)
tensor(1)
>>> t_min = torch.min(t, axis=0)
>>> t_min
torch.return_types.min(
values=tensor(1),
indices=tensor(0))

Would be nice to get some context first - why does this work this way right now?

@rgommers While NumPy and the Array API standard have two functions max and argmax, PyTorch's solution thus far has been to return both the values and indices, not only for this function but many others. Now that we have committed to following the Array API standard, I think it's a good time to consider breaking BC to break up these functions into one that returns the values and one that returns the indices.

We have amax and argmax, which is NumPy compatible and does the same thing.

@rgommers
Copy link
Collaborator

min and max do return a single Tensor if axis/dim isn't given, and the changing return type is pretty odd. I think it's a good candidate for a BC-break.

Also, an explicit dim=None (or axis=None) doesn't work:

>>> torch.max(t, dim=None)
Traceback (most recent call last):
...
RuntimeError: Please look up dimensions by name, got: name = None.

This was referenced Jul 8, 2021
Open
Open
@heitorschueroff
Copy link
Contributor

torch.median and torch.mode have the same problem so I created a umbrella issue here #61490.

@rgommers rgommers mentioned this issue Jan 6, 2022
Open
@asmeurer asmeurer mentioned this issue Jan 9, 2023
Closed
@github-actions github-actions bot mentioned this issue Apr 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module: deprecation module: python array api Issues related to the Python Array API module: reductions triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
3AFD
Development

No branches or pull requests
6 participants
@rgommers @asi1024 @zou3519 @pmeier @heitorschueroff @mruberry
0