Skip to content

Compute contiguity from the actual occupied data - #3475

Merged
angeloskath merged 1 commit into
ml-explore:mainfrom
sofinvalery:as-strided
May 8, 2026
Merged

angeloskath merged 1 commit into
ml-explore:mainfrom
sofinvalery:as-strided

Conversation

@sofinvalery

Copy link
Copy Markdown
Contributor

Proposed changes

Compute AsStrided contiguity from the actual occupied data instead of only checking row or column contiguous layouts.

This allows dense non-row/column views, such as dense stride permutations, to be marked contiguous while keeping overlapping, gapped, broadcast-style, and negative-stride views conservative. It also computes a more accurate data_size for as_strided views.

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

@zcbenz zcbenz left a comment

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.

This looks good to me, but I'm not sure about the negative stride part. @angeloskath

@angeloskath angeloskath left a comment

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.

Looks great! Thanks!

@angeloskath

Copy link
Copy Markdown
Member

This looks good to me, but I'm not sure about the negative stride part. @angeloskath

Indeed the contiguous flag could in theory be set even with negative strides, however, the way we use it, it unfortunately cannot be. The contiguous flag would be used mostly for unary ops to select a fast path that doesn't care about indices. Starts at 0 and computes for data_size elements. However if there are negative strides, it should start at low and compute data_size elements.

Anyway, clean improvement in the PR as you also said.

@angeloskath
angeloskath merged commit a1c0b6f into ml-explore:main May 8, 2026
16 checks passed
@sofinvalery
sofinvalery deleted the as-strided branch May 8, 2026 13:13
maxjustus added a commit to maxjustus/resemble-enhance that referenced this pull request Jul 4, 2026
MLX <= 0.31.2 miscomputes data_size for overlapping as_strided views
(ml-explore/mlx#3475), causing occasional Metal page faults when the
views feed rfft/einsum. mx.contiguous is numerically identity; no
measurable perf change on the 5s enhance benchmark.
@BrewTestBot BrewTestBot mentioned this pull request Jul 7, 2026
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants