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

Build wheels for x86_64, intel and arm64 macOS #7204

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

andreas-el
Copy link
Contributor

@andreas-el andreas-el commented Feb 16, 2024

At this point i'm unsure if we should fix occasionally failing tests or mark them.
I have enabled building of wheels and testing according to the table provided. #7204 (comment)

Screenshot 2024-02-19 at 08 22 57

ref.
https://github.com/equinor/komodo-releases/actions/runs/7932266339


  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure tests pass locally (after every commit!)

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Create Backport PR to latest release

@andreas-el

This comment was marked as resolved.

@pinkwah
Copy link
Contributor

pinkwah commented Feb 19, 2024

Should we not test Intel macs?

@andreas-el andreas-el self-assigned this Feb 19, 2024
@andreas-el
Copy link
Contributor Author

andreas-el commented Feb 19, 2024

Edit: This seems to be resolved somehow.

Currently fails due to precision.

/Users/runner/work/ert/ert/src/clib/tests/enkf/test_row_scaling.cpp:141: FAILED:
  CHECK( A(row, col) == Approx(expected).epsilon(1e-12) )
with expansion:
  -0.0000018113 == Approx( -0.0000018113 )

@andreas-el
Copy link
Contributor Author

Should we not test Intel macs?

Good question.
My impression was that these workflows should match what the developers were using, but there might be some devs still using intel architecture still (for macOS).
Also, should we build wheels for all python versions, and all os + architecture combinations?
I think it would be nice to get some clarification regarding this.

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.26%. Comparing base (7f656ef) to head (9d3d09b).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7204      +/-   ##
==========================================
+ Coverage   84.55%   85.26%   +0.70%     
==========================================
  Files         382      382              
  Lines       22770    22770              
  Branches      886      891       +5     
==========================================
+ Hits        19253    19414     +161     
+ Misses       3404     3250     -154     
+ Partials      113      106       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sondreso
Copy link
Collaborator

sondreso commented Feb 19, 2024

We still have developers on macos intel, so we need to support this.

I borrowed this overview to illustrate which platforms we should build wheels for (this is the same as the latest release):

macOS Intel macOS Apple Silicon Windows 64bit Windows 32bit Windows Arm64 manylinux
musllinux x86_64
manylinux
musllinux i686
manylinux
musllinux aarch64
manylinux
musllinux ppc64le
manylinux
musllinux s390x
CPython 3.6 N/A N/A
CPython 3.7 N/A N/A
CPython 3.8 N/A
CPython 3.9
CPython 3.10
CPython 3.11
CPython 3.12
PyPy 3.7 v7.3 N/A N/A N/A N/A N/A
PyPy 3.8 v7.3 N/A N/A N/A N/A
PyPy 3.9 v7.3 N/A N/A N/A N/A
PyPy 3.10 v7.3 N/A N/A N/A N/A
  • Note 1: We are not building for musllinux.
  • Note 2: We should not publish a source distribution.

In PRs, I think we should test against:

  • Oldest Python version on all platforms (3.8 on macos intel, 3.8 on linux x86_64), but we skip Python 3.10 on Mac Arm
  • Newest Python version on all platforms (3.12)
  • Current and future Python versions for internal users on target platform linux x86_64 (Python 3.8 and 3.11)

Giving the following test matrix for PRs:

macOS Intel macOS Apple Silicon Windows 64bit Windows 32bit Windows Arm64 manylinux
musllinux x86_64
manylinux
musllinux i686
manylinux
musllinux aarch64
manylinux
musllinux ppc64le
manylinux
musllinux s390x
CPython 3.6 N/A N/A
CPython 3.7 N/A N/A
CPython 3.8 🔄 N/A 🔄
CPython 3.9
CPython 3.10
CPython 3.11 🔄
CPython 3.12 🔄 🔄 🔄
PyPy 3.7 v7.3 N/A N/A N/A N/A N/A
PyPy 3.8 v7.3 N/A N/A N/A N/A
PyPy 3.9 v7.3 N/A N/A N/A N/A
PyPy 3.10 v7.3 N/A N/A N/A N/A

Sounds reasonable? 🙂

andreas-el added a commit to andreas-el/ert that referenced this pull request Feb 20, 2024
Focus testing mostly on python 3.8, 3.11, 3.12

See table overview;
equinor#7204 (comment)
@andreas-el andreas-el force-pushed the upgrade_runners_to_arm branch from d88a98f to cbab164 Compare February 20, 2024 07:47
andreas-el added a commit to andreas-el/ert that referenced this pull request Feb 20, 2024
Focus testing mostly on python 3.8, 3.11, 3.12

See table overview;
equinor#7204 (comment)
@andreas-el andreas-el force-pushed the upgrade_runners_to_arm branch from cbab164 to 5cf6054 Compare February 20, 2024 08:02
@andreas-el andreas-el changed the title Move from x64 mac runners to arm runners Build wheels for x86_64, intel and apple macOS Feb 20, 2024
@andreas-el
Copy link
Contributor Author

andreas-el commented Feb 20, 2024

Lint GitHub Actions workflows / Check all workflow files fails

.github/workflows/build_and_test.yml:23:43: label "macos-latest-xlarge" is unknown. available labels are "windows-latest", "windows-2022", "windows-2019", "windows-2016", "ubuntu-latest", "ubuntu-22.04", "ubuntu-20.04", "ubuntu-18.04", "macos-latest", "macos-latest-xl", "macos-13-xl", "macos-13", "macos-13.0", "macos-12-xl", "macos-12", "macos-12.0", "macos-11", "macos-11.0", "macos-10.15", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file [runner-label]
   |
23 |         os: ['ubuntu-latest', 'macos-13', 'macos-latest-xlarge']

Additional macOS labels were added recently, only awaiting a new release for actionlint.
rhysd/actionlint#392

edit: This was resolved in the latest version of actionlint

@andreas-el andreas-el added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Feb 20, 2024
andreas-el added a commit to andreas-el/ert that referenced this pull request Feb 20, 2024
Focus testing mostly on python 3.8, 3.11, 3.12

See table overview;
equinor#7204 (comment)

Brew hdf5 when macOS
Lift exclusion ubuntu-3.11-gui-tests
@andreas-el andreas-el force-pushed the upgrade_runners_to_arm branch from 2b09a74 to 143b9b7 Compare February 20, 2024 20:31
@andreas-el andreas-el changed the title Build wheels for x86_64, intel and apple macOS Build wheels for x86_64, intel and arm64 macOS Feb 28, 2024
andreas-el added a commit that referenced this pull request Mar 4, 2024
Focus testing mostly on python 3.8, 3.11, 3.12

See table overview;
#7204 (comment)

Brew hdf5 when macOS
Lift exclusion ubuntu-3.11-gui-tests
@andreas-el andreas-el force-pushed the upgrade_runners_to_arm branch from 143b9b7 to 9d3d09b Compare March 4, 2024 13:50
@andreas-el andreas-el requested a review from eivindjahren March 4, 2024 16:40
Copy link
Contributor

@eivindjahren eivindjahren left a comment

Choose a reason for hiding this comment

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

10/10

andreas-el added a commit to andreas-el/ert that referenced this pull request Mar 5, 2024
Focus testing mostly on python 3.8, 3.11, 3.12

See table overview;
equinor#7204 (comment)

Brew hdf5 when macOS
Lift exclusion ubuntu-3.11-gui-tests
@andreas-el andreas-el force-pushed the upgrade_runners_to_arm branch from 9d3d09b to 83d35cc Compare March 5, 2024 08:38
Focus testing mostly on python 3.8, 3.11, 3.12

See table overview;
equinor#7204 (comment)

Brew hdf5 when macOS
@andreas-el andreas-el force-pushed the upgrade_runners_to_arm branch from 83d35cc to ee501e9 Compare March 5, 2024 08:52
@andreas-el andreas-el merged commit 2048dfe into equinor:main Mar 5, 2024
66 checks passed
@andreas-el andreas-el deleted the upgrade_runners_to_arm branch March 5, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:maintenance Automatically categorise as maintenance change in release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants