-
Notifications
You must be signed in to change notification settings - Fork 149
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
Make qsimcirq Apple Silicon (M1, M2, ARM64) compatible and improve build scripts #643
Conversation
Thanks for your contribution @basnijholt ! A couple requests regarding tests and release:
And a final word of caution: as you may have already noticed, |
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.
Thank you. In general, it looks good to me. Here is just one small thing in addition to @95-martin-orion's requests.
@95-martin-orion, thanks for your review. I will address your points.
Seems related:
|
3ac3bcb
to
bb92b2e
Compare
08b84e1
to
0d324c0
Compare
9a92787
to
e7649ff
Compare
Awesome! @95-martin-orion @sergeisakov, on my fork the CI passes and builds Wheels for MacOS x86_64 and ARM64 successfully ✅:tada: I would really appreciate if you could take another look. I believe I addressed all concerns. |
Some recent Github outages broke the CI pipeline. Could you make a fresh commit? I'll trigger the necessary approvals. |
@95-martin-orion, I just closed and opened the PR, which should retrigger the CI too. |
Awesome! The CI pipelines have all passed 🎉 Anything else I can do? 😄 |
c5aee34
to
34ca2a5
Compare
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.
LGTM
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.
Thank you @basnijholt! I'll cut a release to make this available via pip install
.
do we have a tracking bug for actually supporting fast execution on apple silicon architectures? could be cool |
I don't think we have plans to support this. Apple Silicon (and ARM64 in general) doesn't have the SSE or AVX instruction sets, so supporting them would essentially require a total rewrite of qsim from the bottom up. |
If I find the time and motivation I could look in writing a Metal implementation. |
Might I make a suggestion? You can try FlyCI's M1 and M2 runners. Our runners are on average 2x faster and 2x cheaper than GitHub's AND we have a free tier for OSS projects (see below). Install InstructrionsEasily replace your M1 runners: jobs:
ci:
- runs-on: macos-latest
+ runs-on: flyci-macos-large-latest-m1
steps:
- name: 👀 Checkout repo
uses: actions/checkout@v4 Or try the M2 runners: jobs:
ci:
- runs-on: macos-latest
+ runs-on: flyci-macos-large-latest-m2
steps:
- name: 👀 Checkout repo
uses: actions/checkout@v4 Pricing
500 mins/month Free for Public ReposIf your repo is public, then FlyCI offers 500 mins/month of free M1 runner usage with the Best Regards, |
I've been working on compiling
qsimcirq
forconda-forge
(conda-forge/staged-recipes#24504) and encountered several challenges with the existing CMake scripts, particularly for Linux and MacOS x86 platforms.This PR introduces modifications that ensure successful builds on these platforms. Additionally, I've extended support to Apple Silicon (ARM architecture), focusing on compiling
qsim_basic
due to the unavailability of most CPU instruction sets on ARM.This inclusion is particularly beneficial for developers who use MacBook M1 laptops, as it enables them to write and test code on their local machines and seamlessly run the same code on more powerful computational clusters.
Looking ahead, I plan to submit another PR to improve CUDA builds, further enhancing
qsimcirq
's performance and compatibility.I kindly request a review from @95-martin-orion, @jakurzak, and @sergeisakov, as your insights would be invaluable. Your feedback will help ensure that these changes align well with
qsim
's development goals.This PR aims to resolve the following issues:
Thank you for considering these changes!