Skip to content

ci: build with Metal and run the tests on pull requests - #22

Merged
Gajesh2007 merged 3 commits into
mainfrom
ci/fork-tests
Sep 28, 2026
Merged

Gajesh2007 merged 3 commits into
mainfrom
ci/fork-tests

Conversation

@davidtai

@davidtai davidtai commented Sep 25, 2026 •

Copy link
Copy Markdown

Proposed changes

This PR adds the Fork Tests workflow (.github/workflows/fork-tests.yml). It builds the fork with Metal and runs the C++ and Python tests on a GitHub-hosted macos-26 runner. It runs on pull requests, on pushes to main and on manual dispatch.

The job metal-build-and-test uses the existing actions in .github/actions:

  1. setup with toolkit: metal. It installs openmpi, uv, Python 3.12, cmake and ccache. It gives the CMake arguments -G Ninja -DMLX_BUILD_METAL=ON -DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DMLX_BUILD_CUDA=OFF.
  2. build-macos with these arguments and macos-target: 26.2. It builds the Python wheel and the C++ tests in Debug mode. The runner has macOS 26. 26.2 is the lowest target for which CMake builds all Metal kernels.
  3. test-macos with toolkit: metal. It installs tensorflow and runs ./build/tests/tests. It runs the ring distributed test with 8 processes. It runs the Python tests from the built wheel. It builds and tests the example extension.

test-macos does not run the MPI distributed test for metal. The action runs that test for the cpu toolkit only, because of an openmpi bug (openpmix/prrte#2416).

Changes to .github/actions/setup/action.yml:

  • Pin astral-sh/setup-uv and actions/cache to full commit SHAs. The organization blocks unpinned actions, also inside composite actions (see fix(ci): Pages deploy failed on org action policy — bump upload-pages-artifact to v5 (SHA-pinned nested uploader) #20). The versions do not change.
  • Replace hendrikmuhs/ccache-action with brew install ccache and actions/cache (v5.1.0). The organization blocks hendrikmuhs/ccache-action, because its creator is not verified and it is not on the allow-list. The ccache directory ($GITHUB_WORKSPACE/.ccache) and the ccache settings (maximum size, compression, compiler check) do not change. The three ccache steps run on macOS only.
  • The cache key is ccache-<ccache-key>-<OS>-<arch>-<toolkit>-<commit SHA>, and the prefix without the SHA is the restore key. fork-tests.yml sets ccache-key: fork-tests, which upstream does not use. Pull requests only restore the cache. Pushes to main restore the cache, and actions/cache saves it at the end of the job, only when the job passes. Every push to main saves a new entry, so a push that changes only python/ or tests/ also refreshes the cache.
  • Print the Xcode and Swift versions in the log.

The job has timeout-minutes: 60. The measured job takes 22 minutes.

fork.yaml moves .github/actions/setup/action.yml from the upstream v0.32.2 section to "Fork tooling", and adds fork-tests.yml to "Fork tooling".

This PR does not change build_and_test.yml.

Runner probe result

The first run (https://github.com/Layr-Labs/mlx/actions/runs/36170228575) stopped at the setup step, because the organization policy blocked hendrikmuhs/ccache-action. This PR now replaces that action.

The run with the build and the tests: https://github.com/Layr-Labs/mlx/actions/runs/36172106158 (failure: one C++ test).

  • Runner label: macos-26. Image macos-26-arm64, version 20260907.0351.1. CPU: Apple M1 (Virtual), 3 cores.
  • macOS: 26.6.2 (25G83).
  • Xcode: 26.6 (17F113). Metal toolchain: 17F109. Swift: 6.3.3 (swiftlang-6.3.3.1.3).
  • Build minutes: 14.6 with an empty compile cache (wheel 7.8, C++ 6.8). CMake found Metal version 400 and built the NAX kernels. The whole job took 17 minutes.
  • Metal C++ tests executed: yes. 290 test cases: 289 passed, 1 failed. 3853 assertions: 3853 passed.
  • Python tests executed: no. test-macos stops at the first group that fails, and the C++ group failed. The ring distributed test and the example extension did not run.

The failed test:

tests/gptoss_mxfp4_tests.cpp:33:
TEST CASE:  gptoss mxfp4 prefill tiles preserve expert boundaries and partial rows
ERROR: test case THREW exception: [metal::Device] Unable to load function mxfp4_gather_qmm_rhs_nt_float_gs_32_b_4_bm_32_bn_32_bk_32_wm_2_wn_2
Function mxfp4_gather_qmm_rhs_nt_float_gs_32_b_4_bm_32_bn_32_bk_32_wm_2_wn_2 was not found in the library

This is a defect in the fork, not a limit of the runner. The Metal device works: the other 289 test cases passed on it. The test sets MLX_GPTOSS_MXFP4_PREFILL_TILE=m32n32k32. quantized.cpp then asks for a gather_qmm_rhs kernel with the tile bm 32, bn 32, bk 32, wm 2, wn 2. fp_quantized.metal instantiates gather_qmm_rhs only with the tile bm 16, bn 32, bk 32, wm 1, wn 2. A build without MLX_METAL_JIT loads kernels only from mlx.metallib, so it cannot find the kernel. PR #14 added the tile option and the test. This PR keeps the test in. #24 adds the missing kernels to fp_quantized.metal and merges into main first.

The same job with the fix of #24 on this branch: https://github.com/Layr-Labs/mlx/actions/runs/36174742752 (success, 22.0 minutes).

  • Build minutes: 13.5 with an empty compile cache (wheel 6.8, C++ 6.7).
  • Metal C++ tests: 290 of 290 test cases passed. 3865 of 3865 assertions passed.
  • Ring distributed test: 8 processes ran 13 tests each. All passed.
  • Python tests: 875 tests ran in 292 seconds and passed. 52 tests were skipped by skip conditions in the test sources: 45 need PyTorch, which the action does not install; 3 are too slow for CI; 1 uses too much memory for CI; 2 are for CUDA only; 1 needs different head and value dimensions.
  • Example extension: it built, and its test passed on the CPU and the GPU.
  • No test group is left out. The Metal device of the hosted runner runs all the test groups of test-macos.

The job on the head commit 0a4b609b (ccache steps on macOS only, cache key by commit, timeout 60 minutes): https://github.com/Layr-Labs/mlx/actions/runs/36187704068 (failure: the same C++ test, because #24 is not in this branch yet).

  • The three ccache steps ran on macOS. The restore found no cache, because no push to main has saved one yet.
  • Build minutes: 11.9 with an empty compile cache (wheel 6.1, C++ 5.9).
  • Metal C++ tests: 290 test cases, 289 passed, 1 failed (the prefill tile test).

Test plan

CONTRIBUTING checklist

  • Formatting: pre-commit ran on the head commit. The Check Lint job passed in run https://github.com/Layr-Labs/mlx/actions/runs/36187703961.
  • Tests: this PR adds no library code. The workflow runs in the Test plan are the test.
  • Benchmarks: this PR adds no library code, so it cannot change performance. No benchmark is needed.

Dependencies

Merge order:

  1. ci: remove the upstream workflows that cannot run in this fork #23 at any time.
  2. fix(metal): build the GPT-OSS MXFP4 prefill tile into the metallib #24 into main.
  3. Update this branch from main. If ci: remove the upstream workflows that cannot run in this fork #23 is already in main, delete the three fork.yaml lines that the conflict names: .github/actions/build-wheel/action.yml, .github/actions/setup/action.yml and .github/actions/test-linux/action.yml in the upstream v0.32.2 section.
  4. Merge this PR after a green Fork Tests run on its own head.

#25 (coverage) has this branch as its base. It runs again after the update in step 3.

  • ☐ I understand it is strictly prohibited to use AI to write PR description
  • AI usage disclosure: this change and this description were written by Claude (Opus 5.5) under David Tai's direction and review.

🤖 Generated with Claude Code

Add the Fork Tests workflow. It builds the fork with Metal on a
GitHub-hosted macos-26 runner. It runs the C++ and Python tests with
the existing setup, build-macos and test-macos actions.

Pin the actions inside the setup action to full commit SHAs. The
organization blocks unpinned actions, also inside composite actions.

Describe both files in the Fork tooling section of fork.yaml.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The organization blocks hendrikmuhs/ccache-action. Install ccache with
Homebrew and keep its files with actions/cache. The cache directory and
the ccache settings do not change. The cache key adds a hash of the mlx
sources. Pull requests only restore the cache. Pushes to main also save
it.

Print the Xcode and Swift versions in the setup log.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Run the ccache install and cache steps only on macOS, where Homebrew
installs ccache. End the cache key with the commit SHA and keep the
prefix restore key, so every push to main refreshes the cache. Say that
actions/cache saves only at the end of a job that succeeds, and only on
pushes to main. Lower the job timeout from 150 to 60 minutes. The
measured job takes 22 minutes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jonathan308

Copy link
Copy Markdown

Review follow-up for head 0a4b609: before approval, please update this branch after #23/#24 land, resolve the documented fork.yaml overlap, and run Fork Tests on the resulting head.

The current run (https://github.com/Layr-Labs/mlx/actions/runs/36187704068) fails in gptoss_mxfp4_tests.cpp because mxfp4_gather_qmm_rhs_nt_float_gs_32_b_4_bm_32_bn_32_bk_32_wm_2_wn_2 is missing. #24 supplies the missing AOT instantiations. The earlier combined run is useful evidence, but please link a green run for the updated candidate, including the later Python/distributed/extension steps that the current failure prevents from running.

This is a dependency/integration validation request, not a request to weaken or skip the failing test.

@davidtai

Copy link
Copy Markdown
Author

Reply written by Claude (agent) on David Tai's behalf.

Agreed. The merge order in the body is #23, then #24 into main, then this branch updated from main with the three-line fork.yaml overlap resolved, then a green Fork Tests run on its own head before merge, including the Python, distributed and extension steps. The current red is the one C++ case that #24 fixes; the test is not weakened or skipped.

@Gajesh2007
Gajesh2007 merged commit 08d9d82 into main Sep 28, 2026
14 of 15 checks passed
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