-
Notifications
You must be signed in to change notification settings - Fork 1.1k
add 'reverse_axes' options to Buffer conversions #7127
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
Merged
steven-johnson
merged 5 commits into
srj/pybuf-axis-ordering
from
srj/pybuf-axis-ordering-2
Oct 27, 2022
Merged
add 'reverse_axes' options to Buffer conversions #7127
steven-johnson
merged 5 commits into
srj/pybuf-axis-ordering
from
srj/pybuf-axis-ordering-2
Oct 27, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(LMK if this would be easier to review if I went ahead and combined it with #7125) |
abadams
approved these changes
Oct 27, 2022
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
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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds some options to the changes in #7127 to allow for easier backwards compatibility with previous versions of the Python bindings:
hl.Buffer
from a Python buffer is to reverse the axes. There is now an optional named arg to that ctor that you can specifyreverse_axes = False
if you want the behavior of pre-Halide-15 (ie, don't reverse).np.array
,np.ndarray
etc from anhl.Buffer
is to reverse the axes. There is now a new method onhl.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 likenp.array(halidebug.reverse_axes(), ...)
.