Build wheels for arm64 macOS

[agriyakhetarpal]

Description

Related to #3462, see there for extended discussion. This PR enables cibuildwheel to use Xcode available on GitHub Actions runners which can cross-compile wheels for macOS with arm64 architectures.

Important

These wheels should be tested on M-series macOS machines, they may be downloaded from this link:
https://github.com/agriyakhetarpal/PyBaMM/actions/runs/6667379053 (updated)

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6851496) 99.58% compared to head (a11bbd5) 99.58%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3465   +/-   ##
========================================
  Coverage    99.58%   99.58%           
========================================
  Files          256      256           
  Lines        20119    20119           
========================================
  Hits         20036    20036           
  Misses          83       83           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[agriyakhetarpal]

I tried this on my end and I am receiving this trace when trying to import the extension module (within pybamm.have_idaklu()):

ImportError: dlopen(/Users/agriyakhetarpal/Desktop/PyBaMM/env/lib/python3.11/site-packages/pybamm/solvers/idaklu.cpython-311-darwin.so, 0x0002): tried: '/Users/agriyakhetarpal/Desktop/PyBaMM/env/lib/python3.11/site-packages/pybamm/solvers/idaklu.cpython-311-darwin.so' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64')), '/System/Volumes/Preboot/Cryptexes/OS/Users/agriyakhetarpal/Desktop/PyBaMM/env/lib/python3.11/site-packages/pybamm/solvers/idaklu.cpython-311-darwin.so' (no such file), '/Users/agriyakhetarpal/Desktop/PyBaMM/env/lib/python3.11/site-packages/pybamm/solvers/idaklu.cpython-311-darwin.so' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64'))

which is confusing since the cibuildwheel documentation mentions that cross-compiling arm64 wheels with Xcode should be as simple as a drop-in addition to CIBW_ARCHS_MACOS. Perhaps this is a bug that I should file on their issue tracker?

Edit: pypa/cibuildwheel#1345 looks related. We would need to adjust setup.py and CMake to handle arm64 architectures. I might take a look into doing this soon since this would not be in the release until next year and shall request reviews sometime later

[agriyakhetarpal]

The newest wheels built on my branch are here: https://github.com/agriyakhetarpal/PyBaMM/actions/runs/6667379053

I tried and tested the import of the extension module on my end, the wheel can be installed correctly and pybamm.have_idaklu() returns True now. 🎉

#3301 removes the file that contains the CMake args and adds them to setup.py directly. I can fix the imminent merge conflict on either branch depending on whichever pull request gets merged earlier.

[agriyakhetarpal]

The question would now be about what combination to provide for the wheels: x86_64 + arm64 or x86_64 + universal2. The universal2 wheel supports both platforms but the binary is double the size of that of a specific architecture and therefore requires more bandwidth and storage capacity. For most cases and for ours, this is completely okay, since our wheel size has been around 3 MB which is very small.

We don't need to provide all three—just two of these three choices would be fine, because pip resorts to downloading a universal wheel only when a platform-specific wheel is not available. Also, universal wheels require pip>=20.3 for installation – the good thing is that Python 3.8+ comes with that version by default. There is some discussion about what version constraints universal wheels can bring up on macOS machines on pypa/cibuildwheel#1333

[kratman]

I am a rather delayed on this review, but I did get an error when importing pybamm with the MacOS ARM wheel:

ModuleNotFoundError: No module named 'pybtex'

After installing that I got an error with anytree and sympy. So I don't think the dependencies are there.

I figure this might change after #3301 is merged.

[Saransh-cpp]

That was a bug in v23.9rc0, but it was fixed in the rc1 release. @agriyakhetarpal could you merge develop here?

I think this PR is independent of the pyproject.toml PR and can be merged before? Please let us know if there is a particular order for merging these PRs.

[agriyakhetarpal]

After installing that I got an error with anytree and sympy. So I don't think the dependencies are there.

Yes, this was a bug and was fixed with #3475. Could you try installing from the wheel file with the "[all]" extras in the meantime while I trigger building another set of wheels on my fork? Edit: I have triggered them here: https://github.com/agriyakhetarpal/PyBaMM/actions/runs/6943261853

I think this PR is independent of the pyproject.toml PR and can be merged before? Please let us know if there is a particular order for merging these PRs.

There is no order as such but I would recommend #3301 getting merged first, since 1. the CMakeBuild.py file here has been directly included in setup.py there, and 2. we will be able to test the creation of these wheels under build isolation. I can rebase this branch after that

[kratman]

The install and import worked on my M2, but idaklu does not seem to be installed with the wheel.

pybamm.have_idaklu()
False

[agriyakhetarpal]

Strange, it works on my end. Could you run the internals of the have_idaklu() function step by step and post the output?

[kratman]

idaklu = importlib.util.module_from_spec(idaklu_spec)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 565, in module_from_spec
  File "<frozen importlib._bootstrap_external>", line 1173, in create_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
ImportError: dlopen(/Users/kratz/anaconda3/envs/testPyBaMM/lib/python3.9/site-packages/pybamm/solvers/idaklu.cpython-39-darwin.so, 0x0002): symbol not found in flat namespace '_IDABBDPrecGetNumGfnEvals'

[kratman]

I was using python 3.9 if that helps.

[agriyakhetarpal]

For very long I tried to reproduce this: across wheels for different Python versions, checked search paths from otool, tried different import mechanisms, etc. And I was finally able to. I think the wheel has now been compiled with the correct architecture, but there has been some issue with either the cross-compilation for SuiteSparse/SUNDIALS or there is a bug in the IDAKLU extension module, because the linker is looking at the right paths and there is no version mismatch between the libraries (see below for more). As far as I can understand there is no restriction on not being able to cross-compile either of these libraries with Xcode. Could you help debug what this symbol is supposed to represent? I don't know enough C++ for this :)

I did trace it down to this line here: https://github.com/pybamm-team/PyBaMM/blob/develop/pybamm/solvers/c_solvers/idaklu/CasadiSolverOpenMP.cpp#L498. The symbol is mentioned as a function in the SUNDIALS documentation for IDA.

Soon after I realised the cause of why it returned True for me earlier in the thread: the .so file has most probably picked up the natively compiled dynamic libraries I already had present in .local/ first rather than the cross-compiled ones bundled inside the wheel – it shouldn't do that though because the hierarchy for looking up runtime paths for a repaired wheel is most definitely altered to avoid this LD_LIBRARY_PATH problem... deleting those and forcing it to use the right ones brought me to the same error.

I'll try the reverse option now, i.e., cross-compiling for x86_64 on my arm64 machine and then testing whether that wheel works as expected on an x86_64 machine.

[agriyakhetarpal]

If cross-compilation doesn't end up working, it might be a good idea to get Cirrus CI for the repository for native compilation. They are freely and readily available and have both macOS arm64 and Linux arm64 runners. SciPy and some other scientific Python libraries are using it: in the case of SciPy it is currently as both a shared resource for PRs' tests and for building wheels for PyPI + nightlies. But if we want to keep its usage to a minimum, our thrice-a-year release cycle we should always be much below the limit for the free plan – tagging @brosaplanella for this. It would be great if this could be added to the agenda for the next developer meeting! It's also pretty easy to write Cirrus CI pipelines for this task owing to the fact that all CI services have inherently similar YAML syntax, so I can volunteer to pick that up if we do end up adding them.

[kratman]

Sure I was planning to work on PyBaMM Friday, so I will take a look and see if I can help out. Debugging linking things is always a pain. Might be easier to build/test in a docker container locally so you are sure nothing exists on your path.

[agriyakhetarpal]

Superseded by #3789. We can explore cross-compilation later, possibly during this year's rendition of GSoC or when we have upgraded SUNDIALS and SuiteSparse to newer versions.