Get raw Runtime::Buffer from Buffer in Python rather than use PyBuffer #8682
Conversation
alexreinking
added
python
|
In the abstract, this approach might resolve #6849 as our buffers do support bfloat16. |
|
I'm not totally seeing the logic behind the many layers of wrapping:
I wonder if adding a virtual dtor to (3) and having (1) wrap (3) directly is a better approach. It feels analogous: (2) is a reference-counted wrapper for C++, while (1) is a reference-counted wrapper for Python. But, again, maybe I'm missing something. |
|
I'm not even sure how to extract the code from PythonExtensionGen.cpp as we don't target the Python Stable ABI. I don't think we even can until our minimum Python version is 3.11 since that's when PyBuffer was added to the ABI. As an aside, moving to the Stable ABI would greatly simplify our distribution of binary wheels. On this subject, the latest Ubuntu LTS (24.04/noble) packages Python 3.12... at the latest, we can drop Python <3.11 in October 2026 (in a year and a half) when it goes EOL. |
|
Furthermore, we should probably find a way to throw an exception if a |
Yes, but it's not clear if the standard numpy buffer decriptors support bfloat16 -- they didn't last time I checked (but that was probably ~1 year ago) |
They still don't, but using Halide buffers directly provides an escape hatch. |
This could be tricky because there are going to be contexts where the mins don't matter and it would be annoying to have to explicitly zero them. E.g. saving a buffer to a png file using a routine that expects the buffer protocol, or displaying it in a window. |
The buffer protocol (PyBuffer) does support all the same metadata that Halide::Runtime::Buffer does. In particular, it does not support coordinate offsets (as in set_min). When an object is passed to an AOT Python pipeline, it is now checked for a _get_raw_halide_runtime_buffer method. If it exists, it should return a pointer to a halide_buffer_t whose lifetime is at least that of the object. Fixes #8652
alexreinking
force-pushed
the
python/buffer-mins
branch
from
004b705 to
75d14cc
Compare
|
After discussion at the dev meeting, we will move ahead with merging this. The name |
alexreinking
removed
the
dev_meeting
The buffer protocol (PyBuffer) does not support all the same metadata that
Halide::Runtime::Bufferdoes. In particular, it does not support coordinate offsets (as inset_min).When an object is passed to an AOT Python pipeline, it is now checked for a
_get_raw_halide_runtime_buffermethod. If it exists, it should return a pointer to ahalide_buffer_twhose lifetime is at least that of the object.I don't love this solution as passing around a
uintptr_tsmells fishy (security implications?). But my first few ideas didn't pan out:.cast<Halide::Buffer<>*>to get a reference to the underlying C++ object. This has some more safety checks implemented. But the generated extension doesn't link to pybind11. Changing this would mean adding a dependency.Halide::Bufferobject and usedynamic_castto check type-safety. But the generated extensions don't have access toHalide.h, either.Halide::Runtime::Buffer, since the generated extensions definitely have access to that. But it's not polymorphic (no virtual functions), so it's not compatible withdynamic_castortypeid.Tagging
dev_meetingto advance discussion.Fixes #8652