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.

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

Kernel fails to write to ti.types.ndarray if it is a numpy ndarray view #6305

Closed
yuanming-hu opened this issue Oct 12, 2022 · 1 comment · Fixed by #6376
Closed

Kernel fails to write to ti.types.ndarray if it is a numpy ndarray view #6305

yuanming-hu opened this issue Oct 12, 2022 · 1 comment · Fixed by #6376
Assignees
@ailzhang
Labels
potential bug Something that looks like a bug but not yet confirmed

Comments

@yuanming-hu
Copy link
Member

yuanming-hu commented Oct 12, 2022 •
edited
Loading

Describe the bug

import taichi as ti
import numpy as np

ti.init(arch=ti.gpu)

@ti.kernel
def fill(img: ti.types.ndarray()):
    img[0] = 1

a = np.zeros(shape=(2, 2))[:, 0] # Creates an ndarray view here, I guess

fill(a)
print(a) # [0. 0.]

b = a.copy()
fill(b)
print(b) # [1. 0.]

Output

[Taichi] version 1.1.3, llvm 10.0.0, commit 0a7ff865, linux, python 3.8.10
[Taichi] Starting on arch=cuda
[0. 0.]
[1. 0.]

Perhaps we should loudly emit an run-time error message when passing in an ndarray view and simultaneously writing to it?
@yuanming-hu yuanming-hu added the potential bug Something that looks like a bug but not yet confirmed label Oct 12, 2022
@taichi-gardener taichi-gardener moved this to Untriaged in Taichi Lang Oct 12, 2022
@FantasyVR FantasyVR moved this from Untriaged to Todo in Taichi Lang Oct 17, 2022
ailzhang pushed a commit to ailzhang/taichi that referenced this issue Oct 19, 2022
[bug][lang] Fix copyback for fortran contiguous numpy arrays
7291e97 
fixes taichi-dev#6305

Taichi kernel assumes input external array are row major / c_contiguous
for now so this PR throws an error message when the input numpy array
isn't contiguous.

Note that we only supports inplace update for c_contiguous
numpy arrays but for historical reason support for f_contiguous array
was added via copying (`ascontiguousarray()`) as well, thus this PR tries to
preserve the support to avoid breaking old code.

Also the support for f_contiguous numpy array was halfly done since
`ascontiguousarray()` may return a new numpy array, Taichi kernels just
read/write on the copied array and don't copy the values back
to the original numpy array.

This PR fixes the bug mentioned above by adding a callback function to
copy values back, although copying behavior isn't efficient it
guarantees correctness so that we can improve it in the future.
@ailzhang ailzhang mentioned this issue Oct 19, 2022
Merged
@ailzhang
Copy link
Contributor

@yuanming-hu there're indeed two issues in our numpy array interaction that #6376 tries to fix, shall we continue discussion for this issue on that PR review directly? ;)

Repository owner moved this from Todo to Done in Taichi Lang Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ailzhang ailzhang

Labels
potential bug Something that looks like a bug but not yet confirmed
Projects
Taichi Lang
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[bug] [lang] Fix copyback for fortran contiguous numpy arrays ailzhang/taichi
2 participants
@ailzhang @yuanming-hu