Skip to content
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

macos arm64 build #53

Merged
merged 15 commits into from
Jan 31, 2021
Merged

macos arm64 build #53

merged 15 commits into from
Jan 31, 2021

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Jan 23, 2021

Copy link
Collaborator

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

Cool! For some reason, I was under the impression that the access to the OS/plat combination in question was somewhat limited for now.

In case you didn't know (you probably do)--our Travis CI credits are pretty limited for this organization (i.e., if experimentation is possible on github actions).

@mattip
Copy link
Collaborator

mattip commented Jan 24, 2021

@tylerjereddy I think this does atraget github actions, just the script is named travis-ci/build_steps.sh for historical reasons. At some point we should rename the directory.

Copy link
Collaborator

@mattip mattip left a comment

Choose a reason for hiding this comment

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

Thanks for this. It looks good to me, just a few minor comments.

- os: macos-latest
PLAT: arm64
INTERFACE64: ''
platform: [x64]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure we need platform at all in this file, it might be some cruft left over from the transition to github actions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment to explain that this entry of the build matrix is using cross-compilation to build an macos/arm64 on a macos/x86_64 host (typically on github actions that does not provide native macos/arm64 runners at this time).

travis-ci/build_steps.sh Show resolved Hide resolved
@mattip
Copy link
Collaborator

mattip commented Jan 24, 2021

This is cross-compiling using x86_64 hardware to compile the arm64 library.

s390x)
local bitness=64
;;
ppc64le)
local bitness=64
local target_flags="TARGET=POWER8"
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this change will impact the future numpy / scipy wheels built for ppc64le. Those wheels are typically built on a PPC host (without cross-compilation) so maybe this won't change a thing (but I agree it's best to make the target architecture explicit).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those wheels are typically built on a PPC host (without cross-compilation) so maybe this won't change a thing

It will if the host becomes a power9 or power10 some day.

Copy link
Contributor

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

The new entry in the build matrix fails after a bunch of ld warnings:

021-01-23T23:07:18.9595570Z ld: warning: object file (sblat1.o) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:18.9597490Z ld: warning: dylib (/opt/gfortran-darwin-arm64/lib/gcc/arm64-apple-darwin20.0.0/10.2.1/libgfortran.dylib) was built for newer macOS version (11.0) than being linked (10.9)
[...]
2021-01-23T23:07:18.9611620Z ld: warning: object file (../libopenblasp-r0.3.13.dev.a(isamax.o)) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:18.9613540Z ld: warning: object file (../libopenblasp-r0.3.13.dev.a(sasum.o)) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:18.9615450Z ld: warning: object file (../libopenblasp-r0.3.13.dev.a(saxpy.o)) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:18.9617340Z ld: warning: object file (../libopenblasp-r0.3.13.dev.a(scopy.o)) was built for newer macOS version (11.0) than being linked (10.9)
[...]
2021-01-23T23:07:50.0315770Z ld: warning: object file (/opt/gfortran-darwin-arm64/lib/gcc/arm64-apple-darwin20.0.0/10.2.1/libgcc.a(_udivmoddi4.o)) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:50.0317750Z ld: warning: object file (/opt/gfortran-darwin-arm64/lib/gcc/arm64-apple-darwin20.0.0/10.2.1/libgcc.a(_udiv_w_sdiv.o)) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:50.0319920Z ld: warning: object file (/opt/gfortran-darwin-arm64/lib/gcc/arm64-apple-darwin20.0.0/10.2.1/libgcc.a(enable-execute-stack.o)) was built for newer macOS version (11.0) than being linked (10.9)
2021-01-23T23:07:50.0321110Z Undefined symbols for architecture arm64:
2021-01-23T23:07:50.0321630Z   "___chkstk_darwin", referenced from:
2021-01-23T23:07:50.0322620Z       _sgemv_ in libopenblasp-r0.3.13.dev.a(sgemv.o)
2021-01-23T23:07:50.0323660Z       _sger_ in libopenblasp-r0.3.13.dev.a(sger.o)
2021-01-23T23:07:50.0324750Z       _cblas_sgemv in libopenblasp-r0.3.13.dev.a(cblas_sgemv.o)
2021-01-23T23:07:50.0325860Z       _cblas_sger in libopenblasp-r0.3.13.dev.a(cblas_sger.o)
2021-01-23T23:07:50.0326930Z       _dgemv_ in libopenblasp-r0.3.13.dev.a(dgemv.o)
2021-01-23T23:07:50.0327970Z       _dger_ in libopenblasp-r0.3.13.dev.a(dger.o)
2021-01-23T23:07:50.0329050Z       _cblas_dgemv in libopenblasp-r0.3.13.dev.a(cblas_dgemv.o)
2021-01-23T23:07:50.0329590Z       ...
2021-01-23T23:07:50.0329980Z ld: symbol(s) not found for architecture arm64
2021-01-23T23:07:50.0330510Z collect2: error: ld returned 1 exit status
2021-01-23T23:07:50.0331500Z make[1]: *** [libopenblasp-r0.3.13.dev.dylib] Error 1
2021-01-23T23:07:50.0332080Z make: *** [shared] Error 2
2021-01-23T23:07:50.0339920Z ##[error]Process completed with exit code 2.

Potential fix below:

.github/workflows/multibuild.yml Show resolved Hide resolved
@mattip
Copy link
Collaborator

mattip commented Jan 30, 2021

The new run is failing after building in a switch statement with echo Did not recognize plat arm64. It seems one of the bash commands is missing set -x and there is somewhere without arm64

@isuruf
Copy link
Contributor Author

isuruf commented Jan 30, 2021

See MacPython/gfortran-install#5

@mattip
Copy link
Collaborator

mattip commented Jan 30, 2021

s390x is failing, everything else passes. This now creates a tarball openblas-v0.3.13-62-gaf2b0d02-macosx_11_0_arm64-gf_f10e307.tar.gz. How do we know it performs correctly without testing it?

@isuruf
Copy link
Contributor Author

isuruf commented Jan 30, 2021

How do we know it performs correctly without testing it?

We don't.

@ogrisel
Copy link
Contributor

ogrisel commented Jan 31, 2021

How do we know it performs correctly without testing it?

While waiting for CI-providers to provide M1 support, we can run the tests manually from time to time on people's laptop. @mattip let me know if you want me to run the numpy tests before a release for instance.

@ogrisel
Copy link
Contributor

ogrisel commented Jan 31, 2021

The s390x builds on travis only fail when trying to install anaconda-client to upload the resulting artifacts:

+++pip install -q git+https://github.com/Anaconda-Platform/[email protected]
  WARNING: Building wheel for clyent failed: [Errno 13] Permission denied: '/home/travis/.cache/pip/wheels/a8'
  WARNING: Building wheel for PyYAML failed: [Errno 13] Permission denied: '/home/travis/.cache/pip/wheels/30'
  WARNING: Building wheel for pyrsistent failed: [Errno 13] Permission denied: '/home/travis/.cache/pip/wheels/34'
ERROR: Could not build wheels for PyYAML which use PEP 517 and cannot be installed directly

Nothing in the current PR seem to be able to cause this, beyond the multibuild update.

@mattip
Copy link
Collaborator

mattip commented Jan 31, 2021

OK, I guess this is required to even think about numpy wheels for M1, so let's merge it.

@mattip mattip merged commit c70ac71 into MacPython:master Jan 31, 2021
@isuruf isuruf deleted the arm64 branch January 31, 2021 20:31
@mattip
Copy link
Collaborator

mattip commented Jan 31, 2021

Thanks @isuruf. The next step for NumPy will be to add arm64 to the accepted platforms in tool/openblas_support.py so that people with a M1 can do python tool/openblas_support.py to download the artifact.

@ogrisel
Copy link
Contributor

ogrisel commented Feb 1, 2021

@mattip @isuruf ok the build was successful:

I downloaded the tarball with:

curl -L --output openblas.tar.gz https://anaconda.org/multibuild-wheels-staging/openblas-libs/v0.3.13-62-gaf2b0d02/download/openblas-v0.3.13-62-gaf2b0d02-macosx_11_0_arm64-gf_f10e307.tar.gz

Then I unzipped it in /opt/arm64-builds/. This is important because this path is hardcoded in the dylib and importing numpy will fail otherwise.

My numpy site.cfg had therefore the following lines:

[openblas]
libraries = openblas
library_dirs = /opt/arm64-builds/lib
include_dirs = /opt/arm64-builds/include
runtime_library_dirs = /opt/arm64-builds/lib

Then I could build numpy (pip install -e . --no-build-isolation -v) and I can import numpy and it dynlinks to openblas successfully:

% python -m threadpoolctl -i numpy
[
  {
    "filepath": "/opt/arm64-builds/lib/libopenblasp-r0.3.13.dev.dylib",
    "prefix": "libopenblas",
    "user_api": "blas",
    "internal_api": "openblas",
    "version": "0.3.13.dev",
    "num_threads": 8,
    "threading_layer": "pthreads"
  }
]

and I confirm that this is the arm64 build:

% file /opt/arm64-builds/lib/libopenblasp-r0.3.13.dev.dylib
/opt/arm64-builds/lib/libopenblasp-r0.3.13.dev.dylib: Mach-O 64-bit dynamically linked shared library arm64

and for Python:

% python -c "import platform; print(platform.platform())"
macOS-11.1-arm64-arm-64bit

I can then run the numpy tests almost successfully. There are only 6 failures and those are the same as those failing with the conda-forge macos/arm64 build of numpy + openblas. Here are the details:

https://gist.github.com/ogrisel/faa61675b490dcf749a60e26840938fb

Note that the first time I had downloaded the archive with Firefox, but when I try to import numpy I get a pop-up that said:

“libopenblasp-r0.3.13.dev.dylib” can’t be opened because Apple cannot check it for malicious software.

This problem does not happen if you download the tarball with curl apparently...

@ogrisel
Copy link
Contributor

ogrisel commented Feb 1, 2021

So I guess the next step would be to adapt https://github.com/MacPython/numpy-wheels to try build the macos/arm64 numpy wheel in a cross-compiler environment (using crossenv) and this OpenBLAS tarball.

Also the openblas_support.py script will need an update because it's confusing OS info and architecture info. It would be better to have platform info as the concatenation of OS identifier and architecture identifier.

@isuruf
Copy link
Contributor Author

isuruf commented Feb 1, 2021

So I guess the next step would be to adapt https://github.com/MacPython/numpy-wheels to try build the macos/arm64 numpy wheel in a cross-compiler environment (using crossenv) and this OpenBLAS tarball.

Yes, multibuild has support now. So, once openblas_support.py is fixed, this should be good to go.

Also the openblas_support.py script will need an update because it's confusing OS info and architecture info. It would be better to define the platform info as the concatenation of an OS identifier and an architecture identifier.

Agreed. It's a mess because of mixing OS and architecture info. Anybody wants to fix that? @mattip, @rgommers, @ogrisel ?

@ogrisel
Copy link
Contributor

ogrisel commented Feb 2, 2021

How do we know it performs correctly without testing it?

@mattip https://www.scaleway.com/en/hello-m1/ is now providing (costly) M1 hosting for continuous integration. Maybe it would be possible to ask them for a CI free runner for github actions for the scipy ecosystem. Or we can just wait for the github and Azure to provide M1 runners by default for free (which is bound to happen at some point I think).

Agreed. It's a mess because of mixing OS and architecture info. Anybody wants to fix that? @mattip, @rgommers, @ogrisel ?

Not in the coming days. But I can give it a try a bit later if nobody did it in the mean time.

@rgommers
Copy link
Collaborator

rgommers commented Feb 2, 2021

It's 10 cents an hour, that's okay. It's just a hosted service though, where you can use SSH/VPN in (https://www.scaleway.com/en/docs/apple-silicon-as-a-service-quickstart/) - there's no GitHub Actions or other CI provider integration as far as I can tell.

If someone wants to work on this connect to the wheel build repos as CI, we can pay for the hosting costs - it'll be 75 euro/hr until we can drop it because free CI providers gain support (maybe 6 months or so?).

@ogrisel
Copy link
Contributor

ogrisel commented Feb 2, 2021

Unfortunately, it appears that it's not trivial to register an Apple M1 machine as a github actions runner: actions/runner#805.

@rgommers
Copy link
Collaborator

rgommers commented Feb 2, 2021

That's too bad.

The opinions on M1 support diverge in interesting ways. From "we want this yesterday, it's the best thing since sliced bread" to "why would we care unless there's CI support or Apple does or pays for all the work".

Could also say it's good advertisement for conda-forge, which has superior infrastructure, so let's point people at it for now (that's what we have to say today anyway).

Guess we'll end up being pragmatic and do manual testing + upload to PyPI.

@ogrisel
Copy link
Contributor

ogrisel commented Feb 3, 2021

"we want this yesterday, it's the best thing since sliced bread"

I really like this hardware. It's really a step up in terms of computational performance - battery autonomy - no noise - low waste heat / $.

Could also say it's good advertisement for conda-forge, which has superior infrastructure, so let's point people at it for now (that's what we have to say today anyway).

I agree but. I really had a smooth experience with conda-forge (especially with the faster mamba), but apparently that won't prevent users to complain and ask for pip-installable wheels for M1 machines.

Guess we'll end up being pragmatic and do manual testing + upload to PyPI.

I guess so :) Finalizing the infra (e.g. fixing openblas_support.py) for this will not be wasted because it will be reused once github actions / azure pipelines and co start to natively host M1 runners.

@ilayn
Copy link

ilayn commented Feb 3, 2021

"why would we care unless there's CI support or Apple does or pays for all the work".

That would be me. I wholeheartedly agree with the efforts but covering apple's back is something I resent a lot given how precious maintainers' time is. Especially if the next model is going to be yet another "revolutionary upgrade" this will all go to waste which is pretty much possible given their track record.

Not to mention there are a huge population of Python users who won't or can't use conda so this needs a bit of a pushback towards apple.

@matthew-brett
Copy link
Contributor

The divergence of opinions doesn't surprise me - I bet it tracks closely with the divergence of opinions on Apple and Macs generally.

The key point for me is that I want Numpy and scientific Python to be the natural best choice for scientific computing. In practice, a lot of scientists are going to own M1 Macs, soon, and they will feel strongly about getting best performance out of their machines. If Numpy is fairly well behind, or perceived as being fairly well behind the best performance, on these machines, that will be a problem for the ecosystem.

And for the maintainer time - I don't own an M1 Mac personally, but I would be suprised if they weren't fairly common as developer machines in a few months.

@mattip
Copy link
Collaborator

mattip commented Feb 4, 2021

@matthew-brett I am not sure what you are proposing. I do not feel comfortable releasing software we have not tested. Can some of these people urging us to release packaging prompt Apple and the CI vendors to make runners available?

@matthew-brett
Copy link
Contributor

My suggestion above was only that we can't afford to dismiss M1 as a major platform for Numpy and scientific computing, whatever we think of Apple or its engagement with the open source community. It's really a matter of tone. At the moment I think we're giving off a vague impression of "why should we help you to use your fancy and exotic hardware" - rather than "excellent, we're planning to support this well, as soon as we can, here's how you can help".

@ilayn
Copy link

ilayn commented Feb 4, 2021

That's very true but I think there is a balance here. There is a difference between "OK here is one more architecture we have to support, there are no tools for it yet but it's popular" and "can we contact apple about this? we would love to have it but currently that's too much risk."

Similarly we ignored the whole windows land if it wasn't for the fantastic efforts of Gohlke's builds. It is almost identical to that situation now. So I find this apple adventure a bit too enthusiastic.

Other than that fully agree with the tone part

@rgommers
Copy link
Collaborator

rgommers commented Feb 4, 2021

Similarly we ignored the whole windows land if it wasn't for the fantastic efforts of Gohlke's builds

Not quite right - we spent a ton of effort building .exe MSI installers (on Linux, via Wine) before Appveyor, wheels etc. came into existence.

"excellent, we're planning to support this well, as soon as we can, here's how you can help".

That sounds about right to me.

@ilayn
Copy link

ilayn commented Feb 4, 2021

Well for more than a decade we couldn't have pip install for SciPy on windows I don't think that counts as supporting it

@isuruf
Copy link
Contributor Author

isuruf commented Feb 4, 2021

@ogrisel numpy/numpy#18331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants