Added support for Apple Silicon builds in Github Actions #58
Conversation
|
From your comment #3 (comment):
How did you come to that conclusion? |
Sorry, I should have been more clear. There is no stable Fortran compiler with cross-compilation support. The native ones for x86 and arm64 are stable, the one with cross-compilation is an experimental fork that fails to compile some programs in some rare cases. That fork is the one being used in my pull request to compile our Fortran code, and it does work. It is the same one being used by other projects like libAtoms, QUIP, Scikit, and SciPy. The real problem was that the GitHub macOS runner is only on x86 as of now, and the gfortran compiler on x86 macOS cannot cross-compile to arm64. The forked compiler can cross-compile on x86 to arm64 and vice versa. (https://github.com/isuruf/gcc/releases/tag/gcc-10-arm-20210728) Does this pull request look good to go? Cheers! |
|
@lpsinger https://github.com/kokroo/radbelt/actions/runs/5863498123 Here's the GitHub Action and the artifacts at the bottom with builds for all platforms and archs we had before, including the new Apple Silicon ones if you'd like to test them. |
Would you please point me to the parts of some of those projects' GitHub Actions workflows where they pull in this compiler? |
https://github.com/libAtoms/QUIP/blob/public/.github/workflows/build-wheels.yml You can search for "isuruf" on the above links and you can see the lines where it is downloaded, extracted, and later on gfortran is reinstalled to force it to relink, making it aware of this forked compiler. We are doing the same thing. Cannot find the scikit example on my phone right now, perhaps it was in the commit history and not on the latest version. |
Unfortunately, the CIBW_BEFORE_ALL config environment variables and its variants do not let us specify an arch, only a platform. If you take a look at the workflow runs in my fork, I ran variations of the workflow dozens of times to try what you suggested and went through the cibuildwheel documentation as well but no luck. There is no CIBW ENV variable either for determining which platform and arch is currently being built within the workflow. The repos I mentioned above have quite complicated build workflows which tackle this by creating their own custom list of OS/arch combinations. We could do that as well by specifying specific combinations of OS/arch in the CIBW_BUILD variable, but it might not be future-proof and I did not want to make this PR too complicated. 
Is there a disadvantage to using the approach in this PR? We have parallel builds for all 3 jobs, so they are even faster and they all end up in the same artifact and get uploaded to PyPI all the same. |
|
Is this a fork of gcc, or is it just @isuruf's build of gcc from pristine, upstream sources? How long will this workaround be necessary? Will we be able to use the runner-provided version of gfortran once GitHub makes Apple Silicon runners generally available? |
If you look at this thread here https://twitter.com/isuru_f/status/1408511707291455489, isuruf mentions that the gfortran source is from "fxcoudert/gcc/tree/gcc-10-arm". From what I can tell, it is not a 100% pristine, upstream source in the sense that fxcoudert did make some changes which you can see here "https://github.com/fxcoudert/gcc/commits/gcc-10-arm?author=fxcoudert". I would not worry about it because projects like QUIP and SciPY would not have used this without due diligence. Apart from that isuruf is a part of the condaforge team and fxcoudert is a reputed academic so I don't think any malice could be involved.
I don't know of a more appropriate place to mention this, but I am inches away from making the Windows build for radbelt work, and it also requires running scripted commands. If we keep our config for each platform separate like in this PR, it will be easier to tune platform and arch specific builds. |
That's not what I'm worried about. Do the patches on this fork need to be upstreamed before we can use mainline gcc, regardless of what GitHub does? Where can I see the progress on upstreaming those patches? There may be alternatives to gcc --- llvm, I understand, has a frontend for Fortran. Furthermore, I don't want to carry around this complicated build infrastructure myself in my own project. If there was a reusable GitHub Action to install the patched compilers, that would be a different story. |
|
These patches enable cross-compilation, and at this time I do not see any requests for these patches to be upstreamed, so we probably won't be able to use mainline gcc for cross-compilation on an x86 host any time soon. I ran cibuildwheel locally on this M1 Mac that I am typing on right now, and it worked smoothly, since the gcc/gfortran on brew for arm64 compiles an arm64 binary, as it should. So, using mainline gcc works for us only if we get to run it on an arm64 macOS runner, at this time. llvm should work but then again, I believe it will require a script or other moving parts. I agree with you that it's not the most ideal solution to carry this build infra. I see these possibilities:
I would appreciate your thoughts on this and proceed accordingly. I have a Windows Build support PR pending because of this one. Once we conclude how to integrate macOS builds, I will make sure the Windows builds are incorporated in a similar fashion. |
|
FYI, upstream gcc does not support Apple silicon yet. These patches from gcc developer iains are carried by homebrew and conda-forge. |
|
Alright, so I would be making a long-term commitment to use this gcc fork. That's fine. However, I still don't want to carry around this build infrastructure in my own source tree. Given that this has already proven to be a long-term issue for many projects that use cibuildwheel and build fortran code, I would like to see a solution that is reusable across multiple projects --- for example, a GitHub Action that configures cibuildwheel appropriately. |
|
Duly noted. I will start figuring out how to roll this into a GitHub action. As far as I can see all GitHub actions must be stored in a public repo. Should I keep the source code in a public repo on my account or would you like to keep it on yours and add me with write access?
I got scared for a moment there when I started reading your comment because I did use cibuildwheel on my M1 Mac to create a radbelt package and it worked, and the first few words of yours made me wonder how is that possible :D. By the way, I'm curious what brought you here. Was there a notification because I mentioned your repo? |
You should make the repository yourself and maintain it yourself. However, you could develop it in tree here in this branch and split it off into a separate repo once it is close. |
|
@lpsinger I have incorporated the GitHub Action and it works. It's a part of this PR now. |
|
OK, would you please undo all of the changes except for adding the new step that sets up your compilers? |
|
@lpsinger If I just keep this one change, it won't work because then I cannot detect the arch and apply this action for a specific OS and arch. One strategy I can think of, is creating matrices for OS and arch, but even that is complicated since it'll start trying to build unsupported combinations of operating systems and architectures and to prevent that I'll need to add additional if/else style checks. We cannot implement this without a way of knowing the arch. How shall I proceed in this case? EDIT: I forgot to mention, we will need a custom GitHub action for the Windows build as well, so we'll encounter the same situation again. I've noticed that other projects using cibuildwheel also separate the build commands and steps for different operating systems and archs to combat the kind of situation we're facing here. |
|
Why do you need to detect the arch at all? Does the compiler only work for cross compiling from x86_64 to arm64? |
|
@lpsinger I actually did not test that but I was thinking that if the current implementation for x86 to x86 compilation works, I should not touch it at all. "If it ain't broke, don't fix it" principle. The other reason I am suggesting detecting the OS and arch, is to make the Windows builds possible for both x86 and arm64 versions. I can even add support for PyPy and musllinux with short build times after I am done with macOS and Windows, and then again, this branched-out strategy will be easier to read and maintain. Another benefit is that all operating systems and archs can be built in parallel, and if anything fails, it will fail quicker so we won't have to wait for an entire workflow to run, only to observe a failure when it's 90% done. I agree with you, it should not have to be complicated but as far as I can see, other open-source projects out there also have to resort to these measures, so to the best of my knowledge, this is unavoidable if we want wider support. |
|
Please undo all of the changes except for adding the new step that sets up your compilers, and use the new compilers for all architectures supported on macOS. |
|
@lpsinger I just did that, it's failing: https://github.com/kokroo/radbelt/actions/runs/5998736719/job/16267533965 It's using arm64 .so files for x86. |
|
Alright. Can you combine all of the wheel jobs into one matrix, as they were before? |
@lpsinger If you take a look at the commit/branch above, that's what I did. This is how I had started and ran into these problems, hence I was forced to use the branched out strategy. I did exactly as you instructed here:
|
|
@lpsinger I dug deeper into why it's failing. When we prepare the forked compiler, we also force it to link arm64 libs. Since we cannot detect the arch by keeping the OS matrix as it is, the GitHub action is used for both macOS x86 and arm64, which fails because it cannot find x86 .so files. If we were able to detect the target arch somehow, even as an environment variable, we could have altered the compiler setup for the x86 and arm64 separately within the GitHub action itself. |
|
@lpsinger GitHub has just added Apple Silicon runners for CI ( https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/ ). Unfortunately, it's not available for the free plan. I built radbelt locally on my M1 using cibuildwheel, and the wheel works perfectly. If I change the PR here to use the Apple Silicon runner, you will need to test it using your own GitHub account in another branch, if you don't mind (I am assuming the NASA GitHub account is on a paid plan, and hence, it should work out). This change will allow us to skip installing the forked compiler and related steps. I would appreciate your thoughts on this :) |
I've successfully added Apple Silicon support for Radbelt. I used an open-source fork of gfortran which can cross-compile to Apple Silicon.
I had to separate Linux, macOS Intel, and macOS ARM builds because there was no way to figure out which platform and architecture is being targeted by cibuildwheel, in the midst of a Github Action workflow.
Below is a screenshot of Radbelt installed from the wheel found in the artifacts of the Github Runner running natively on my M1 Macbook Pro. There's an infinitesimal variation in the calculation due to the lower precision inherent in Apple Silicon and also due to the fact that the Fortran compiler is still not optimized to handle higher precision. I will try to rectify this sometime soon by looking into the forked compiler and seeing how it translates integer and float types to ARM binary code.
Fixes #3
@lpsinger