data-interchange description of DLPack

[oleksandr-pavlyk]

The implementation section of the data_interchange document contains some incorrect statements:

1. The __dlpack__ method will produce a PyCapsule containing a DLPackManagedTensor, ....

The DLPackManagedTensor should be DLManagedTensor consistent with dlpack.h and the DLPack diagram.

I also think it would be beneficial to make it more precise:

The __dlpack__ method will produce a PyCapsule object containing a reference to DLPackManagedTensor struct, ....

2. The consumer must set the PyCapsule name to "used_dltensor" , and call the deleter of the DLPackManagedTensor when it no longer needs the data.

The DLPackManagedTensor here also should be DLManagedTensor, but more importantly, the consumer should not be calling the deleter function. The DLManagedTensor.deleter must be called by the PyCapsule_Destructor function provided by the producer.

I also would like to call the mandatory nature of capsule renaming from "dltensor" to "used_dltensor" into question. With proper memory management the capsule can be consumed more than once, creating multiple views into the same allocation.

I would therefore like to propose to change that statement from a requirement to a recommendation.

[rgommers]

Thanks @oleksandr-pavlyk! Agreed with most of what you wrote.

With proper memory management the capsule can be consumed more than once, creating multiple views into the same allocation.

IIRC that's not the case, and anyway it's not possible with normal use of the array API standard - there is no separate capsule anywhere unless one calls the __dlpack__ method directly (which is a really bad idea).

[seberg]

I also would like to call the mandatory nature of capsule renaming from "dltensor" to "used_dltensor" into question. With proper memory management the capsule can be consumed more than once, creating multiple views into the same allocation.

That may be OK, but you have to be very clear about the fact that a producer must produce a fresh capsule every time, because consumers may rename the capsule and "steal" ownership of the DLManagedTensor! And there can only be one owner for it (either the capsule or the importer).
Currently, everyone renames the capsule and this is not an issue (there is no reason to forget it). You also may have to be clear, that this means it is only OK for an import function that calls __dlpack__ explicitly (just in case).

With the above exception, what should be added is something like: The consumer must take ownership of the DLManangedTensor by renaming the capsule to "used_dltensor" (note that there can only be a single owner of the DLManagedTensor, renaming the capsule transfers that ownership from the capsule to your object). After taking ownership by renaming the capsule, the consumer must call the deleter function when it does not need the memory anymore.

[oleksandr-pavlyk]

I like this verbiage. I am going to transfer it to #338.

[oleksandr-pavlyk]

@seberg I agree that there must be one owner of the DLManagedTensor.

1. The owner DLManagedTensor is the PyCapsule object, and it ensures that memory allocated for DLManagedTensor is freed in the logic of PyCapsule_Destructor function. The array created from such capsule can reference the capsule in the .base field ensuring that the capsule, DLManagedTensor and the context stored in it are alive while the array is alive.
2. The owner of DLManagedTensor is transferred to the consumer. The consumer must set a different name for the capsule, whose destructor function should be sure to not call DLManagedTensor.deleter, which now becomes consumer's responsibility.
   The consumer should create its own object to manage DLManagedTensor lifetime, and this object is referenced in .base field of the array created from such a memory.

The docs seem to require adopting 2, while I find 1 simpler, so if we agree that both are viable ways to manage DLManagedTensor lifetime, I am asking that docs do not mandate 2 over 1.

[seberg]

The docs seem to require adopting 2, while I find 1 simpler, so if we agree that both are viable ways to manage DLManagedTensor lifetime, I am asking that docs do not mandate 2 over 1.

I agree they do, but getting the mix right is a bit tricky, although probably possible.

It may help to split it out into what you have to do as a producer and as a consumer.

The producer must always export a new DLManagedTensor and a new capsule, because the consumer may rename the capsule and steal ownership. (To me, violating this seems tempting if you do not use the renaming option for consumers.)

Also, a producer must set up the capsule so that it calls DLManagedTensor.delete if and only if its name is dltensor.

The consumer must either:

• rename the capsule (transferring ownership) and then call the deleter when done. (the capsule itself is irrelevant and may be deleted as soon as it is renamed)
• ensure that it is the "only" consumer and keep ownership with the capsule, e.g. by only ever accepting capsules returned by __dlpack__(). (The "only" could be violated in certain cases.)

---

However, while I agree that the second thing should work and is simpler. It is tricky!

For example, if, say pytorch accepts a bare capsule, and you expose myobj.base to be the capsule, then pytorch.from_dlpack(myobj.base) would steal your ownership!

And because I think it is legal to import from the capsule directly (as in some libraries probably may be doing it), you would have to add another requirement: If you do not rename the capsule, you must ensure the the capsule is not available through public API. And that last part is tricky...

Of course you could also hope that nobody does pytorch.from_dlpack(myobj.base) or that nobody allows importing the capsules explicitly.

[oleksandr-pavlyk]

@seberg Thank you for taking the time to explain the issue, which I understand better now. I will try to improve on the data_interchange.md to explain what the consumer/producer are expected to do.

[oleksandr-pavlyk]

@seberg I have pushed changes to data_interchange.md in #338 per discussion in this ticket. The spec continues to mandate renaming, but it now explicitly states why it does so and stipulates the need to create another object to transfer ownership of the managed tensor to.