-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Build aarch64 wheels #5197
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 aarch64 wheels #5197
Conversation
I had to separate each of the python build jobs. Since QEMU is used to emulate aarch64, the build time for each python version took around 2-3 hours. Github Actions times out at 6 hours. To successfully build all the wheels, it was necessary to run all the wheel builds in parallel. |
@@ -30,10 +179,9 @@ jobs: | |||
|
|||
- name: Install cibuildwheel | |||
run: | | |||
python -m pip install cibuildwheel==1.6.3 | |||
python -m pip install cibuildwheel |
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
From v1.8.0, cibuildwheel allows to build non-native architecture using the CIBW_ARCHS_LINUX option.
Updated to consolidate linux 3.7, 3.8 and 3.9 python jobs into 1 build step that spawns parallel jobs. |
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.
@janaknat Thank you so much for taking the initiative here again! 🙏🙏🙏 I only have one question about the 36 job — happy to merge temporarily, push out a new 0.17, and then remove, I just want to make sure we're on the same page. Going forward, we don't intend to support 3.6, nor the earlier 0.17 branch.
.github/workflows/cibuildwheel.yml
Outdated
@@ -10,13 +10,89 @@ on: | |||
- v* | |||
|
|||
jobs: | |||
build_wheels: | |||
build_linux_36_wheels: |
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.
@janaknat Do I interpret this action correctly that it only builds wheels for 36 on the 0.17 branch? Would you like us to push out a final 0.17.x release, after which this job can be deleted? Or am I misinterpreting this?
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 went by what was already present in the cibuildwheel.yml file for x86. I just checked PyPI and it looks like python 3.6 didn't have any wheels. Then I would go ahead and only release 3.7, 3.8 and 3.9 wheels. I can update the PR to remove the config for python 3.6.
Support for python 3.6 is ending for scikit-image.
@jni I've added a patch that removes the python 3.6 build config. |
@scikit-image/core anyone else want to review/merge this? |
I'll ask if @hmaarrfk has some input before merging, Juan 🙂 |
Looks great sorry! |
@hmaarrfk Thanks for merging. Any dates on when the aarch64 wheels show up in PyPI? |
@meeseeksdev backport to v0.18.x |
…7-on-v0.18.x Backport PR #5197 on branch v0.18.x (Build aarch64 wheels)
The backported wheels from #5210 failed to build for 3.9, which means no wheels were uploaded. If anyone has ideas for uploading separate artifacts for each wheel build, please suggest them, because that would save a lot of headaches! Maybe we should separate all these into separate GHA workflows? In the meantime I'm restarting the build. I've saved the log from the failed build here: https://gist.github.com/jni/63fd707a5e13a15f66a50276def316d8 The error seemed suspicious (below), but the 3.9 wheels built on master so I still hope it's just a sporadic failure. 3.9 wheel test error
|
We could potentially build locally. I guess the issue with githu actions is that they are tough to run locally. |
(a) It tragically looks like the error is reproducible: https://github.com/scikit-image/scikit-image/runs/1803653523?check_suite_focus=true#step:6:596 I wonder if this is an issue with NumPy 1.20, which was just released? They've been making a lot of changes in OpenBLAS which seems to be the source of this error. (b) @hmaarrfk there's an app for that. =P https://github.com/nektos/act It is onerous enough to set up that I haven't done so yet, but I think I will have to bite the bullet shortly... |
Ah, I also just noticed (c) actually, the finished workflows do upload wheels anyway, so we have 3.7 and 3.8 aarch wheels, which is nice. But (d) I did bump the version number on the 0.18.x branch after releasing 0.18.1, so we will need to cut a new release once we figure this out. I don't think it's that onerous now that all the kinks have been ironed out. I don't want to cut a new release where we don't release 3.9 Linux wheels, so we have three options:
|
Another observation: the build on master did work, see here: https://github.com/scikit-image/scikit-image/actions/runs/523589320 that happened prior to the NumPy 1.20 release, and the failure does not appear to happen in skimage code that has changed since 0.18, so that's starting to smell a lot like a NumPy 1.20 issue... I'll be curious to see whether the build fails on the next merge to master or next nightly build. |
Strange that it only failed on 3.9.... can the issue be seen on the master branch now? |
@jni It would be good to have python 3.9 wheels. However, it is better to release wheels that work and aim to get 3.9 fixed. Don't most distros ship with python 3.6 - 3.8 as default? |
Hmm, I thought we had nightly wheel builds but that doesn't seem to have been the case. Anyway, just merged a PR, let's monitor this build for the failure: https://github.com/scikit-image/scikit-image/actions/runs/529162467 |
Failure: ____________________ test_multiscale_basic_features_channel ____________________
def test_multiscale_basic_features_channel():
img = np.zeros((10, 10, 5))
img[:10] = 1
img += 0.05 * np.random.randn(*img.shape)
n_sigmas = 2
features = multiscale_basic_features(img, sigma_min=1, sigma_max=2, multichannel=True)
assert features.shape[-1] == 5 * n_sigmas * 4
assert features.shape[:-1] == img.shape[:-1]
# Consider last axis as spatial dimension
> features = multiscale_basic_features(img, sigma_min=1, sigma_max=2)
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/tests/test_basic_features.py:28:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/_basic_features.py:171: in multiscale_basic_features
features = list(itertools.chain.from_iterable(all_results))
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/_basic_features.py:159: in <genexpr>
_mutiscale_basic_features_singlechannel(
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/_basic_features.py:87: in _mutiscale_basic_features_singlechannel
out_sigmas = list(
/opt/python/cp39-cp39/lib/python3.9/concurrent/futures/_base.py:600: in result_iterator
yield fs.pop().result()
/opt/python/cp39-cp39/lib/python3.9/concurrent/futures/_base.py:433: in result
return self.__get_result()
/opt/python/cp39-cp39/lib/python3.9/concurrent/futures/_base.py:389: in __get_result
raise self._exception
/opt/python/cp39-cp39/lib/python3.9/concurrent/futures/thread.py:52: in run
result = self.fn(*self.args, **self.kwargs)
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/_basic_features.py:89: in <lambda>
lambda s: _singlescale_basic_features_singlechannel(
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/_basic_features.py:28: in _singlescale_basic_features_singlechannel
results += (*_texture_filter(gaussian_filtered),)
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/_basic_features.py:14: in _texture_filter
eigvals = feature.hessian_matrix_eigvals(H_elems)
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/corner.py:413: in hessian_matrix_eigvals
return _symmetric_compute_eigenvalues(H_elems)
/tmp/tmp.ZJZYgfG740/venv/lib/python3.9/site-packages/skimage/feature/corner.py:275: in _symmetric_compute_eigenvalues
eigs = np.linalg.eigvalsh(matrices)[..., ::-1]
<__array_function__ internals>:5: in eigvalsh
???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
a = array([[[[[-3.51607800e-04, -2.20549107e-03, 1.62541866e-04],
[-2.20549107e-03, -1.12116337e-04, -9.8598003...[-8.58545303e-04, 6.41047955e-04, -8.41438770e-04],
[-2.33471394e-04, -8.41438770e-04, 1.06841326e-03]]]]])
UPLO = 'L'
@array_function_dispatch(_eigvalsh_dispatcher)
def eigvalsh(a, UPLO='L'):
"""
Compute the eigenvalues of a complex Hermitian or real symmetric matrix.
Main difference from eigh: the eigenvectors are not computed.
Parameters
----------
a : (..., M, M) array_like
A complex- or real-valued matrix whose eigenvalues are to be
computed.
UPLO : {'L', 'U'}, optional
Specifies whether the calculation is done with the lower triangular
part of `a` ('L', default) or the upper triangular part ('U').
Irrespective of this value only the real parts of the diagonal will
be considered in the computation to preserve the notion of a Hermitian
matrix. It therefore follows that the imaginary part of the diagonal
will always be treated as zero.
Returns
-------
w : (..., M,) ndarray
The eigenvalues in ascending order, each repeated according to
its multiplicity.
Raises
------
LinAlgError
If the eigenvalue computation does not converge.
See Also
--------
eigh : eigenvalues and eigenvectors of real symmetric or complex Hermitian
(conjugate symmetric) arrays.
eigvals : eigenvalues of general real or complex arrays.
eig : eigenvalues and right eigenvectors of general real or complex
arrays.
scipy.linalg.eigvalsh : Similar function in SciPy.
Notes
-----
.. versionadded:: 1.8.0
Broadcasting rules apply, see the `numpy.linalg` documentation for
details.
The eigenvalues are computed using LAPACK routines ``_syevd``, ``_heevd``.
Examples
--------
>>> from numpy import linalg as LA
>>> a = np.array([[1, -2j], [2j, 5]])
>>> LA.eigvalsh(a)
array([ 0.17157288, 5.82842712]) # may vary
>>> # demonstrate the treatment of the imaginary part of the diagonal
>>> a = np.array([[5+2j, 9-2j], [0+2j, 2-1j]])
>>> a
array([[5.+2.j, 9.-2.j],
[0.+2.j, 2.-1.j]])
>>> # with UPLO='L' this is numerically equivalent to using LA.eigvals()
>>> # with:
>>> b = np.array([[5.+0.j, 0.-2.j], [0.+2.j, 2.-0.j]])
>>> b
array([[5.+0.j, 0.-2.j],
[0.+2.j, 2.+0.j]])
>>> wa = LA.eigvalsh(a)
>>> wb = LA.eigvals(b)
>>> wa; wb
array([1., 6.])
array([6.+0.j, 1.+0.j])
"""
UPLO = UPLO.upper()
if UPLO not in ('L', 'U'):
raise ValueError("UPLO argument must be 'L' or 'U'")
extobj = get_linalg_error_extobj(
_raise_linalgerror_eigenvalues_nonconvergence)
if UPLO == 'L':
gufunc = _umath_linalg.eigvalsh_lo
else:
gufunc = _umath_linalg.eigvalsh_up
a, wrap = _makearray(a)
_assert_stacked_2d(a)
_assert_stacked_square(a)
t, result_t = _commonType(a)
signature = 'D->d' if isComplexType(t) else 'd->d'
> w = gufunc(a, signature=signature, extobj=extobj)
E ValueError: On entry to DSYEVD parameter number 8 had an illegal value |
this might be an error in blas or lapack. Maybe we should skip on aarch64? |
We can probably use shippable with
to enable aarch64 testing. |
Could you break this out into a stand-alone test that we can run against NumPy? It may be due to the CPU model identification in OpenBLAS failing for qemu, so it would be nice if we could reproduce. |
@jni Any updates on scikit-image aarch64 wheels being available on PyPI? |
Lets see if backporting: Improves the builds |
@hmaarrfk Any updates on scikit-image arm64 wheels? |
#5299 is the latest. |
From v1.8.0, cibuildwheel allows to build non-native architecture using the CIBW_ARCHS_LINUX option.
Build log: https://github.com/janaknat/scikit-image/actions/runs/510106566
Description
Checklist
./doc/examples
(new features only)./benchmarks
, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py
.doc/release/release_dev.rst
.