Skip to content

Should torch.reshape support integer shape? #28

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
thomasjpfan opened this issue Mar 7, 2023 · 6 comments
Closed

Should torch.reshape support integer shape? #28

thomasjpfan opened this issue Mar 7, 2023 · 6 comments

Comments

@thomasjpfan
Copy link
Contributor

torch.reshape requires an tuple as the shape:

import torch
import array_api_compat 

x = torch.asarray([1.0, 2.0])
xp = array_api_compat.get_namespace(x)

# works
xp.reshape(x, (-1,))

xp.reshape(x, -1)
# TypeError: reshape(): argument 'shape' (position 2) must be tuple of ints, not int

Note that integer shape works with numpy.array_api:

import numpy.array_api as np
import array_api_compat 

x = np.asarray([1.0, 2.0])
xp = array_api_compat.get_namespace(x)

xp.reshape(x, -1)
@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2023

This looks like a bug in numpy.array_api. reshape in the spec only accepts a tuple (in all versions of the spec) https://data-apis.org/array-api/draft/API_specification/generated/array_api.reshape.html#array_api.reshape

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2023

Looks like broadcast_to and the axis argument of permute_dims also have this issue. We might consider changing this in the spec. Most other places that accept shape or axis accept both an int or tuple of ints.

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2023

Let's see what the decision here is data-apis/array-api#608. We should probably update numpy.array_api either way.

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2023

Of course, we can still work around this in the wrappers here. But it wouldn't be portable to other array API compliant libraries (not that any necessarily exist). But this should be too hard to work around in upstream code either.

@rgommers
Copy link
Member

rgommers commented Mar 8, 2023

I think the numpy.array_api support for ints is a bug indeed, and requiring a tuple is good practice here. It's easy to fix this up in scikit-learn, so I suggest doing it there rather than in this repo.

@thomasjpfan
Copy link
Contributor Author

I'll close this issue because torch.reshape is following the spec correctly and there is nothing to change without this repo.

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

3 participants