From 8e87a55aa9900bf1546746cd96da57a1b3ee96a5 Mon Sep 17 00:00:00 2001 From: Seyed Yahya Shirazi Date: Sat, 15 Aug 2026 23:22:26 -0700 Subject: [PATCH 1/2] Add a macOS Apple Silicon CI job Every job was ubuntu-latest, so pamica/tests/mlx_tests/ had never run in CI and the Accelerate BLAS path was untested. The second gap is not theoretical: a test_ng_convergence.py assertion stayed green on Linux through #241 and failed immediately on macOS, because the ~1e-6 trajectory shift moved the stopping iteration off the multiple of mir_step the assertion assumed. macos-26 is the current arm64 image and standard runners are free for public repositories. The job installs the mlx extra and asserts the import rather than letting mlx_tests self-skip into a green no-op. Refs #246. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 44 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a50793d..be19a2c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,6 +113,50 @@ jobs: files: coverage.xml fail_ci_if_error: false + # Apple Silicon coverage (issue #246). The Linux jobs above cannot exercise + # two things the project depends on: + # * pamica/tests/mlx_tests/ -- MLX is Apple-GPU only and is the recommended + # Apple backend (AGENTS.md), so it had never run in CI at all. + # * the Accelerate BLAS numerical path. Summation order differs from + # OpenBLAS enough to move a stopping iteration, which is how a broken + # assertion in test_ng_convergence.py stayed green on Linux through #241 + # and failed immediately on macOS. + # macos-26 is the current arm64 image (macos-14/15 are also arm64; 14 is + # deprecated). Standard runners are free and unmetered for public repos -- + # do not switch to a -large/-xlarge variant, which is billed even here. + test-macos: + name: Test (macOS 26, Apple Silicon) + needs: [lint, typecheck] + runs-on: macos-26 + steps: + - uses: actions/checkout@v4 + - uses: astral-sh/setup-uv@v5 + with: + python-version: "3.12" + enable-cache: true + # --extra mlx is the whole point of this job: without it mlx_tests/ + # self-skips and the run adds nothing the Linux job did not already do. + - run: uv sync --extra mlx + - name: Assert MLX is importable + # A silent skip here would make this job green while testing nothing, + # which is the failure mode it exists to prevent. + run: uv run python -c "import mlx.core; print('mlx', mlx.core.__version__)" + - name: Build native AMICA binary (for engine E2E tests) + # Same recipe as the macos-arm64 target in release-binaries.yml: + # gfortran from brew's gcc, LAPACK/BLAS from the Accelerate framework. + run: | + brew install gcc + LAPACK_LIBS="-framework Accelerate" bash native/build.sh + echo "PAMICA_NATIVE_BINARY=$PWD/native/amica15_shim" >> "$GITHUB_ENV" + # "not slow" matches the Linux job: the slow tests drive the bundled + # x86_64 amica15mac reference binary, which needs Rosetta on an arm64 + # runner. Pointing them at the arm64 binary built above is the follow-up + # tracked in #246, not part of closing the MLX/Accelerate gap. + # --no-cov: the Linux job owns the coverage gate and the Codecov upload; + # a second partial run would report a different total for the same commit. + - name: pytest (excluding slow / Fortran-binary parity tests) + run: uv run pytest -m "not slow" -n auto --no-cov + test-mne: name: Test MNE wrapper (optional [mne] extra) needs: [lint, typecheck] From da41ebd4db3b8a57666b8f1fcdf7d7568a9e2476 Mon Sep 17 00:00:00 2001 From: Seyed Yahya Shirazi Date: Sat, 15 Aug 2026 23:41:16 -0700 Subject: [PATCH 2/2] Assert the Apple GPU; correct the macOS job comments Review findings. Assert mx.default_device().type == gpu, not just that mlx imports. The mlx_tests module is gated on both (test_mlx_backend.py:28-33), so a runner that imported mlx and fell back to CPU would skip all 11 tests and still report success. The hosted macos-26 image does expose the GPU -- all 11 passed on the first run -- but nothing in the job would have caught it if a future image stopped. Add PYTORCH_ENABLE_MPS_FALLBACK=1. The two Linux jobs already set it defensively; this is the only job whose hardware can actually auto-select MPS. Add EXTRA_LDFLAGS=-static-libquadmath so the build really is the release-binaries.yml macos-arm64 recipe the comment claims. Correct the -m 'not slow' rationale: it was attributed to Rosetta and the x86_64 amica15mac, but most slow tests never invoke a binary, and the two that do resolve through PAMICA_NATIVE_BINARY (native/resolver.py:112), which this job sets to the arm64 binary it just built. They are excluded for wall-clock, and enabling them is a real follow-up rather than blocked work. Refs #246. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 43 ++++++++++++++++++++++++++++++---------- pyproject.toml | 6 ++++-- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be19a2c..d42acff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -128,6 +128,11 @@ jobs: name: Test (macOS 26, Apple Silicon) needs: [lint, typecheck] runs-on: macos-26 + env: + # The only job on hardware where torch auto-selects MPS (torch_impl/ + # utils.py picks it whenever no device is pinned and MPS is available), + # so it is the only job where the fallback actually does anything. + PYTORCH_ENABLE_MPS_FALLBACK: "1" steps: - uses: actions/checkout@v4 - uses: astral-sh/setup-uv@v5 @@ -137,24 +142,40 @@ jobs: # --extra mlx is the whole point of this job: without it mlx_tests/ # self-skips and the run adds nothing the Linux job did not already do. - run: uv sync --extra mlx - - name: Assert MLX is importable - # A silent skip here would make this job green while testing nothing, - # which is the failure mode it exists to prevent. - run: uv run python -c "import mlx.core; print('mlx', mlx.core.__version__)" + - name: Assert MLX sees an Apple GPU + # mlx_tests/ is gated on BOTH an mlx import and + # mx.default_device().type == gpu (test_mlx_backend.py:28-33). A hosted + # runner that imported mlx but fell back to CPU would skip all 11 tests + # and still report success -- green while testing nothing, the exact + # failure this job exists to prevent. Assert the device, not just the + # import, so that regression fails here instead of passing silently. + run: | + uv run python -c " + import mlx.core as mx + dev = mx.default_device() + print('mlx', mx.__version__, 'device', dev) + assert dev.type == mx.DeviceType.gpu, f'no Apple GPU: {dev}' + " - name: Build native AMICA binary (for engine E2E tests) # Same recipe as the macos-arm64 target in release-binaries.yml: - # gfortran from brew's gcc, LAPACK/BLAS from the Accelerate framework. + # gfortran from brew's gcc, LAPACK/BLAS from Accelerate, and the static + # libquadmath the release build uses. run: | brew install gcc - LAPACK_LIBS="-framework Accelerate" bash native/build.sh + LAPACK_LIBS="-framework Accelerate" EXTRA_LDFLAGS="-static-libquadmath" \ + bash native/build.sh echo "PAMICA_NATIVE_BINARY=$PWD/native/amica15_shim" >> "$GITHUB_ENV" - # "not slow" matches the Linux job: the slow tests drive the bundled - # x86_64 amica15mac reference binary, which needs Rosetta on an arm64 - # runner. Pointing them at the arm64 binary built above is the follow-up - # tracked in #246, not part of closing the MLX/Accelerate gap. + # "not slow" matches the Linux job, but NOT for the Linux job's reason. + # There it is because the slow tests need the macOS-only reference binary; + # here they are excluded purely for wall-clock (2000-iteration fits). + # Note the two that do shell out (test_ng_backend.py:1096,:1179) would + # work on this runner: they pass binary_path=None, which resolves via + # PAMICA_NATIVE_BINARY (native/resolver.py:112) -- set above -- so they + # would use the arm64 binary this job just built, never Rosetta. Enabling + # them would give CI its first Fortran parity coverage; tracked in #246. # --no-cov: the Linux job owns the coverage gate and the Codecov upload; # a second partial run would report a different total for the same commit. - - name: pytest (excluding slow / Fortran-binary parity tests) + - name: pytest (excluding slow tests) run: uv run pytest -m "not slow" -n auto --no-cov test-mne: diff --git a/pyproject.toml b/pyproject.toml index 6fdfdba..4f3a1ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,8 +84,10 @@ omit = [ # argparse CLI entrypoint: exercised via subprocess, not unit-covered here. "pamica/numpy_impl/cli.py", # Optional MLX backend (Apple Silicon only): its tests require MLX + an Apple - # GPU, so CI (ubuntu, no mlx) cannot exercise it and always measures 0%. It is - # covered locally by tests/mlx_tests/ on Apple hardware (issue #76). + # GPU. The `test-macos` job does now run them for real (issue #246), but it + # runs with --no-cov, so the coverage total still comes only from the ubuntu + # `test` job -- where mlx is absent and this package always measures 0%. The + # omission stays required until coverage is combined across both runners. "pamica/mlx_impl/*", # Optional MNE compatibility layer (issue #139): imports mne, absent from the # base test env, so the base coverage run always measures 0%. Exercised by