Repository navigation
update: reconcile Metal nightly fix with the current MLX pin - #2138
Merged
Merged
Conversation
inureyes
added
status:review
inureyes
added a commit
that referenced
this pull request
Oct 6, 2026
This was referenced Oct 6, 2026
`GatherQMM::eval_gpu` passes `x.size() / K` to `gather_qmm_rhs` as its row count. That is the row count of x before `gather_qmm_rhs` broadcasts it against the indices, so when one activation row is gathered against B sorted slots (M == 1, B >= 16, B / E >= 4) the grid and the kernel row bound cover one row and rows 1..B-1 of the freshly allocated output are never written. That is the nightly failure on the M1 Ultra runner since 2026-09-30: the `SortedSharedActivation` case of `mxfp4/mxfp8_gather_qmm_matches_host_reference` reads uninitialized memory (relative L2 7.4e32, non-finite values), while the same case passes on MLX's CPU backend in the same run. The overlay now passes `B * M`, which equals the old value whenever x is already expanded (every `gather_sort` caller, so production MoE paths are unchanged) and covers the NAX variant too. Upstream ml-explore/mlx main carries the same line. Not verified on Metal: this host is CUDA, where the overlay is not compiled. Refs #1599
inureyes
force-pushed
the
fix/issue-1599-metal-gather-qmm-rhs-rows
branch
from
October 7, 2026 14:34
838936a to
2520219
Compare
Member
Author
|
Metal validation passed on Apple M1 Ultra (128 GB), macOS 27.0 (26A428), full Xcode 27 RC, after rebasing this PR onto main
This satisfies #2158 item 4's local gate. Keep #1599 open until a post-merge Metal nightly |
inureyes
marked this pull request as ready for review
October 7, 2026 14:39
Keep the broadcast-row Metal gather fix and make split-MTP staging tests follow canonical macOS paths and the host filesystem's invalid-byte filename support. Filesystems that reject the name must now return the same I/O error without leaving staging residue. Update the bilingual report with the exact historical M1 evidence and current Linux build, focused-test, contract, and independent-review results. A fresh Metal nightly remains required before #1599 closes. Refs #1599
Include the merged Qwen3 Next prefill contract update before validating the Metal correction against the current default branch. The tested Metal kernel and portability test sources remain unchanged.
…gather-qmm-rhs-rows # Conflicts: # src/lib/mlx-cpp/patches/mlx/backend/metal/quantized.cpp
Normalize local cooperative-tensor operand types before passing them to the MPP destination factory. Register the pinned-source overlay so the new MLX gated-delta kernel compiles under Xcode 27.
Carry the upstream Apple copyright in the whole-file overlay so the repository license contract recognizes its source provenance.
This was referenced Oct 14, 2026
Integrate the mlxcelverse overlay migration and subsequent main fixes into PR #2138. Resolve the sole location conflict by relocating the Xcode 27 gated-delta overlay under mlxcelverse while preserving the Metal B * M gather correction and split-MTP portability fixes. Validation: focused Metal MXFP tests pass 5/5; focused split-MTP portability tests pass 2/2; license, shell and Rust MLX pin parsers, formatting, and diff checks pass. The full candidate Metal nightly remains required. Refs #1599
4 tasks
Attribute the whole-file MLX NAX overlay in its preserved header, NOTICE, and README, including the upstream MIT terms and web-accessible source links. Refresh the bilingual technical report with the current-main integration, exact-source green Metal nightly, and fresh compatibility-matched Qwen3-30B release A/B evidence. Validation: license-header check, formatting, diff check, focused Metal and split-MTP tests, candidate nightly 38582194425, and byte-identical real-model A/B output. Refs #1599
inureyes
added a commit
that referenced
this pull request
Oct 14, 2026
## Summary Advance MLX from `59d600b5` to captured upstream main `243bdf09` (111 commits, 312 files). Reconcile whole-file overlays to retain mlxcel's numerical behavior and runtime controls without overwriting upstream fixes. - Rebase 11 CPU/CUDA/Metal overlays, retain 17 unchanged upstream-backed files and two helpers, and retire two overlays covered by upstream #4514 and #4554. Preserve CUDA chunking, gathered GEMM, dtype controls, and the #1599 Metal `B * M` correction; use upstream #4594 for Xcode 27. - Retarget ROCm while preserving fork `75915908`. Adapt move-only allocation ownership, borrowed-storage donation safety, compiled callbacks, SDPA sinks, gated-delta fallback, floor division, and odd inverse FFT. Add actual-header host ownership regressions; these do not prove HIP completion ordering. - Fix the new CUDA RHS route's device argument and add aligned MXFP4/MXFP8 reference coverage. Retain reviewed macOS filesystem test portability fixes from #2138. ## Validation - Pin/parser (27), overlay tooling (29), ownership, provenance, contract gates, Python tooling (123), and formatting pass. - Earlier CUDA compilation and 94 focused numerical executions pass. After the final overlay retirement, 45 numerical executions and two forced RHS runs pass; fresh PTX proves f16/bf16 × MXFP4/MXFP8 route execution. - [Earlier Metal CI on production source `3aeccc2b`](https://github.com/lablup/mlxcel/actions/runs/38492190049): fmt/Clippy pass; 12,285 tests pass, 390 ignored; 51 FFmpeg video tests pass. - Actual GB10 Kimi 48B-A3B 4-bit inference passes three focused tests, five exact short/long controls, and a coherent 58-token explanation. This uses combined source `53473340` = production `3aeccc2b` + independent #2296 mask fix `1fec677b`; that model fix is not included here. CLI SHA-256: `b52d70388e0420582c4f60f3341d570c2afd736685b37e2c0c6e50ed9eaf5fae`. ROCm compilation/runtime and M5-specific execution remain unavailable. Ordinary PR CI is tracked separately; queued checks are not passes. The requested whole-batch CUDA/Clippy and fresh main Metal integration gate remains deferred until the expanded follow-ups are ready. This PR does not independently close #1599, #2158, or #1986. English/Korean technical reports record provenance and remaining limits. Latest main `59974283` is integrated, including the mlxcelverse layout migration. Resolve three rename/delete or relocation conflicts, update ownership-test paths and ROCm record numbering, and preserve main's ROCm/MoE improvements. Post-merge CUDA compilation, contract/ownership/provenance gates, Python tests, and 112 focused test executions pass, including directly forced RHS kernels. CUDA library suites on the merged production source pass 11,247 tests (170 ignored); full CUDA workspace/all-targets Clippy and CPU-only linking pass on final `4befdbc7`. The CUDA block-float archive audit and six quantization tests pass. Upgrade rustls from 0.23.45 to 0.23.46 to resolve RUSTSEC-2026-0346 through 0349; local and hosted cargo-deny pass. [Final Metal CI at `4befdbc7`](https://github.com/lablup/mlxcel/actions/runs/38583586205) passes fmt, Clippy, 12,315 workspace tests (390 ignored), and 51 FFmpeg video tests. Earlier actual Kimi results retain their original source attribution. Closes #2303
Merge main after #2304 while retaining its gathered-row correction and portable tests. Remove the obsolete NAX compatibility overlay now supplied by upstream, and preserve MLX license terms with updated source attribution. The remaining diff from main is documentation only. Production source matches the successful final Metal nightly and CUDA library validation recorded in the reports. Refs #1599
inureyes
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Reconcile the Metal nightly fix with main
9bfabbabafter #2304. Main already contains thegather_qmm_rhsbroadcast-row correction (B * M) and the two split-MTP portability fixes. Retain those implementations, and remove the obsolete Xcode 27 NAX overlay because pinned upstream MLX243bdf09supplies the correction from ml-explore/mlx#4594.The remaining diff from main contains only MLX attribution and bilingual technical reports. Preserve the full MIT terms and expm1f attribution while removing references to the deleted overlay. Rustls 0.23.46 is already included through #2304.
Validation:
4befdbc7passed Metal nightly 38583586205: fmt, Clippy, 12,315 workspace tests (390 ignored), and 51 FFmpeg video tests.44045522: a 96-token prompt and 64 greedy output tokens, two runs per arm with fused MoE disabled, produced identical outputs (cbf1670d0b554a35c029ec4104ae3ecfc8a46e07296d7e33a5f3b2c214a5fa8f). The baseline retained Xcode compatibility and reverted only the row-count fix. This is not a fresh model run on the new pin. Reports preserve the artifact hashes.Review found no remaining blocking implementation, security, or performance issue. M5 execution remains unverified. Keep #1599 open until the final post-merge main Metal nightly passes.
Refs #1599