Skip to content

Add macOS arm64 job to CI - #4194

Open
kavyawadhwa134 wants to merge 2 commits into
openmc-dev:developfrom
kavyawadhwa134:macos-ci
Open

kavyawadhwa134 wants to merge 2 commits into
openmc-dev:developfrom
kavyawadhwa134:macos-ci

Conversation

@kavyawadhwa134

@kavyawadhwa134 kavyawadhwa134 commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Description

OpenMC's CI only runs on Linux, so problems on macOS, and on Apple silicon in particular, are only found when users hit them (e.g. #3415). This PR adds a macos job that builds OpenMC on a GitHub-hosted Apple silicon runner (macos-26) and runs the C++ tests with ctest and the Python unit tests with pytest.

The regression tests are left out for now. Their reference results are generated on Linux, and on macOS 21 of them give different results because of differences in the system math library (#3820). They can be added to this job once #3820 is resolved.

This PR depends on #4196: without it, the five tests in tests/unit_tests/test_tally_nuclide_names.py fail on macOS, because the error message OpenMC prints there isn't recognized with libc++.

Summary

  • Matrix over OPENMC_USE_OPENMP=OFF/ON, with Apple clang. For OpenMP, CMake is pointed at Homebrew's libomp through OpenMP_ROOT, so no Homebrew LLVM is needed.
  • Same settings as the Linux jobs where they apply: CMake 3.31, RelWithDebInfo, OPENMC_ENABLE_STRICT_FP=on, Python 3.13, pip install -e .[test,vtk,ci], ncrystal and mcpl, NJOY2016 2016.78, and the cross section data from tools/ci/download-xs.sh, cached with the same key.
  • Coverage isn't collected on macOS.
  • The job is not added to ci-pass, so it reports on PRs without blocking merges. It can be made required later.

Design

.github/workflows/ci.yml, job macos:

  • HDF5_ROOT is set to the Homebrew prefix, overriding the workflow-level /usr.
  • NJOY is built as in tools/ci/gha-install-njoy.sh, but with gfortran-15 from the runner and without -Dstatic=on, since macOS doesn't support fully static executables.
  • OpenMC is built in build/ and installed with sudo cmake --install build, as on Linux.
  • The pytest call is the one from tools/ci/gha-script.sh without tests/regression_tests, with a comment pointing to Regression Tests Not Portable Between Different Compilers and Systems #3820.

Commits

  1. Add macOS arm64 build and C++ test job to CI. Build and ctest only, the first version of this PR.
  2. Run the Python unit tests in the macOS CI job. Adds Python, NJOY and the cross section data, as suggested in review.

Testing

Prepared with help from Claude Code (Opus 5.5, medium effort).

Summary by CodeRabbit

  • Chores
    • Expanded automated checks to run on Apple Silicon macOS, covering builds with OpenMP enabled and disabled.
    • The checks now run CTest and selected Python test suites, along with the required setup and cross-section data download. Regression tests remain excluded from this run.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 48fe5cf7-8737-489c-bbf1-e638073ae517
📥 Commits

Reviewing files that changed from the base of the PR and between d69d001 and 4bd6270.

📒 Files selected for processing: 1
  • .github/workflows/ci.yml

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The CI workflow adds a source-change-gated macOS 26 job. It builds and installs NJOY and OpenMC, downloads cross sections, and runs CTest and selected pytest suites with OpenMP disabled and enabled.

Changes: macOS CI coverage

Layer / File(s) Summary
Add macOS build and test matrix
.github/workflows/ci.yml
The job installs dependencies, builds NJOY and OpenMC with strict floating-point mode, downloads cross sections, and runs CTest and selected pytest suites for both OpenMP settings.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: paulromano

Merge Risk: ⚪ Minimal · up to 4bd62

This adds non-blocking macOS validation for both OpenMP modes. No actionable merge-blocking risk is established; it is ready for normal checks.

Security architecture risk: 🔵 Low · up to 4bd62

The change adds CI coverage without adding deployment, publishing, or explicitly elevated token permissions. It carries existing remote-data trust assumptions onto another platform. Archive authenticity, cache recovery behavior, and effective token permissions remain incompletely established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The additional exposure is the macOS build-and-test execution and its OS-specific data cache. The new job does not itself provide a publishing path to production. Compromised archive contents could affect extracted runner files and test inputs, but archive contents, effective credentials, and cache access policy were not verified.

Trust Boundaries and Controls

  • observed — Fixed HTTPS archive URLs feed wget output directly into tar extraction under the runner home directory. The script defines no digest or signature check. This remote-content-to-filesystem trust assumption already existed at the PR base; the new caller extends its use to macOS rather than introducing the downloader condition.

Resilience and Maintainability Implications

  • inferred — Sharing one macOS data-cache key across both OpenMP variants is consistent with their identical downloader and expected paths; it does not establish a live-filesystem race. Cache producer selection, failed-job saves, interruption recovery, and restored-data provenance still depend on unverified external behavior.

Hardening Proposals

  • proposed — As shared CI hardening, verify archives against trusted digests before extraction, validate staged directory contents before promotion, and cache only completed data trees. Explicit least-privilege token permissions would also make the execution boundary less dependent on external defaults.
Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely identifies the main change: adding a macOS ARM64 CI job.
Description check Passed The description provides the motivation, implementation details, testing results, limitations, dependencies, and AI assistance disclosure. It omits the template checklist and structured issue and AI-a…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@GuySten

GuySten commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

I think #3820 should be resolved first.

@kavyawadhwa134 kavyawadhwa134 changed the title Add macOS arm64 build and C++ test job to CI Add macOS arm64 job to CI Oct 14, 2026
@kavyawadhwa134

Copy link
Copy Markdown
Contributor Author

@GuySten thanks, agreed. I've changed this PR to run only the C++ tests and the unit tests on macOS for now, and left the regression tests out until #3820 is resolved. The unit tests all pass on macOS with #4196.

I ran the commits of this PR together with #4196 and #4197 through the full CI on my fork, and everything passes, including both macOS jobs: https://github.com/kavyawadhwa134/openmc/actions/runs/38577581256

I also tested jtramm's CORE-MATH branch on macOS and Linux runners. The macOS regression differences go away there; details are on #3820.

@kavyawadhwa134

Copy link
Copy Markdown
Contributor Author

The two macOS failures here are the five tests in tests/unit_tests/test_tally_nuclide_names.py, which #4196 fixes (everything else passes: ctest 14/14, 1310 Python tests). With #4196 and #4197 applied, the full CI including both macOS jobs is green: https://github.com/kavyawadhwa134/openmc/actions/runs/38577581256

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants