Fix Python buffer handling

[steven-johnson]

In the category of "how did this ever work"...

TL;DR: in general, Halide Buffers have the opposite axis ordering from Python/NumPy buffers; in Halide, the most-frequently-varying dimension comes first, while in Python, it comes last. This isn't surprising, though, since Halide's indexing scheme is effectively column-major while NumPy's is row-major.

Anyway: what we should have done was to reverse the order of dimensions when converting to/from Halide Buffers vs Python buffers; instead, we kept the same order, then jumped thru hoops to rearrange buffers to fit this setup. This PR does the appropriate axis reordering, fixing the apps and tests as needed.

It also adds some helper code for image reading and writing; by default, we use imageio for this, but imageio ~always wants RGB/RGBA images to be interleaved (vs the planar that Halide prefers). So, I added the halide.imageio package, that has wrapper functions to quietly convert to/from planar as needed.

Needless to say, this change is likely to break existing code that is using 3d buffers in Halide, but I think it's the right long-term thing to do.

Opinions greatly welcomed here.

[abadams]

nit: remove "for better vectorization" here and below.

Yes, we want to vectorize across the most-rapidly-varying dimension, which is the first in Halide's convention, but we could vectorize across the most rapidly varying dimension no matter where it showed up in the arg list.

[steven-johnson]

Done.

[abadams]

This makes sense to me. Halide is by convention col-major, and numpy is by convention row-major, though both support other layouts. By default you would expect my_halide_func[x, y] == my_halide_buffer[x, y] == my_numpy_array[y, x]

This makes it in keeping with the C backend, for which buffer(x, y) == c_array[y][x]

Would be curious to hear from @jiawen as an outside sanity check if he has a minute to weigh in.

[jiawen]

I have similar code at Adobe that also uses Pybind11 - but it's for @dsharlet's array instead of Halide buffers. I'll get around to open sourcing it at some point.

The API I ended up deciding on is split into two parts: generic ndarrays, and images, which have special treatment.

For ndarrays:
On the PyBind side, to construct a Halide::Runtime::Buffer, I pass in a py::buffer (or py::array_ for the strongly-typed version) along with a bool reverse_axes = true. I don't have a separate bool make_copy parameter because for nda has an explicitly non-owning array_ref class. There is a similar bool reverse_axes for constructing instances of np.ndarray.

Here's the abstraction: most Python code does not call this directly. I instead expose a separate Python interface with make_ndarray(src: NativeArrayRef, reverse_axes = True, copy = False). It does what you expect.

The idea is that reversing axes is the common case that preserves the expected strides. In the default allocation in both np and Halide (sans slicing, cropping, etc), the indexing convention reflects those strides (innermost last in Python, innermost first in Halide).

For images:

It's easier to paste the code than describe what it does:

def make_ndarray(im_ref, copy=False):
  """Makes an ndarray that is a view or a copy of an nda.ImageRef's contents.

  Args:
    im_ref: the source "nda.ImageRef". It can be Chunky, Planar, or generic (no
      prefix) and the datatype must be one of the supported datatypes.
    copy: whether the returned ndarray is a view or a copy.

  Returns:
    An ndarray that is a view or a copy of the same memory as `im_ref`.
  """
  im_np = np.array(im_ref, copy=copy)
  return np.transpose(im_np, (1, 2, 0))

im_ref is an opaque "image ref" - a handle to a Halide::Runtime::Buffer in your case. It's indexed in C++ as (x, y, c) as usual but there is no Python API for it. To access the bytes, call image.make_ndarray on it to get a view or a copy of the data. Since you're calling image.make_ndarray instead of array.make_ndarray, it does the image-specific 3-axis transpose rather than reversing axes.

Effectively, the approach I chose was "when converting, make it so that in the target language, the indexing is using the usual conventions).

[jiawen]

I highly recommend returning a copy in either case. Otherwise you'll have odd behavior if the caller relies on the copy behavior but things start mutating when later loading in a different file that's already interleaved.

[jiawen]

I got annoyed enough at our own code to start naming these things interleaved_copy().

[steven-johnson]

Otherwise you'll have odd behavior if the caller relies on the copy behavior

Good catch!

[steven-johnson]

PTAL

[steven-johnson]

I have similar code at Adobe that also uses Pybind11 - but it's for @dsharlet's array instead of Halide buffers. I'll get around to open sourcing it at some point.

(Threadjack: as I've mentioned in an issue elsewhere, we really need to add support for GPU-backed buffers in our python bindings (perhaps via DLPack?), but it didn't make the cut for Halide 15 because we had to stop somewhere; when work starts on that, def want to hear your thoughts on the matter)

[steven-johnson]

@jiawen So, if I understand correctly: your code doesn't do implicit conversions from YourEquivalentOfHalideBuffer into ndarray (or py::_bufferinfo, presumably); instead, you require an explicit conversion using make_ndarray() or similar (which allows for flexibility of conversion)?

If that's the case... I think I like it (would need to experiment more to be sure)... but I suspect it's too late in the game to adopt this for Halide 15. (We intend to release once we are confident the Python-related stuff is stable, and that the Python tutorials / examples are up to date).

That said, I wonder if it would be appropriate to do something like:

• for the hl.Buffer(pybuffer_info, name) ctor, add reverse_axes = true as an optional trailing arg
• add an explicit conversion method to hl.Buffer() that offers the reverse_axes and copy arguments as an option. (It probably wouldn't be make_ndarray() as our Python bindings don't currently have a hard dependency on NumPy, but conversion to py::buffer or py::bufferinfo ought to be adequate).

[steven-johnson]

Update: I added the reverse_axes option in s separate, dependent PR, so that it would be easier to review; see #7127 and LMK

[steven-johnson]

Thanks -- want to review #7127 as well?