-
Notifications
You must be signed in to change notification settings - Fork 368
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
Add CI jobs to cross-build arm64 macOS wheels #1362
Conversation
CMakeLists.txt
Outdated
if(CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "x86_64" OR CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "AMD64") | ||
if(APPLE OR UNIX) | ||
if(NOT CMAKE_OSX_ARCHITECTURES STREQUAL "arm64" AND (APPLE OR UNIX)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the problem is that CMAKE_OSX_ARCHITECTURES
is not really set and that we are using CMAKE_HOST_SYSTEM_PROCESSOR
instead of CMAKE_SYSTEM_PROCESSOR
, so it incorrectly assumes is in x86_64
.
I would try with:
if(CMAKE_SYSTEM_PROCESSOR STREQUAL "x86_64" OR CMAKE_SYSTEM_PROCESSOR STREQUAL "AMD64")
if(APPLE OR UNIX)
ecd70ba
to
86fd342
Compare
…ing used" This isn't needed I found the other location in a cmakefile triggering the compilation of the avx2 file so we don't need to print this anymore (it also wouldn't have shown what I was looking for). This reverts commit 9a6883c.
There was a second cmake file embedded in the source tree triggering the avx2 file compilation. This commit adds an if condition to that location as well to avoid trying to build avx2 on arm64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks solid, and not too much of a change, which is nice!
if (NOT CMAKE_OSX_ARCHITECTURES STREQUAL "arm64") | ||
# We build SIMD filed separately, because they will be reached only if the | ||
# machine running the code has SIMD support | ||
set(SIMD_SOURCE_FILE "../../../../../src/simulators/statevector/qv_avx2.cpp") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
../../../../../
lol. I know it's pre-existing, but is it worth quickly swapping it to ${PROJECT_SOURCE_DIR}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, sure I'll update this now to make this ready for merge and do that change as part of it
The wheels work for me (well Py3.9 one did). |
This commit finalizes the iterative process in earlier commits to prepare this PR for merging. It makes 3 primary changes, the first is to add the proper release wheel publish job. This is a dual of the job added in the previous commits that runs in the build ci workflow. That job but will run on tags and publish the built wheels to pypi. The build job that runs on every commit will still stay around to ensure we don't regress and break macOS arm64 builds since testing of it will be limited. The second change is a small update to the cmake files to clean them up slightly based on review comments. The last change here is adding a release note to document the addition of support for arm64 macOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🎉 🎉
run: python -m pip install -U cibuildwheel==2.1.2 | ||
- name: Build Wheels | ||
env: | ||
CIBW_SKIP: "cp310-* pp*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line will need updating in #1430 after merge, and similar in deploy.yml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I'll update it here or in #1430 depending on which merges first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually on second thought it's not strictly needed as the cibuildwheel configuration already filters arm64 macOS wheels to >= 3.8 as those are the only python versions which officialy support arm64 macOS. So we don't strictly need to change anything here because it'll already skip 3.6 and 3.7
This PR adds CI jobs to build qiskit-aer wheels on arm64 macOS wheels. Since no CI system provides native arm64 macOS environments these CI jobs are configured to cross compile arm64 binaries from an x86_64 macOS environment. This has the limitation of not letting us execute the binaries but we can at least test building them in PR CI and publish the built binaries at release time. The two jobs added here do just that, the first is run as part of the Build workflow that runs on every proposed commit to test that we can at least compile on arm64 macOS (for supported python versions). The second job is a release publishing job that will build and publish arm64 macOS wheels to PyPI whenever there is a release. Fixes Qiskit#1286 Fixes Qiskit#1397
The supported platform list in the install documentation wasn't quite up to date. The supported platforms are also a bit more nuanced than that list made it appear. This commit expands the platform support section and documents the different levels of support for platforms and the details about each level. This should hopefully make it clearer to users which platforms are expected to work and what guarantees are made for each platform. This is related to Qiskit/qiskit#7553, Qiskit/qiskit#7549, and Qiskit/qiskit-aer#1362 which are expanding (or just changing the level of support) the supported platforms but each have a different set of tradeoffs that we should be documenting.
* Expand platform support section to install doc The supported platform list in the install documentation wasn't quite up to date. The supported platforms are also a bit more nuanced than that list made it appear. This commit expands the platform support section and documents the different levels of support for platforms and the details about each level. This should hopefully make it clearer to users which platforms are expected to work and what guarantees are made for each platform. This is related to Qiskit/qiskit#7553, Qiskit/qiskit#7549, and Qiskit/qiskit-aer#1362 which are expanding (or just changing the level of support) the supported platforms but each have a different set of tradeoffs that we should be documenting. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake@binhbar.com>
This PR adds CI jobs to build qiskit-aer wheels on arm64 macOS wheels. Since no CI system provides native arm64 macOS environments these CI jobs are configured to cross compile arm64 binaries from an x86_64 macOS environment. This has the limitation of not letting us execute the binaries but we can at least test building them in PR CI and publish the built binaries at release time. The two jobs added here do just that, the first is run as part of the Build workflow that runs on every proposed commit to test that we can at least compile on arm64 macOS (for supported python versions). The second job is a release publishing job that will build and publish arm64 macOS wheels to PyPI whenever there is a release. Fixes #1286 Fixes #1397
This PR adds CI jobs to build qiskit-aer wheels on arm64 macOS wheels. Since no CI system provides native arm64 macOS environments these CI jobs are configured to cross compile arm64 binaries from an x86_64 macOS environment. This has the limitation of not letting us execute the binaries but we can at least test building them in PR CI and publish the built binaries at release time. The two jobs added here do just that, the first is run as part of the Build workflow that runs on every proposed commit to test that we can at least compile on arm64 macOS (for supported python versions). The second job is a release publishing job that will build and publish arm64 macOS wheels to PyPI whenever there is a release. Fixes Qiskit#1286 Fixes Qiskit#1397
…kage#1402) * Expand platform support section to install doc The supported platform list in the install documentation wasn't quite up to date. The supported platforms are also a bit more nuanced than that list made it appear. This commit expands the platform support section and documents the different levels of support for platforms and the details about each level. This should hopefully make it clearer to users which platforms are expected to work and what guarantees are made for each platform. This is related to Qiskit#7553, Qiskit#7549, and Qiskit/qiskit-aer#1362 which are expanding (or just changing the level of support) the supported platforms but each have a different set of tradeoffs that we should be documenting. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake@binhbar.com>
…kage#1402) * Expand platform support section to install doc The supported platform list in the install documentation wasn't quite up to date. The supported platforms are also a bit more nuanced than that list made it appear. This commit expands the platform support section and documents the different levels of support for platforms and the details about each level. This should hopefully make it clearer to users which platforms are expected to work and what guarantees are made for each platform. This is related to Qiskit#7553, Qiskit#7549, and Qiskit/qiskit-aer#1362 which are expanding (or just changing the level of support) the supported platforms but each have a different set of tradeoffs that we should be documenting. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake@binhbar.com>
…kage#1402) * Expand platform support section to install doc The supported platform list in the install documentation wasn't quite up to date. The supported platforms are also a bit more nuanced than that list made it appear. This commit expands the platform support section and documents the different levels of support for platforms and the details about each level. This should hopefully make it clearer to users which platforms are expected to work and what guarantees are made for each platform. This is related to Qiskit/qiskit#7553, Qiskit/qiskit#7549, and Qiskit/qiskit-aer#1362 which are expanding (or just changing the level of support) the supported platforms but each have a different set of tradeoffs that we should be documenting. * Apply suggestions from code review Co-authored-by: Jake Lishman <jake@binhbar.com> Co-authored-by: Jake Lishman <jake@binhbar.com>
Summary
This PR adds CI jobs to build qiskit-aer wheels on arm64 macOS wheels. Since no CI system provides native arm64
macOS environments these CI jobs are configured to cross compile arm64 binaries from an x86_64 macOS
environment. This has the limitation of not letting us execute the binaries but we can at least test building them in
PR CI and publish the built binaries at release time. The two jobs added here do just that, the first is run as part
of the
Build
workflow that runs on every proposed commit to test that we can at least compile on arm64 macOS(for supported python versions). The second job is a release publishing job that will build and publish arm64
macOS wheels to PyPI whenever there is a release.
Details and comments
Fixes #1286
Fixes #1397