Repository navigation
Tests: seed the random input of the Python tests - #46
Merged
Merged
Conversation
Diagnostic for a draft PR; the next commit removes it. test_gemm and test_scans draw unseeded random input and fail now and then in Fork Tests. The two new tests run the unchanged upstream tests under fixed seeds (2000 for test_gemm on the CPU, 100 for test_scans) and fail with each failing seed and its worst element against the tolerance. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Upstream tests draw unseeded random input, so each run tests other input, and tests with a fixed tolerance fail now and then. The seed search of the previous commit (Fork Tests run 40118435105) found test_gemm failing for 152 of 2000 seeds on the CPU, always on a near-zero element where atol 1e-8 is below the float32 rounding of the partial sums, and test_scans failing for 4 of 100 seeds on the GPU and 2 of 100 on the CPU (bfloat16 cumsum against the tri-matrix sum). MLXTestCase.setUp now seeds mx.random and np.random with 0 before each test; seed 0 passes both. The search tests are removed. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
davidtai
marked this pull request as ready for review
October 24, 2026 18:24
davidtai
added a commit
that referenced
this pull request
Oct 24, 2026
Brings in mlx #44 and #46 so the CI run of this PR is on a head that contains them. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
jonathan308
added a commit
that referenced
this pull request
Oct 26, 2026
Brings in main at f78f0fa (2026-10-24): the upstream ml-explore/mlx sync to afc03c5 (#38, with its history restored by #42) and the fork fixes #40 and #43-#46. Conflicts and how they were resolved: - mlx/distributed/jaccl/lib/jaccl/mesh.cpp: main's per-wire queue setup (upstream ml-explore#4607, a second cable for the mesh) with this branch's COMPLETION_QUEUE_DEPTH for every completion queue. - mlx/distributed/jaccl/lib/jaccl/mesh_impl.h: main's stride parameters for all_gather and sum_scatter, with this branch's progress-guard calls and stage_send in every send path. - mlx/distributed/jaccl/lib/jaccl/ring_impl.h: this branch keeps its guarded member dispatch_wires (pool-wire errors are reported, the group closes after all wires return). Main moved an unguarded copy into threadpool.h and called it as dispatch_wires(pool_, ...); the ring call sites call the member again. The all_gather slice code is main's (identical to this branch's backport of upstream ml-explore#4443). - quantized_nax.h, fp_quantized_nax.h: main's kernels. Main already keeps the revert of the upstream ml-explore#4352 simdgroup skip (no sg_active) and adds the ragged-K tail bound of upstream ml-explore#4009. - fork.yaml: main's file, plus the JACCL description and section. The upstream backports this branch carried (ml-explore#4443, ml-explore#4557, ml-explore#4558, the scoped_env helper of ml-explore#4474) are now upstream ancestry, so their "not fork work" section and its references are gone. The JACCL section now says that a mesh with more than one wire has one guard per wire. Also: - python/tests/ring_test_distributed.py: main's file. The auto-merge had added test_strict_init_after_fallback twice. - tests/jaccl: the mesh all_gather and sum_scatter calls pass main's new stride argument (equal to the chunk size, as main's own all_reduce_scatter_gather does). Tests: tests/jaccl (simulated verbs, ASan and UBSan): 10 of 10 passed. check_forkdiff.py --upstream-ref upstream/main: 111 changed files, all described, base afc03c5 is the merge-base. clang-format 21: no changes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
jonathan308
added a commit
that referenced
this pull request
Oct 26, 2026
…ectives branch Brings in merge/33 at 187b6c6, which is main f78f0fa (the upstream ml-explore/mlx sync to afc03c5 with its restored history, and the fork fixes #40, #43-#46) merged into the JACCL send-frame and progress-guard branch. This branch had merged main up to ad4745a; everything main added since comes in here. Conflicts and how they were resolved: - mlx/fence.h, mlx/backend/{metal,cuda,no_gpu}/fence.cpp, mlx/transforms.cpp: main's value-based fence API (upstream ml-explore#4552: update returns a value, wait takes it) with this branch's fenced inputs and eager commit. Fence::wait_in_consumer now takes the value of the update it waits for, and FencedInput carries that value, so a fenced input never waits for a later update than its own. The Metal fence keeps the two-word buffer of the fused coherent update, now owned through main's allocator::Data. - Metal fast fence wait: this branch orders the spin with wait_for_all_pending (the pass waits for every pass in flight). Upstream ml-explore#4689 gets the same order by putting the spin in a pass of its own (end_encoding() before it, end_encoding(true) after it). The merge keeps the spin in the current pass and adds a buffer barrier after wait_for_all_pending, so it also follows the earlier kernels of the same pass, as upstream's separate pass does (the same line that the next branch, fix/cluster-metal-fence-wait, adds). A wait adds no encoder boundary; tensor-parallel decode waits for a CPU-stream all-sum about twice a layer. end_encoding(bool wait_in_flight) stays and shares one loop with wait_for_all_pending. - docs/src/usage/environment_variables.rst: both sides' entries (MLX_METAL_FUSED_FENCE_UPDATE, MLX_METAL_FENCED_INPUTS, MLX_CONV_WINOGRAD). - fork.yaml: the merged JACCL branch's file plus this branch's sections (startup readiness, two-rank sealed all-sum, fused fence update, fenced inputs, cheaper custom-kernel launches). The fenced-input section now explains the fence-wait order against upstream ml-explore#4689 and the value-based waits. The "not fork work" backport section is gone (upstream ancestry). Also adapted: - mlx/distributed/jaccl/lib/jaccl/mesh.h: main's mesh has one MeshImpl per wire (upstream ml-explore#4607). The startup wait budget goes to every wire; the two-rank exchange of the streamed sealed sum runs on the first wire. The two-Mac cluster's device matrix has one device per peer, so it has one wire and nothing changes for it. Unchanged by the merge: main's quantized, matmul and SDPA kernels and their dispatch (this branch had no change there beyond what main already carried since #37), main's revert of the upstream ml-explore#4352 simdgroup skip, and this branch's revert of 4d91320 (main does not carry 4d91320; the footprint policy keeps {page, page, 0, 0, page}). Tests (Ultra A, M5 Ultra; Debug build with Metal, 8 jobs): - tests/tests on the GPU: 310 of 316 cases passed. The 6 failures (linalg QR, inverse, Cholesky, pseudo-inverse, eigh, LU) fail the same way on main f78f0fa on this machine. - tests/tests with DEVICE=cpu: 316 of 316 passed. - ctest process_allocation_cap: 15 of 15 passed. - check_forkdiff.py --upstream-ref upstream/main: 137 changed files, all described, base afc03c5 is the merge-base. Co-Authored-By: Claude Opus 5.5 <[email protected]>
jonathan308
added a commit
that referenced
this pull request
Oct 26, 2026
…e fence-ordering branch Brings in merge/35 at 2b5253a: main f78f0fa (the upstream ml-explore/mlx sync to afc03c5 and the fork fixes #40, #43-#46) through the JACCL and cluster-collectives branches, with their conflict resolutions. This branch's tip is d11238f (32 x 32 steel tiles where kernels are precompiled). Conflicts and how they were resolved: - mlx/transforms.cpp: this branch's event fences (a GPU stream never spins on another GPU stream, nor on CPU work that can wait for one) with main's value-based waits (upstream ml-explore#4552). FenceNeed records the value of each of its two updates (fast_value, event_value); every wait uses the value of the update it waits for, fast and event fences alike, and a fenced input gets the fast value. Waits look the fence up with at(), as main does. - mlx/backend/{metal,cuda,no_gpu}/fence.cpp: Fence(Stream, bool fast) next to the value-based wait. The Metal fence-wait barrier was already in the merged collectives branch (the same line), so it merged cleanly. - mlx/backend/metal/quantized.cpp (qmm): this branch's affine steel tiles (MLX_QMM_STEEL_TILE, 64 x 32 / 32 x 64 for prompts) beside main's FP4 half-loader 64 x 64 tile (upstream ml-explore#4709). The modes do not overlap: wide affine tiles keep the "_bm<m>_bk<k>_bn<n>" name, the FP4 tile keeps main's precompiled "_bm_64_bn_64" name. The wide-tile special case is gone: main's affine_qmm_t has has_global_scale before BM, BK, BN, so every transposed qmm now passes (global scale, bm, bk, bn) in main's order. The old call would have instantiated affine_qmm_t<..., bm, bk, bn> into the wrong template parameters. d11238f's kernels_jit_compiled() gate is kept. - mlx/ops.cpp: main's 1-bit affine_quantize branch, with this branch's caller stream on the scales select. - mlx/distributed/sealed.cpp: this branch's __APPLE__ guard around the dispatch include. - tests/CMakeLists.txt: metal_fence_chain beside main's GPU_TEST_SOURCES (renamed from METAL_TEST_SOURCES upstream), which now also lists d11238f's quantized_tile_tests.cpp. - .github/workflows/fork-tests.yml: main's restructured workflow; the bounded Metal fence chain step now runs in the build-cpp job after the standalone allocation cap tests (main has no single build-and-test job any more). fork.yaml merged without conflicts (this branch's sections inside the merged file). The steel-tile section now also lists kernels.h, jit_kernels.cpp and nojit_kernels.cpp: main dropped the "Upstream v0.32.2 sync" section that described them on this branch, and d11238f's kernels_jit_compiled() lives there. Tests (Ultra A, M5 Ultra; Debug build with Metal, 8 jobs): - tests/tests on the GPU: 315 of 321 cases passed. The 6 failures (linalg QR, inverse, Cholesky, pseudo-inverse, eigh, LU) fail the same way on main f78f0fa on this machine: TF32 is on by default on a NAX GPU. With MLX_ENABLE_TF32=0: 321 of 321 (with quantized_tile_tests). - tests/tests with DEVICE=cpu: 321 of 321 passed (with the sealed stream tests). - ctest process_allocation_cap: 15 of 15; metal_fence_chain (fast synch, 8/50/1024 ops a buffer): 3 of 3. - check_forkdiff.py --upstream-ref upstream/main: 144 changed files, all described. clang-format 21: no changes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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
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.
Summary
Upstream Python tests draw their random input from unseeded
mx.randomandnp.random, so each run tests other input. Tests that compare with a reference under a fixed tolerance fail now and then. Two did in our Fork Tests:test_ops.TestOps.test_scans(test_ops.py:2900, float16/bfloat16 cumsum against a tri-matrix sum, rtol 0.02, atol 1e-3): failed with the CPU and the GPU in run 40035016891 attempt 1 (Compute float16 and bfloat16 floor divide in float32 #40 head 842d49a); attempt 2 green on the same tree.test_double.TestDouble.test_gemm(test_double.py:170, float32 8x8 matmul against float64,mx.allclosedefaults rtol 1e-5, atol 1e-8): failed in python-cpu (0) of main run 40090732192 (2668d98; tree = 83dbdb0, which was green).Fix:
MLXTestCase.setUpseedsmx.randomandnp.randomwith 0 before each test. Each test now draws the same input in every run. Tolerances do not change.test_gemm is tolerance, not a stale output
matmul<float>/matmul<double>inmlx/backend/cpu/gemms/cblas.cpp(dispatch_inexact_typesinmatmul.cpp;bnns.cppserves only float16/bfloat16). Both callclear_gemm_outputbeforecblas_sgemm(line 33) andcblas_dgemm(line 77) when beta is 0 (fork section "CPU BLAS gemm clears C when beta is 0").test_scans
The seed search found 4 failing seeds of 100 with the GPU (14, 55, 67, 83) and 2 of 100 with the CPU (14, 67). In each, the bfloat16 cumsum differs from the bfloat16 tri-matrix sum by more than the tolerance, for example seed 14 (GPU): element (4, 22), tri-sum 10.9375, cumsum 11.1875, |diff| 0.25, tolerance 0.22. One bfloat16 ulp at 8 to 16 is 0.0625, and a sequential sum of 32 bfloat16 values can round by a few ulps.
Commits
97d3d774(diagnostic, removed by the next commit):test_gemm_seed_searchandtest_scans_seed_searchrun the unchanged upstream tests under seeds 0..1999 (gemm, CPU) and 0..99 (scans) and fail with each failing seed and its worst element. Fork Tests https://github.com/Layr-Labs/mlx/actions/runs/40118435105: FAILED as intended, python-gpu (0) job 120944935552 (scans), python-cpu (0) job 120944935584 (scans and gemm). Seed 0 passes both tests.9787fed4(fix): seeds inMLXTestCase.setUp; the search tests are removed;test_double.pyandtest_ops.pyare equal to main. ALL GREEN: Fork Tests https://github.com/Layr-Labs/mlx/actions/runs/40119439820 (test_gemmandtest_scanspassed in python-cpu (0) and python-gpu (0)), Build and Test https://github.com/Layr-Labs/mlx/actions/runs/40119439885, Fork Diff Check https://github.com/Layr-Labs/mlx/actions/runs/40119439814.What the seed changes
test_random.test_global_rng, the RNG tests intest_compile.py, thenp.random.seedcalls intest_ops.pyand others) still do; their seed comes aftersetUp.unittest.TestCasedirectly (test_device.TestDefaultDevice,test_extension.TestMetalExtension,test_metal_threadgroup_bounds.ThreadgroupBoundsTests) do not get the seed. None of them draws random input.fork.yaml: section "Seeded Python tests".Draft upstream issue (not opened)
🤖 Generated with Claude Code