Skip to content

provide macos universal2 wheel#4642

Merged
jcwchen merged 4 commits intoonnx:mainfrom
daquexian:macos_universal2
Nov 13, 2022
Merged

provide macos universal2 wheel#4642
jcwchen merged 4 commits intoonnx:mainfrom
daquexian:macos_universal2

Conversation

@daquexian
Copy link
Copy Markdown
Member

@daquexian daquexian commented Nov 9, 2022

Description

Fixes #4436

It is easier than I thought to build macos universal2 binaries. A proper CMAKE_OSX_ARCHITECTURES is all we need 😂

I have tested the universal2 wheel on my m1 machine.

@daquexian
build universal2
bbfe93e 
Signed-off-by: daquexian <daquexian566@gmail.com>
@daquexian
support both universal2 and x86_64
48bc193 
Signed-off-by: daquexian <daquexian566@gmail.com>
@daquexian daquexian added the topic: build Issues related to ONNX builds and packages label Nov 9, 2022
@daquexian daquexian requested a review from a team as a code owner November 9, 2022 11:13
@daquexian daquexian mentioned this pull request Nov 9, 2022
Closed
@jcwchen jcwchen added the run release CIs Use this label to trigger release tests in CI label Nov 9, 2022
@jcwchen jcwchen self-assigned this Nov 9, 2022
@jcwchen
Copy link
Copy Markdown
Member

jcwchen commented Nov 9, 2022

Thank you for the contribution! Please allow me to reopen this PR to trigger release CIs

@jcwchen jcwchen closed this Nov 9, 2022
@jcwchen jcwchen reopened this Nov 9, 2022
@jcwchen jcwchen added this to the 1.13 milestone Nov 9, 2022
jcwchen
jcwchen approved these changes Nov 9, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. I also tested this universal2 wheel on my Mac (non M1/M2) and it worked well.

Comment thread .github/workflows/release_mac.yml
python setup.py bdist_wheel -p macosx_10_12_${{ matrix.target-architecture }} --weekly_build
else
python setup.py bdist_wheel -p macosx_10_12_x86_64
python setup.py bdist_wheel -p macosx_10_12_${{ matrix.target-architecture }}
Copy link
Copy Markdown
Member

@jcwchen jcwchen Nov 9, 2022

Choose a reason for hiding this comment

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

nit: not sure whether we want to use macosx_10_12_ for universal2 as well. Probably we can assume only 10.15+ users will try universal2 wheel.

Copy link
Copy Markdown
Member Author

@daquexian daquexian Nov 10, 2022

Choose a reason for hiding this comment

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

Thanks for your review! It is what I ignored.

What's the reason for choosing 10.15? The minimum macos version that can run on apple silicon chips seems 11. Maybe we can use macosx_11_0_.

Copy link
Copy Markdown
Member

@jcwchen jcwchen Nov 10, 2022

Choose a reason for hiding this comment

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

Good point. macosx_11_0_ looks better. Thanks!

Copy link
Copy Markdown
Member Author

@daquexian daquexian Nov 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I updated the code and it's a little strange that CI fails. I'll fix it tomorrow.

Copy link
Copy Markdown
Member Author

@daquexian daquexian Nov 13, 2022

Choose a reason for hiding this comment

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

@jcwchen I found that python3.7 is incompatible with macosx_11_0_.

$ python3.7 -m pip install onnx-1.12.0-cp37-cp37m-macosx_11_0_x86_64.whl
ERROR: onnx-1.12.0-cp37-cp37m-macosx_11_0_x86_64.whl is not a supported wheel on this platform.
$ python3.7 -m pip install onnx-1.12.0-cp37-cp37m-macosx_10_15_x86_64.whl
Processing ...
$ python3.8 -m pip install onnx-1.12.0-cp38-cp38-macosx_11_0_x86_64.whl
Processing ...

So I prefer keeping the versions of x86_64 and universal2 wheels the same. It is also what numpy does:

image

@jcwchen jcwchen enabled auto-merge (squash) November 13, 2022 16:48
@jcwchen jcwchen merged commit 70c4a7d into onnx:main Nov 13, 2022
justinchuby pushed a commit to justinchuby/onnx that referenced this pull request Jan 27, 2023
@daquexian @jcwchen @justinchuby
provide macos universal2 wheel (onnx#4642)
2a5e3ad 
* build universal2

Signed-off-by: daquexian <daquexian566@gmail.com>

* support both universal2 and x86_64

Signed-off-by: daquexian <daquexian566@gmail.com>

Signed-off-by: daquexian <daquexian566@gmail.com>
Co-authored-by: Chun-Wei Chen <jacky82226@gmail.com>
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
@daquexian @jcwchen
provide macos universal2 wheel (onnx#4642)
e4f5988 
* build universal2

Signed-off-by: daquexian <daquexian566@gmail.com>

* support both universal2 and x86_64

Signed-off-by: daquexian <daquexian566@gmail.com>

Signed-off-by: daquexian <daquexian566@gmail.com>
Co-authored-by: Chun-Wei Chen <jacky82226@gmail.com>
kmantel pushed a commit to kmantel/onnx that referenced this pull request Aug 24, 2023
@daquexian @jcwchen @kmantel
provide macos universal2 wheel (onnx#4642)
88d3ae5 
* build universal2

Signed-off-by: daquexian <daquexian566@gmail.com>

* support both universal2 and x86_64

Signed-off-by: daquexian <daquexian566@gmail.com>

Signed-off-by: daquexian <daquexian566@gmail.com>
Co-authored-by: Chun-Wei Chen <jacky82226@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcwchen jcwchen jcwchen approved these changes

Assignees

@jcwchen jcwchen

Labels

run release CIs Use this label to trigger release tests in CI topic: build Issues related to ONNX builds and packages

Projects

PR Tracker
Status: Done

Milestone

1.13.0

Development

Successfully merging this pull request may close these issues.

Add Apple Silicon support (M1/M2 machines): provide universal2 wheel

2 participants

@daquexian @jcwchen