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] Only ban passing non contiguous torch tensors to taichi kernels. #4258

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

ailzhang
Copy link
Contributor

Thanks to @ifsheldon for reporting the bug!

It's too strict to ban all view tensors as Pytorch view tensor return
itself as a new view Tensor when nothing changes.
So what we really wanted to ban was actually non contiguous tensors.

In [2]: a = torch.ones(4, 4)

In [4]: b = a.view(4, 4)

In [6]: b._is_view()
Out[6]: True

In [7]: b.is_contiguous()
Out[7]: True

In [8]: a._is_view()
Out[8]: False

FYI in from_torch we actually explicitly call tensor.contiguous() to
make sure it's valid. For ndarrays we want users to do that explicitly
so that they're aware of the memcpy.

Related issue = #

Thanks to @ifsheldon for reporting the bug!

It's too strict to ban all view tensors as Pytorch view tensor return
itself as a new view Tensor when nothing changes.
So what we really wanted to ban was actually non contiguous tensors.

```
In [2]: a = torch.ones(4, 4)

In [4]: b = a.view(4, 4)

In [6]: b._is_view()
Out[6]: True

In [7]: b.is_contiguous()
Out[7]: True

In [8]: a._is_view()
Out[8]: False
```
FYI in `from_torch` we actually explicitly call `tensor.contiguous()` to
make sure it's valid. For ndarrays we want users to do that explicitly
so that they're aware of the memcpy.
@netlify
Copy link

netlify bot commented Feb 10, 2022

✔️ Deploy Preview for docsite-preview ready!

🔨 Explore the source changes: 9de8ab3

🔍 Inspect the deploy log: https://app.netlify.com/sites/docsite-preview/deploys/620513ed770bdf00075c19ec

😎 Browse the preview: https://deploy-preview-4258--docsite-preview.netlify.app

Copy link
Member

@k-ye k-ye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ailzhang ailzhang merged commit 973c04d into taichi-dev:master Feb 11, 2022
@ailzhang ailzhang deleted the fix_view branch February 11, 2022 03:12
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

Successfully merging this pull request may close these issues.

None yet

2 participants