Skip to content

Fix Device and Stream lexicographic ordering - #4086

Merged
zcbenz merged 1 commit into
ml-explore:mainfrom
wgu9:fix-device-stream-ordering
Aug 9, 2026
Merged

Fix Device and Stream lexicographic ordering#4086
zcbenz merged 1 commit into
ml-explore:mainfrom
wgu9:fix-device-stream-ordering

Conversation

@wgu9

@wgu9 wgu9 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

Fixes #4083.

This makes Device and Stream comparisons lexicographic, so they satisfy strict weak ordering when the first and second fields sort in opposite directions.

I also added regression coverage for asymmetry, transitivity, and std::set ordering for both types.

Testing

  • pre-commit run --files mlx/device.h mlx/stream.h tests/device_tests.cpp
  • cmake -S . -B build -DMLX_BUILD_METAL=OFF -DMLX_BUILD_EXAMPLES=OFF -DMLX_BUILD_PYTHON_BINDINGS=OFF
  • cmake --build build --target tests -j 8
  • ctest --test-dir build --output-on-failure -j 1 (248/248 passed)

Checklist

  • 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)

@wgu9
wgu9 force-pushed the fix-device-stream-ordering branch from adb0455 to 10a2170 Compare August 9, 2026 02:54

@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 is a very beautiful fix, thanks!

@zcbenz
zcbenz merged commit e2aa0d0 into ml-explore:main Aug 9, 2026
28 checks passed
CharlieTLe added a commit to CharlieTLe/mlx-swift that referenced this pull request Aug 9, 2026
ml-explore/mlx#4083 was fixed by ml-explore/mlx#4086, which uses `std::tie`
rather than the hand-rolled comparison this patch carried. The fix is not in a
release yet -- v0.32.0 is still the newest tag and it landed after -- so the
patch has to stay for now.

Making it byte-identical to the upstream commit means `patch-mlx.sh` sees the
patch as already applied once mlx carries the fix, so it no-ops rather than
failing. Verified against e2aa0d096. Note in vendor-README.md what to delete
when mlx is next bumped to a release containing it.
@wgu9
wgu9 deleted the fix-device-stream-ordering branch August 9, 2026 23:02
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.

[BUG] Device::operator< and Stream::operator< are not strict weak orderings

2 participants