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

CI: adds aarch64 wheels #121

Closed
wants to merge 5 commits into from

Conversation

Transurgeon
Copy link

reference issue: #111 .
suggestion for fix by @matteoettam09 .
following the PR in cvxpy#2054.

@SteveDiamond
Copy link
Contributor

@bodono can you look at this? We are getting a lot of complaints about cvxpy not being able to install on linux because of the SCS dependency. This MR would fix the issue potentially.

Comment on lines +143 to +144
CIBW_ARCHS_MACOS: x86_64 universal2
CIBW_ARCHS_LINUX: auto aarch64
Copy link
Owner

Choose a reason for hiding this comment

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

I'm no expert on this stuff, but it seems to be failing due to only wanting one of these to be set at a time. I think you will need to put the two architectures inside the matrix, so you can use something like {{ matrix.arch }} here, and then it will kick off 2x the jobs, one for each arch choice.

@bodono
Copy link
Owner

bodono commented Jan 2, 2025

Sure, let's get this landed! I took a look at the problem and it seems relatively minor to fix, I left the following comment:

I'm no expert on this stuff, but it seems to be failing due to only wanting one of these to be set at a time. I think you will need to put the two architectures inside the matrix, so you can use something like {{ matrix.arch }} here, and then it will kick off 2x the jobs, one for each arch choice.

@Transurgeon
Copy link
Author

Transurgeon commented Jan 2, 2025

Sure, let's get this landed! I took a look at the problem and it seems relatively minor to fix, I left the following comment:

I'm no expert on this stuff, but it seems to be failing due to only wanting one of these to be set at a time. I think you will need to put the two architectures inside the matrix, so you can use something like {{ matrix.arch }} here, and then it will kick off 2x the jobs, one for each arch choice.

Hey @bodono , thanks for your reply. However, I am following the cibuildwheel setup from cvxpy, see here and it seems to allow setting both the linux and macos aarch envs.

Also I noticed that all CI tests are passing except the source distribution. It has something to do with meson builds and not being able to find BLAS? Not sure if this has anything to do with the changes I've added. Could you take a look please?

@bodono
Copy link
Owner

bodono commented Jan 2, 2025

Oh is that the only one failing now? That's nothing to do with this change, I also ran into that error and fixed it here: #122

Probably easiest thing is for me to pull that change out as a separate PR, land it, then you rebase on top (assuming the test pypi upload still gives me trouble).

@Transurgeon
Copy link
Author

Oh is that the only one failing now? That's nothing to do with this change, I also ran into that error and fixed it here: #122

Probably easiest thing is for me to pull that change out as a separate PR, land it, then you rebase on top (assuming the test pypi upload still gives me trouble).

ok sounds good :), let me know if I can be of any help for the pypi upload issue (seems to still be some, looking at your other PR)

@bodono
Copy link
Owner

bodono commented Jan 3, 2025

Ok that change has landed, go ahead and rebase on top and the tests should hopefully pass.

@bodono
Copy link
Owner

bodono commented Jan 3, 2025

Sorry to be a pain here, but I'm looking at what was built in the PR I submitted and I see aarch64 wheels:

https://github.com/bodono/scs-python/actions/runs/12595266757/job/35104211556

I'm wondering if this change is necessary? I never got around to tagging a new release (which might have created new wheels), but can do it asap.

@Transurgeon
Copy link
Author

Sorry to be a pain here, but I'm looking at what was built in the PR I submitted and I see aarch64 wheels:

https://github.com/bodono/scs-python/actions/runs/12595266757/job/35104211556

I'm wondering if this change is necessary? I never got around to tagging a new release (which might have created new wheels), but can do it asap.

You are right maybe it's not necessary, I am not sure. Can you try tagging a new release to see if it would work?

@bodono
Copy link
Owner

bodono commented Jan 4, 2025

It looks to me to have worked: https://pypi.org/project/scs/#files

@Transurgeon
Copy link
Author

seems to have worked on my local (using py13 and linux)

Collecting scs>=3.2.4.post1 (from cvxpy)
  Downloading scs-3.2.7.post2-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (2.1 kB)
Collecting numpy>=1.20 (from cvxpy)
  Downloading numpy-2.2.1-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (62 kB)
Collecting scipy>=1.1.0 (from cvxpy)
  Downloading scipy-1.15.0-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (61 kB)
Collecting qdldl (from osqp>=0.6.2->cvxpy)
  Downloading qdldl-0.1.7.post5-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (1.7 kB)
Downloading cvxpy-1.6.0-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.2 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.2/1.2 MB 3.4 MB/s eta 0:00:00
Using cached clarabel-0.9.0-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.8 MB)
Downloading numpy-2.2.1-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (16.1 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 16.1/16.1 MB 3.2 MB/s eta 0:00:00
Using cached osqp-0.6.7.post3-cp313-cp313-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl (300 kB)
Downloading scipy-1.15.0-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (40.2 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 40.2/40.2 MB 3.4 MB/s eta 0:00:00
Downloading scs-3.2.7.post2-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (10.4 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 10.4/10.4 MB 3.4 MB/s eta 0:00:00
Downloading qdldl-0.1.7.post5-cp313-cp313-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.2 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.2/1.2 MB 3.7 MB/s eta 0:00:00
Installing collected packages: numpy, scipy, scs, qdldl, clarabel, osqp, cvxpy
Successfully installed clarabel-0.9.0 cvxpy-1.6.0 numpy-2.2.1 osqp-0.6.7.post3 qdldl-0.1.7.post5 scipy-1.15.0 scs-3.2.7.post2

Thanks @bodono !

@bodono
Copy link
Owner

bodono commented Jan 6, 2025

Thanks for your help with this @Transurgeon , and also thanks to @enzbus who updated the action to allow aarch64 wheels.

@bodono bodono closed this Jan 6, 2025
@enzbus
Copy link
Contributor

enzbus commented Jan 6, 2025

Yes @bodono this was already done in the PR from last year, but you hadn't made a new tag 🙏👍

@Transurgeon Transurgeon deleted the add-aarch-wheels branch January 6, 2025 19:27
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.

4 participants