Skip to content

Conversation

steven-johnson
Copy link
Contributor

@steven-johnson steven-johnson commented Oct 26, 2022

This adds some options to the changes in #7127 to allow for easier backwards compatibility with previous versions of the Python bindings:

  • The default when creating hl.Buffer from a Python buffer is to reverse the axes. There is now an optional named arg to that ctor that you can specify reverse_axes = False if you want the behavior of pre-Halide-15 (ie, don't reverse).
  • The default when creating np.array, np.ndarray etc from an hl.Buffer is to reverse the axes. There is now a new method on hl.Buffer, reverse_axes(), which simply returns another Buffer that is a view onto the same memory, but with the order of the axes reverse. This allows you to keep the behavior of pre-Halide-15 (ie, don't reverse) by doing something like np.array(halidebug.reverse_axes(), ...).

@steven-johnson steven-johnson added backport me This change should be backported to release versions release_notes For changes that may warrant a note in README for official releases. python Issues related to Halide/Python interop labels Oct 26, 2022
@steven-johnson
Copy link
Contributor Author

(LMK if this would be easier to review if I went ahead and combined it with #7125)

@steven-johnson steven-johnson merged commit 25ac02e into srj/pybuf-axis-ordering Oct 27, 2022
@steven-johnson steven-johnson deleted the srj/pybuf-axis-ordering-2 branch October 27, 2022 22:43
steven-johnson added a commit that referenced this pull request Oct 28, 2022
* Fix Python buffer handling

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.

* Update PyBuffer.cpp

* -"for better vectorization"

* public halide.imageio utilities should copy() buffers

* PEP8

* Update imageio.py

* Update imageio.py

* add 'reverse_axes' options to Buffer conversions (#7127)

* add 'reverse_axes' options to Buffer conversions
steven-johnson added a commit that referenced this pull request Oct 28, 2022
* Fix Python buffer handling

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.

* Update PyBuffer.cpp

* -"for better vectorization"

* public halide.imageio utilities should copy() buffers

* PEP8

* Update imageio.py

* Update imageio.py

* add 'reverse_axes' options to Buffer conversions (#7127)

* add 'reverse_axes' options to Buffer conversions
steven-johnson added a commit that referenced this pull request Oct 28, 2022
* Require Python 3.8+ in CMake build (#7117)
* Add range-checking to Buffer objects in Python (#7128)
* Fix Python buffer handling (#7125)
* add 'reverse_axes' options to Buffer conversions (#7127)
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* Fix Python buffer handling

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.

* Update PyBuffer.cpp

* -"for better vectorization"

* public halide.imageio utilities should copy() buffers

* PEP8

* Update imageio.py

* Update imageio.py

* add 'reverse_axes' options to Buffer conversions (halide#7127)

* add 'reverse_axes' options to Buffer conversions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport me This change should be backported to release versions python Issues related to Halide/Python interop release_notes For changes that may warrant a note in README for official releases.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants