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: musllinux_x86_64 #22864

Merged
merged 11 commits into from
Jan 20, 2023
Merged

CI: musllinux_x86_64 #22864

merged 11 commits into from
Jan 20, 2023

Conversation

andyfaff
Copy link
Member

Creates a musllinux_x86_64 test run on github actions, as discussed on the mailing list.

@mattip
Copy link
Member

mattip commented Dec 23, 2022

Thanks. It seems we have errors around divide-by-zero warnings:

>       d = np.absolute(np.arcsinh(x)/np.arcsinh(z).real - 1)
E       RuntimeWarning: divide by zero encountered in divide

dtype      = <class 'numpy.complex64'>
real_dtype = dtype('float32')
rtol       = 2.5033950805664064e-07
x          = array([9.99999968e-21, 1.21736865e-20, 1.48198648e-20, 1.80412373e-20,
       2.19628376e-20, 2.67368701e-20, 3.254862...55170e-04, 4.54267400e-04,
       5.53010846e-04, 6.73218106e-04, 8.19554611e-04, 9.97700030e-04],
      dtype=float32)
z          = array([9.99999968e-21+0.j, 1.21736865e-20+0.j, 1.48198648e-20+0.j,
       1.80412373e-20+0.j, 2.19628376e-20+0.j, 2.67...67400e-04+0.j, 5.53010846e-04+0.j, 6.73218106e-04+0.j,
       8.19554611e-04+0.j, 9.97700030e-04+0.j], dtype=complex64)

As far as I can tell the spurious fp exception tests are passing.

@mattip
Copy link
Member

mattip commented Dec 25, 2022

The summary of failing tests is

FAILED numpy/core/tests/test_mem_policy.py::test_new_policy - AssertionError:...
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_precisions_consistent
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_branch_cuts
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_branch_cuts_complex64
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_loss_of_precision[complex64]
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_loss_of_precision[complex128]
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_loss_of_precision[complex256]
FAILED numpy/linalg/tests/test_linalg.py::TestInv::test_sq_cases - AssertionE...
FAILED numpy/linalg/tests/test_linalg.py::TestInv::test_generalized_sq_cases
FAILED numpy/linalg/tests/test_linalg.py::TestCholesky::test_basic_property[float32-shape3]
FAILED numpy/linalg/tests/test_linalg.py::TestCholesky::test_basic_property[float64-shape3]

I guess the place to start is with the failing umath tests.

@charris charris added 03 - Maintenance component: CI Dependencies Pull requests that update a dependency file and removed Dependencies Pull requests that update a dependency file labels Jan 3, 2023
@rgommers
Copy link
Member

This can only be merged once the job is green. Here are the failures:

FAILED numpy/core/tests/test_mem_policy.py::test_new_policy - AssertionError:...
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_precisions_consistent
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_branch_cuts
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_branch_cuts_complex64
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_loss_of_precision[complex64]
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_loss_of_precision[complex128]
FAILED numpy/core/tests/test_umath.py::TestComplexFunctions::test_loss_of_precision[complex256]
FAILED numpy/linalg/tests/test_linalg.py::TestInv::test_sq_cases - AssertionE...
FAILED numpy/linalg/tests/test_linalg.py::TestInv::test_generalized_sq_cases
FAILED numpy/linalg/tests/test_linalg.py::TestCholesky::test_basic_property[float32-shape3]
FAILED numpy/linalg/tests/test_linalg.py::TestCholesky::test_basic_property[float64-shape3]

I'd say that the TestComplexFunctions are not very interesting, let's mark them as xfail for musl. The test_mem_policy.py::test_new_policy is also not a blocker, I suggest to xfail as well and open a separate issue for it. A bonus would be to include the Docker commands to reproduce the failure locally.

That leaves only the four linalg ones. I don't know what is going on there, but it looks important to check that soon. It looks similar to failures reported on macOS arm64 (conda-forge/numpy-feedstock#287 (comment)), so very likely there's a real bug there that needs fixing soon. Let's also mark that xfail on musl and open a separate issue.

@seberg
Copy link
Member

seberg commented Jan 19, 2023

Oh, the linalg failures indeed look bad, OTOH, I see this setup:

Run apk update --quiet
[22](https://github.com/numpy/numpy/actions/runs/3768324456/jobs/6406661512#step:3:23)
(1/3) Installing openblas (0.3.9-r2)
[23](https://github.com/numpy/numpy/actions/runs/3768324456/jobs/6406661512#step:3:24)
(2/3) Installing openblas-ilp64 (0.3.9-r2)
[24](https://github.com/numpy/numpy/actions/runs/3768324456/jobs/6406661512#step:3:25)
(3/3) Installing openblas-dev (0.3.9-r2)

And honestly openblas 0.3.9 is very old and we shouldn't be using it.

@andyfaff
Copy link
Member Author

I'll mark them all as xfail, and create separate issues for fixing them. The docker commands will be a simple copy and paste of the CI run. I won't be able to make the actual fix though.

@seberg
Copy link
Member

seberg commented Jan 19, 2023

Can you see if we could update openblas easily? I would prefer that a lot. They look like potentially serious and correct issues that correctly warn about using that version of openblas.

@andyfaff
Copy link
Member Author

And honestly openblas 0.3.9 is very old and we shouldn't be using it.

The openblas_setup in numpy and scipy doesn't deal with musllinux and needs to be updated. They are available for download from multibuild-wheels-staging.

@rgommers
Copy link
Member

The openblas_setup in numpy and scipy doesn't deal with musllinux and needs to be updated. They are available for download from multibuild-wheels-staging.

Can't we just use system packages? Those are better than the home-baked wheel libs. Alpine has openblas at 0.3.21: https://pkgs.alpinelinux.org/packages?name=openblas&branch=edge&repo=&arch=&maintainer=

@andyfaff
Copy link
Member Author

andyfaff commented Jan 19, 2023

Can't we just use system packages

alpine:latest has 0.3.21. The PR as currently exists uses the quay.io/pypa/musllinux_1_1_x86_64 image, which is more akin to what would be used for cibuildwheel run. That older image only has openblas=0.3.9.
My thought was that it'd be better to use the actual image that would be used to build a wheel.

I can either use the more recent alpine image or update the openblas_support file a la https://github.com/scipy/scipy/pull/17323/files. (the file needs updating anyway)

@rgommers
Copy link
Member

Ah okay. https://anaconda.org/multibuild-wheels-staging/openblas-libs/files has 0.3.21 musllinux packages, so in that case updating openblas_support is a small effort, and worth a first try. I had assumed the needed openblas builds were still missing from github.com/macPython/openblas-libs/.

tools/openblas_support.py Outdated Show resolved Hide resolved
@mattip
Copy link
Member

mattip commented Jan 19, 2023

CPython is still trying to decide how to handle expressing linking to musl libc instead of glibc. There is python/cpython#87278 which is discussing whether to change SOABI. I commented there about maybe changing _multiarch.

@andyfaff
Copy link
Member Author

I think this should be good to go now. I've marked the relevant tests as xfail and have opened issues for them. Let me know if you'd like to squash the commits, if squash merge isn't preferred.

@mattip
Copy link
Member

mattip commented Jan 20, 2023

LGTM, thanks.

The macos failure is a transient network problem when trying to pip install. We might want to add some caching to avoid those types of failures, definitely not something for this PR.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM too. Thanks a lot @andyfaff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants