-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[CI] Add circleci build and test for arm64. #20460
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
Conversation
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.
Nice!
It means that we can probably remove the following entry in the travis CI config and update the documentation to remove the [arm64]
manual flag in the scikit-learn CI documentation.
https://github.com/scikit-learn/scikit-learn/blob/main/.travis.yml#L33-L40
In a follow-up PRs we might also migrate the full travis config for [cd build]
to ARM-based Circle CI but this is significantly more work because as it would also involve configuring secret tokens. But since we did not run out of credits for travis in a while, maybe that's not needed.
I also wonder if the test failures with |
Note that numpy has similar ARM64 failures ( https://travis-ci.com/github/numpy/numpy/jobs/521809468 and I also observed similar failures today on our travis arm64 build when working with |
We did get the failure on our weekly arm64 build on travis. https://travis-ci.com/github/scikit-learn/scikit-learn/jobs/521594473 but not 8 days ago: https://travis-ci.com/github/scikit-learn/scikit-learn/builds/230982232 |
The last successful arm64 nightly build for numpy was 4 days ago: https://travis-ci.com/github/numpy/numpy/builds/231467572 |
I opened numpy/numpy#19411 to track the issue upstream in numpy but this is probably something that changed in the runtime environment other than numpy / scipy and openblas since those jobs use the same released wheels for those libs... |
I have removed the trigger and the related documentation. |
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. It is quite nice that the CI takes only 25 minutes to build and tests. This is quite impressive indeed. I was recalling that it was almost an hour with the first ARM64 platform.
@ogrisel the |
After investigation, it seems it was fixed in OpenBLAS 0.3.16:
|
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 am not 100% sure why this is working again because we are still using scipy 1.7.0 which had the problem previously... but as long at it works now.
Anyways here are a few suggestion for further improvements:
sudo apt-get install python3-scipy python3-matplotlib libopenblas-base \ | ||
python3-virtualenv ccache | ||
python3 -m virtualenv --system-site-packages --python=python3 testenv | ||
source testenv/bin/activate |
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.
Just in case pip fix bugs in the future, let's use the latest version in the venv:
source testenv/bin/activate | |
source testenv/bin/activate | |
pip install --upgrade pip |
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Something has changed in openmp management? Two azure builds are failing with
|
I made a mistake because the github UI was not up to date and applied some of the review suggestion twice ... I will try to fix. Sorry for the mess. |
I will merge as soon as the last CI run passes. |
It's green! Merging. Thank you very much @cmarmo for the improved CI setup! |
@cmarmo are you interested in a follow-up PR to also migrate the |
Nice Thanks @cmarmo |
This is great work! Thank you @cmarmo :) |
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
What does this implement/fix? Explain your changes.
This pull request adds continuous integration on circleCI for arm64 architectures.
Thanks for considering it.
Any other comments?
We spoke about that during a Consortium meeting but at that time the service was opt-in and needed the organization to subscribe.
I have checked the workflow on my github account, so I guess it is now available to everyone.
Please note that a number of tests are failing.
Also, the list of arm related issues is accessible here.