Skip to content

python: fix bytes() on non-contiguous arrays - #4449

Merged
zcbenz merged 2 commits into
ml-explore:mainfrom
axiom-of-choice:fix/bytes-non-contiguous
Sep 10, 2026
Merged

python: fix bytes() on non-contiguous arrays#4449
zcbenz merged 2 commits into
ml-explore:mainfrom
axiom-of-choice:fix/bytes-non-contiguous

Conversation

@axiom-of-choice

Copy link
Copy Markdown
Contributor

__bytes__ reads the raw buffer pointer, which for a strided view (slice, transpose, broadcast) points into the parent array, so bytes(x[::2]) dumps neighboring memory instead of the view's values.

Make the array row-contiguous before reading, which is a no-op copy for already-packed arrays. Fixes #4445.

@simeetnayan81 simeetnayan81 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, we need tests to catch these issues with calling bytes on a view. The issue was not caught earlier because test only considered packed array.

We might need these cases:
x[::2]
x[::-1]
m.T (transpose view)
m[:, 1]
broadcast_to
x[1:6] as a packed view with offset

Thanks

Comment thread python/src/array.cpp
"__bytes__",
[](mx::array& a) {
a.eval();
auto c = mx::contiguous(a);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this be better? We might want to save a second call if a is already packed/contiguous :

a.flags().row_contiguous ? a : mx::contiguous(a);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a is not evaluated a.flags().row_contiguous would return garbage so we have to use mx::contiguous.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh alright. Then we may skip this. Thanks.

@zcbenz zcbenz added low priority await response This pull request is waiting for response from the author. labels Sep 2, 2026
@zcbenz

zcbenz commented Sep 10, 2026

Copy link
Copy Markdown
Member

@axiom-of-choice Can you add a test for bytes() non-contiguous arrays?

@axiom-of-choice

Copy link
Copy Markdown
Contributor Author

@axiom-of-choice Can you add a test for bytes() non-contiguous arrays?

@zcbenz Done!

@zcbenz zcbenz removed the await response This pull request is waiting for response from the author. label Sep 10, 2026
@zcbenz
zcbenz merged commit a3af4d1 into ml-explore:main Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: bytes(array) dumps the wrong memory when the array is not packed

3 participants