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]: Calling from_dlpack with a DLPack tensor is deprecated. #729

Closed
hpkfft opened this issue Sep 19, 2024 · 6 comments
Closed

[BUG]: Calling from_dlpack with a DLPack tensor is deprecated. #729

hpkfft opened this issue Sep 19, 2024 · 6 comments

Comments

@hpkfft
Copy link
Contributor

hpkfft commented Sep 19, 2024

Problem description

An extension that returns an ndarray<nb::jax> will give the user the following DeprecationWarning:

Calling from_dlpack with a DLPack tensor is deprecated. The argument to from_dlpack should be an array from another framework that implements the __dlpack__ protocol.

This is seen using test_ndarray.py::test19_return_jax in PR #728

Reproducible example code

python -m pip install -U "jax[cpu]"
pytest tests/test_ndarray.py
@wjakob
Copy link
Owner

wjakob commented Sep 20, 2024

Thank you for the report, this is now fixed on master.

@hpkfft
Copy link
Contributor Author

hpkfft commented Sep 21, 2024

Thanks!

When a framework is not specified, would it be a good idea to implement the __dlpack__ protocol as you did for JAX?
For example, looking at the ndarray test test15_consume_numpy, the python code does the following:

    class wrapper:
        def __init__(self, value):
            self.value = value
        def __dlpack__(self):
            return self.value
    a = t.return_dlpack()
    x = np.from_dlpack(wrapper(a))

Would it be good if a already had the two member functions __dlpack__ and __dlpack_device__ so the wrapper above was not needed?

@wjakob
Copy link
Owner

wjakob commented Sep 21, 2024

That would be a break in documented behavior, so I don't think this could be changed in a minor version bump.

@wjakob
Copy link
Owner

wjakob commented Sep 21, 2024

@wjakob
Copy link
Owner

wjakob commented Sep 22, 2024

@hpkfft: see commit 8dc834c for a pattern that is probably more broadly useful: directly implementing these methods on the underlying array types.

@hpkfft
Copy link
Contributor Author

hpkfft commented Sep 22, 2024

Thank you for letting me know about my website problem! It should be fixed now.

Yes, I've already realized I could implement these two methods in my extension.
(Though, I did not think to provide kwargs; I will look into this and see if it might be useful to me.)
If you're curious, see the example I have at the bottom of this section: https://hpkfft.com/python.html#data-types

It was nice of you to add this advice to the nanobind documentation, both for me and for others.

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

No branches or pull requests

2 participants