Skip to content

ENH: add wheels built against Accelerate for arm64 macOS >=14 #25012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 17 commits into from

Conversation

rgommers
Copy link
Member

@rgommers rgommers commented Oct 26, 2023

Also clean up pyproject.toml further, the cibuildwheel configurations are pretty understandable now.

The new Accelerate-based wheels weigh in at about 5.0 MB, which is impressively small compared to the OpenBLAS arm64 wheels at ~14 MB and the OpenBLAS x86-64 wheels at ~21 MB.

@rgommers rgommers added this to the 2.0.0 release milestone Oct 26, 2023
@rgommers rgommers marked this pull request as draft October 26, 2023 20:57
@rgommers
Copy link
Member Author

Ugh:

tart isolation failed: failed to create VM cloned from "ghcr.io/cirruslabs/macos-sonoma-xcode:15": tart command returned non-zero exit code: "Error: The file “15” couldn’t be saved in the folder “macos-sonoma-xcode” because a file with the same name already exists."

I failed to get Cirrus CI installed on my own fork - guess I'll have to try harder.

@rgommers
Copy link
Member Author

Hmm, there are a large amount of f2py errors and failures in the wheel builds:( I'll have to have a look at the open PRs with fixes and recent changes I guess.

@rgommers
Copy link
Member Author

Good news is that the macOS 14 + XCode 15 image is now running. Bad news is that it needs more tweaks, it dies quickly when talking to Homebrew:

See: https://docs.brew.sh/Homebrew-and-Python
ln -s python3 /opt/homebrew/opt/python@3.10/bin/python
which python
����

@rgommers rgommers force-pushed the accelerate-wheels branch 12 times, most recently from 938cb0f to 3a74dd6 Compare November 22, 2023 22:44
… config

It's already included in the top-level `meson.build` file, which
is more correct. See numpygh-25004 for where `-fno-strict-aliasing` came
from.

Also clean up the `-Wl,--strip-debug` flag, it's unused since the
buildtype is release and there is no debug info in the binaries.
…[wheel build]

[skip actions] [skip circle] [skip azp]
[skip actions] [skip azp] [skip circle]
[skip actions] [skip azp] [skip circle]
[skip actions] [skip azp] [skip circle]
[skip actions] [skip azp] [skip circle]
@rgommers
Copy link
Member Author

Something about gfortran in CI is not working:

gfortran: warning: could not understand version �14.0�
ld: warning: ignoring duplicate libraries: '-lemutls_w', '-lgcc', '-lgfortran'
ld: library 'bundle1.o' not found
collect2: error: ld returned 1 exit status
error: Command "/usr/local/bin/gfortran -Wall -g -Wall -g -undefined dynamic_lookup -bundle /var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/src.macosx-11.0-arm64-3.10/_test_value_attrspec_TestValueAttr_ext_modulemodule.o /var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/src.macosx-11.0-arm64-3.10/fortranobject.o /var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp8ye9twj6/gh21665.o /var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/var/folders/c2/b2kmn__12wq57m82ltz0hl1w0000gn/T/tmp56jwuv5g/src.macosx-11.0-arm64-3.10/_test_value_attrspec_TestValueAttr_ext_module-f2pywrappers2.o -L/opt/gfortran-darwin-arm64-native/bin/../lib/gcc/arm64-apple-darwin20.0.0/11.3.0 -L/opt/gfortran-darwin-arm64-native/bin/../lib/gcc/arm64-apple-darwin20.0.0/11.3.0/../../.. -L/opt/gfortran-darwin-arm64-native/bin/../lib/gcc/arm64-apple-darwin20.0.0/11.3.0/../../.. -lgfortran -o ./_test_value_attrspec_TestValueAttr_ext_module.cpython-310-darwin.so" failed with exit status 1
6 failed, 40769 passed, 374 skipped, 48 xfailed, 4 xpassed, 35 warnings, 215 errors in 85.86s (0:01:25)

bundle1.o should be found under SDKROOT.

On the 11.0 deployment target job, getting:

ld: library not found for -lm

for f2py tests is also a sign of some env var like LD_LIBRARY_PATH not being set the way gfortran wants it.

@rgommers
Copy link
Member Author

This is not the cause, since I see the same message on a run that just passed on main:

Auto-upgraded from ghcr.io/cirruslabs/macos-monterey-xcode:14 to ghcr.io/cirruslabs/macos-monterey-xcode:latest

@mattip
Copy link
Member

mattip commented Nov 23, 2023

I seem to remember something about needing the sdk-path in the environment

        # if [ "macos-11" == "${{ matrix.os }}" ]; then
        #   Use xcrun --sdk macosx --show-sdk-path instead of hardcoding the path
        #   echo "LDFLAGS=-L/Library/Developer/CommandLineTools/SDKs/MacOSX12.1.sdk/usr/lib" >> $GITHUB_ENV;
        #   echo "LIBRARY_PATH=-L/Library/Developer/CommandLineTools/SDKs/MacOSX12.1.sdk/usr/lib" >> $GITHUB_ENV;

@andyfaff
Copy link
Member

Ahhh, CI debugging!

I think this might be a call for some @isuruf magic. From memory when we see errors like ld: library not found for -lm, that usually means theres something gone wrong with the gfortran toolchain.

Having said that, the wheel build seems to be working, but it's the test that is failing. I'm wondering if the subprocess.Popen call is getting the same set of environment variables that are supplied to the original build. For example, if SDKROOT is not set properly, then I think you'll see those errors. Can you check what environment variables are being given to/inherited by the call? I wouldn't be surprised if cibuildwheel doesn't supply them in the test run.

CIBW_ENVIRONMENT_MACOS: >
RUNNER_OS=macOS
MACOSX_DEPLOYMENT_TARGET=11.0 # 14.0
# SDKROOT needs to be set also here (see gfortran_utils.sh)
SDKROOT=/Applications/Xcode-14.0.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path might be wrong.
I suggest

Suggested change
SDKROOT=/Applications/Xcode-14.0.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk
SDKROOT=/Applications/Xcode-14.0.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test run tries to compile a fortran program in a subprocess. I suspect that this env variable isn't being passed down to the compilation.

@rgommers
Copy link
Member Author

Yeah, some paths are wrong. I think @isuruf's suggestion may be right; there may be other things wrong. The problem is that the setup is so brittle that it's almost impossible to work with:

  • Homebrew Python is used, needs symlinking and a hardcoded PATH
  • OpenBLAS needs a bunch of manual copying around and LD_LIBRARY_PATH
  • Gfortran needs LD_LIBRARY_PATH and SDKROOT hardcoded
  • There's also a DYLD_LIBRARY_PATH floating around inside one of the shell scripts for one of the above
  • It's not possible to dynamically determine things like SDKROOT, that doesn't carry over well between cibuildwheel steps, so it needs hardcoding in CIBW_ENVIRONMENT_MACOS - and then good luck determining if a new OS image changed it or not

I'm going to have to start over and change less in the shell scripts. I am also going to drop gfortran completely from the Accelerate jobs (and possibly the OpenBLAS wheel build jobs too); we don't need any Fortran compiler really - the f2py tests don't need to run in every job, nothing can be wrong if those pass in regular CI, since no compiled code inside the numpy wheel is involved (any errors are going to be CI config, not f2py bugs).

@andyfaff
Copy link
Member

Things can definitely be made more robust, but to be fair it's not the easiest to work in CI directly. I wonder if it'd be easier to install something like miniconda, and use that for python, compiler (gfortran) provision. This would hopefully get rid of the SDKROOT issues.

GHA is better in this regard because there are pre-written Actions that do all this setup for us.

@rgommers
Copy link
Member Author

Yes, agreed - definitely not easy to work with this setup. I think that's the right plan (probably micromamba rather than miniconda, faster and more self-contained). Since the gfortran we use is repackaged from conda-forge, I think we should be able to use it directly from conda-forge instead. @isuruf does that sound right?

For OpenBLAS we should stay with our own builds I believe - but of course we're moving to the openblas-scipy64 wheel, which will avoid all the hardcoded paths and copying around as well.

@isuruf
Copy link
Contributor

isuruf commented Nov 24, 2023

Since the gfortran we use is repackaged from conda-forge, I think we should be able to use it directly from conda-forge instead. @isuruf does that sound right?

conda-forge toolchain also requires this variable. Let me send a PR to fix this more robustly.

@isuruf
Copy link
Contributor

isuruf commented Nov 24, 2023

See rgommers#59

@andyfaff
Copy link
Member

I'm going to investigate https://tart.run/quick-start/ as a way of running a macos container locally.

@andyfaff
Copy link
Member

I've had good success with using tart to run a macOS container locally, such that would be used on CirrusCI (the ghcr.io/cirruslabs/macos-sonoma-xcode:latest image). @rgommers, if you have a mac it's going to be much faster to use a local container than try and fix things in CI.

Rough Steps:

  1. brew install cirruslabs/cli/tart on your local machine
  2. tart clone ghcr.io/cirruslabs/macos-sonoma-xcode:latest sonoma-xcode (this is a large download).
  3. tart run sonoma-xcode &. This will start a tart window with a Sonoma GUI in it.
  4. you can connect to the container using ssh admin@$(tart ip sonoma-xcode) from the host computer.
  5. In order to get cibuildwheel to work you'll need to use the tart UI window to install a CPython. This is because when you eventually run cibuildwheel in the container it needs the official CPython install. Only when cibuildwheel is run in CI will it use other python installs.
  6. Navigate to Applications/CPython and run the install certificates command file. Otherwise you won't be able to use pip to install anything.
  7. Go back to the ssh window to the container from step 4 (alternately start up a terminal window in the tart UI window). You'll have to use git to clone the repo.

This whole setup gives you a container that is identical to that used to build wheels on cirrusci, but on your local machine.
From the ssh connection I did: brew install micromamba, created a conda env, activated that conda env, installed the conda-forge compilers package and python3.11 from conda-forge, pip installed cibuildwheel, set the appropriate CIBW env vars from the numpy wheel build configuration, then ran cibuildwheel --platforms macos. The whole build and test worked without having to set SDKROOT, LD_LIBRARY_PATH, or any of the other variables.

hardcode SDKROOT in gcc spec files

[skip actions] [skip circle] [skip azp]
@rgommers
Copy link
Member Author

Thanks for the fix @isuruf and for investigating Tart @andyfaff! That local testing setup looks really useful. I'm traveling and on a slow connection right now, but I'll try it out next week.

@andyfaff
Copy link
Member

Actually you can install the cirrus-cli, brew install cirruslabs/cli/cirrus, once you've installed tart. This will enable you to do a cirrus run from the numpy directory, which will run a .cirrus.yml file. You have to create that file, I just copied the relevant files that I was trying to modify.

@@ -144,6 +147,7 @@ if [ "$(uname)" == "Darwin" ]; then
pushd /opt
sudo tar -xvf gfortran-darwin-${arch}-${type}.tar.gz
sudo rm gfortran-darwin-${arch}-${type}.tar.gz
find gfortran-darwin-${arch}-${type} -name "libgfortran.spec" -exec sed -i.bak "s@\-lm@-lm -isysroot ${SDKROOT}@g" {} +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
find gfortran-darwin-${arch}-${type} -name "libgfortran.spec" -exec sed -i.bak "s@\-lm@-lm -isysroot ${SDKROOT}@g" {} +
find gfortran-darwin-${arch}-${type}/lib/gcc -name "libgfortran.spec" -exec sed -i.bak "s@\-lm@-lm -isysroot ${SDKROOT}@g" {} +

@andyfaff
Copy link
Member

Noting that #25255 is an alternate to this PR

@rgommers
Copy link
Member Author

The commits from this PR not directly related to Accelerate wheel builds are now merged. The rest of this is superseded by gh-25255, where @andyfaff got the build to work. So I'll close this issue. Thanks all!

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.

4 participants