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] [lang] Fix copyback for fortran contiguous numpy arrays #6376

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

ailzhang
Copy link
Contributor

fixes #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.

Issue: #

Brief Summary

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.
@netlify
Copy link

netlify bot commented Oct 19, 2022

Deploy Preview for docsite-preview ready!

Name Link
🔨 Latest commit 7291e97
🔍 Latest deploy log https://app.netlify.com/sites/docsite-preview/deploys/634f9c7362957700084e1ed3
😎 Deploy Preview https://deploy-preview-6376--docsite-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ailzhang ailzhang changed the title [bug][lang] Fix copyback for fortran contiguous numpy arrays [bug] [lang] Fix copyback for fortran contiguous numpy arrays Oct 19, 2022
Copy link
Contributor

@strongoier strongoier left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Kernel fails to write to ti.types.ndarray if it is a numpy ndarray view
2 participants