Skip to content
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

Add support for kwargs to tensor() when arg is an ndarray #3797

Merged
merged 6 commits into from
Sep 17, 2022
Merged

Add support for kwargs to tensor() when arg is an ndarray #3797

merged 6 commits into from
Sep 17, 2022

Conversation

SaadAhmedGit
Copy link
Contributor

As discussed in the comments of #3781,
You could not create a tensor using an ndarray with named arguments. The aforementioned function now accepts **kwargs and creates a tensor using torch.as_tensor() which shares memory with the ndarray.

In the following code snippet:

x = np.array([3., 4., 5.])
tensor = fastai.torch_core.tensor(x, requires_grad=True, pin_memory=True)

print(f"Is pinned: {tensor.is_pinned()}")
print(f"Requires gradient: {tensor.requires_grad}")

Previous behaviour:

Expected:

Is pinned: True
Requires gradient: True

Output:

Is pinned: False
Requires gradient: False

After commit:

Expected:

Is pinned: True
Requires gradient: True

Output:

Is pinned: True
Requires gradient: True

As discussed in the comments of #3781,
_array2tensor() should be able to create a tensor from an ndarray using kwargs and this commit adds that feature.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@SaadAhmedGit SaadAhmedGit marked this pull request as draft September 16, 2022 12:43
@SaadAhmedGit
Copy link
Contributor Author

I just found out that you have you to build the notebooks too (I should read the nbdev stuff in more detail). So I'll open this once I have done that.

@SaadAhmedGit SaadAhmedGit marked this pull request as ready for review September 16, 2022 16:13
@jph00
Copy link
Member

jph00 commented Sep 16, 2022

That's a clever approach. Would this work and be a bit cleaner?:

def _array2tensor(x, requires_grad=False, pin_memory=False, **kwargs):
    if x.dtype==np.uint16: x = x.astype(np.float32)
    # windows default numpy int dtype is int32, while torch tensor default int dtype is int64
    # https://github.com/numpy/numpy/issues/9464
    if sys.platform == "win32" and x.dtype==np.int: x = x.astype(np.int64)
    t = torch.as_tensor(x, requires_grad=requires_grad, pin_memory=pin_memory, **kwargs)
    t.requires_grad_(requires_grad)
    if pin_memory: t.pin_memory()
    return t

@SaadAhmedGit
Copy link
Contributor Author

@jph00 yeah your approach is far cleaner but your code doesn't work because as_tensor() doesn't accept requires_grad and pin_memory keyword arguments. I just erased them in as_tensor()'s arguments and it works correctly as we are already changing these attributes after that. I have also made the commit

Just changed this from your code:

t = torch.as_tensor(x, requires_grad=requires_grad, pin_memory=pin_memory, **kwargs)

to this:

t = torch.as_tensor(x, **kwargs)

@jph00 jph00 merged commit 43944fa into fastai:master Sep 17, 2022
@jph00 jph00 changed the title Added **kwargs to _array2tensor() Add support for kwargs to tensor() when arg is an ndarray Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants