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

BUG, ENH: np._from_dlpack: export correct device information #21119

Merged
merged 1 commit into from Feb 25, 2022

Conversation

tirthasheshpatel
Copy link
Contributor

fixes gh-20340

It is possible to export incorrect device information if a new view has been created from an imported array:

import numpy as np
from torch

x = torch.arange(10).pin_memory()
y = np._from_dlpack(x)
y.base  # PyCapsule named 'numpy_dltensor' (contains device information)
z = y[::2]
z.base  # y (doesn't contain original device information)
z.__dlpack_device__()  # (1, 0) --> wrong (should be (3, 0) for pinned memory)

This PR fixes this behavior by walking the bases (if they are ndarray) as suggested in #20340 (comment). The test just coves the new changes but doesn't really test against an array from a different device.

cc @seberg @mattip

@seberg
Copy link
Member

seberg commented Feb 25, 2022

Looks good, thanks.

Because it was maybe not clear: I do not care for attempting to retain this information. But this seemed to be the intention of the original PR – i.e. it seemed the original PR fell short of its intention because it is a bit tricky.

@seberg seberg merged commit 08248aa into numpy:main Feb 25, 2022
@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Feb 25, 2022
@charris charris added this to the 1.22.3 release milestone Feb 25, 2022
@tirthasheshpatel tirthasheshpatel deleted the fix-gh20340 branch February 25, 2022 15:29
@leofang
Copy link
Contributor

leofang commented Feb 25, 2022

Good catch!

@mattip mattip mentioned this pull request Feb 27, 2022
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Mar 2, 2022
@charris charris removed this from the 1.22.3 release milestone Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG,ENH: DLPack tries to export the imported "device" information but can fail
4 participants