Skip to content
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.

WIP, MAINT: backport 3.10 support #135

Merged
merged 6 commits into from
Nov 5, 2021

Conversation

tylerjereddy
Copy link
Collaborator

  • try to backport some of the pertinent
    changes for Python 3.10 support to the wheels
    repo v1.7.x branch, which needs to continue
    supporting Python 3.7

  • perhaps we can do away with some of the manylinux
    bumps--not sure on those--things are going to change
    a fair bit anyway with a huge bump in OpenBLAS version
    as well; perhaps the bumps could be confined to just 3.10
    using newer manylinux??

  • point BUILD_COMMIT at the tip of the target maintenance
    branch to get more useful feedback on where we stand
    for the 3.10 wheel builds

* try to backport some of the pertinent
changes for Python 3.10 support to the wheels
repo `v1.7.x` branch, which needs to continue
supporting Python 3.7

* perhaps we can do away with some of the manylinux
bumps--not sure on those--things are going to change
a fair bit anyway with a huge bump in OpenBLAS version
as well; perhaps the bumps could be confined to just 3.10
using newer manylinux??

* point `BUILD_COMMIT` at the tip of the target maintenance
branch to get more useful feedback on where we stand
for the 3.10 wheel builds
@@ -118,7 +142,8 @@ jobs:
- MB_PYTHON_VERSION=3.9
- PLAT=aarch64
- CYTHON_BUILD_DEP="Cython"
- DOCKER_TEST_IMAGE=multibuild/xenial_arm64v8
- DOCKER_TEST_IMAGE=multibuild/focal_{PLAT}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I feel like there was some contention about the need for this bump--if we can test on an older version that may be more valuable for compatibility check?

Copy link
Contributor

Choose a reason for hiding this comment

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

When I created the images, the idea was to build via manylinux which is an ancient system, and test that it works on a modern system to make sure all the backward compatible shims are still in place. So I think we should use the latest stable image available.

amd64-linux-py38:
MB_PYTHON_VERSION: "3.8"
MB_ML_VER: "2014"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if these bumps are harmful, or a least disruptive--I'm somewhat tempted to only use the shiny/new stuff for 3.10, although OpenBLAS version is shooting up several versions (years) anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. The only reason to continue to use manylinux2010 is for very old pip versions. The centos6 base for manylinux2010 went EOL in Nov, 2020. Users of python3.8 will have a pip that is capable of using manylinux2014 wheels.

@tylerjereddy
Copy link
Collaborator Author

I don't see Azure running in the CI matrix list at the time of writing--it is possible I've introduced an error in the yml there perhaps.

@tylerjereddy
Copy link
Collaborator Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

tylerjereddy added a commit to tylerjereddy/scipy that referenced this pull request Oct 27, 2021
* changes related to enabling Python `3.10` support
on the `maintenance/1.7.x` branch based on failures
in the wheels repo here:
MacPython/scipy-wheels#135
* allow azure pipelines to run in PRs against
the v1.7.x branch (at least for debugging purposes..)
@tylerjereddy
Copy link
Collaborator Author

I've re-enabled Azure pipelines to run against the v1.7.x branch, at least for now, for assessment purposes.

@tylerjereddy
Copy link
Collaborator Author

Python 2.4 error (!) showing up in one of the Azure runs:

  File "/usr/lib64/python2.4/site-packages/M2Crypto/SSL/Connection.py", line 167, in connect_ssl
    return m2.ssl_connect(self.ssl, self._timeout)
M2Crypto.SSL.SSLError: tlsv1 alert protocol version

@rgommers
Copy link
Contributor

Python 2.4 error (!) showing up in one of the Azure runs:

Wow, that's impressively outdated stuff:)

* bump the manylinux versions for some Azure
builds to see if we can get around the Python 2.4
errors

[skip travis]
@tylerjereddy
Copy link
Collaborator Author

I've tried bumping from manylinux1 -> manylinux2010 for those Azure matrix entries that were having Python 2.4 errors. Skipping Travis CI for now, while iterating.

@tylerjereddy
Copy link
Collaborator Author

That helped a bit, the remaining errors currently fall into two categories:

  1. Python 3.10 on MacOS has an error like the following, and perhaps a newer MacOS image may be needed there or something like that:
warning: /Applications/Xcode_12.4.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: archive library: build/temp.macosx-10.9-universal2-3.10/libsuperlu_src.a will be fat and ar(1) will not be able to operate on it
xcrun: adding 50 object files to build/temp.macosx-10.9-universal2-3.10/libsuperlu_src.a
ar: build/temp.macosx-10.9-universal2-3.10/libsuperlu_src.a is a fat file (use libtool(1) or lipo(1) and ar(1) on it)
ar: build/temp.macosx-10.9-universal2-3.10/libsuperlu_src.a: Inappropriate file type or format
  1. Python 3.10 elsewhere simply isn't considered a supported version for SciPy until we backport the pyproject.toml/metadata support for it in the main repo.

I'm also wondering if we really can justify universal2 wheels for MacOS Python 3.8 and 3.9? The next planned point release is for 3.10 support, not M1 support yet (those jobs do pass CI though).

@rgommers
Copy link
Contributor

I'm also wondering if we really can justify universal2 wheels for MacOS Python 3.8 and 3.9? The next planned point release is for 3.10 support, not M1 support yet (those jobs do pass CI though).

Yes, we should not ship universal2 wheels unless we know the segfaults are fixed (which they are not at this point).

rgommers pushed a commit to scipy/scipy that referenced this pull request Oct 29, 2021
changes related to enabling Python `3.10` support
on the `maintenance/1.7.x` branch based on failures
in the wheels repo here:
MacPython/scipy-wheels#135
@rgommers
Copy link
Contributor

Python 3.10 on MacOS has an error like the following, and perhaps a newer MacOS image may be needed there or something like that:

This doesn't look familiar to me (but I haven't followed the recent 3.10 work closely).

Python 3.10 elsewhere simply isn't considered a supported version for SciPy until we backport the pyproject.toml/metadata support for it in the main repo.

This is done now in the main repo.

@tylerjereddy
Copy link
Collaborator Author

Awesome, thanks I'll probably merge that in the main repo then revise this PR based on comments and to point to the latest version of the maintenance branch hash so we have an updated view on the situation for 1.7.2.

* we're not ready to ship `universal2` wheels for Python 3.8
and 3.9, so remove those backport additions; for Python 3.10
`universal2` is apparently our only option (based on the comments
I've seen from Chuck at least?)

* still skipping travis CI for now to save credits

* point `BUILD_COMMIT` at latest version of the maintenance
branch to get updated feedback on our status here

[skip travis]
@tylerjereddy
Copy link
Collaborator Author

Ok, latest update pushed--quoting from the commit message:

    * we're not ready to ship `universal2` wheels for Python 3.8
    and 3.9, so remove those backport additions; for Python 3.10
    `universal2` is apparently our only option (based on the comments
    I've seen from Chuck at least?)
    
    * still skipping travis CI for now to save credits
    
    * point `BUILD_COMMIT` at latest version of the maintenance
    branch to get updated feedback on our status here

@tylerjereddy
Copy link
Collaborator Author

@charris @matthew-brett do I understand correctly that for MacOS X Python 3.10 universal2 is our only option, even though I only want to ship Intel Mac support for this point release? I'm basing this just on having read some comments/release information related to NumPy I think.

@charris
Copy link
Contributor

charris commented Oct 30, 2021

MacOS X Python 3.10 universal2 is our only option

That was my understanding. Python 3.10 only supports universal2 installers, but I'm not sure what that means. However, I did notice that thin macosx_11 arm64 wheels were also produced by universal2, so I suspect Intel wheels could be produced, but multibuild won't do it that I know of. Note that multibuild sets the mac build version to 11+ for 3.10 even though the universal2 wheel is marked 10_9. I can't find much documentation on this stuff. If scipy fails on M1 silicon, it may be a problem...

@rgommers
Copy link
Contributor

That was my understanding. Python 3.10 only supports universal2 installers, but I'm not sure what that means.

That cannot be correct. Maybe you mean that Python.org offers only a universal2 installer? If so, I don't see how that is relevant. We should produce wheels that make sense, so also thin x86_64 and arm64 wheels in the future. And for now only a x86_64 one, unless there's some pragmatic reason that that is difficult.

If it is difficult, then shipping universal2 wheels for Python 3.10 alone seems okay, with a note that there may be issues left for arm64 users. It's still better than letting all 3.10 users build from source.

However, I did notice that thin macosx_11 arm64 wheels were also produced by universal2, so I suspect Intel wheels could be produced, but multibuild won't do it that I know of. Note that multibuild sets the mac build version to 11+ for 3.10 even though the universal2 wheel is marked 10_9. I can't find much documentation on this stuff. If scipy fails on M1 silicon, it may be a problem...

Ah. That seems wrong. But well, multibuild is a bit of a pain to update and we really need to ship 1.7.2, so let's just do what is easiest.

@charris
Copy link
Contributor

charris commented Oct 30, 2021

so let's just do what is easiest.

If the Intel wheel problem gets solved, those wheels can always be uploaded later. Universal 3.10 wheels were OK for NumPy, so I stopped trying to get the Intel builds working. There might be a solution there, I just didn't find it in the short time I devoted to it.

@rgommers
Copy link
Contributor

Universal 3.10 wheels were OK for NumPy, so I stopped trying to get the Intel builds working. There might be a solution there, I just didn't find it in the short time I devoted to it.

For NumPy it's fine right now, because there's nothing really wrong with numpy on arm64, tests pass modulo a few warnings. SciPy still has crashes in several modules.

@charris
Copy link
Contributor

charris commented Oct 30, 2021

there's nothing really wrong with numpy on arm64

Yep. If you want, I could do more work on getting the Intel build working. NumPy takes a lot less time to build and test than SciPy does.

EDIT: One of the reasons I stopped trying is that multibuild might need fixing, and there was no easy way to do that while working in numpy-wheels, the commits need to be in the upstream multibuild devel branch.

@rgommers
Copy link
Contributor

EDIT: One of the reasons I stopped trying is that multibuild might need fixing, and there was no easy way to do that while working in numpy-wheels, the commits need to be in the upstream multibuild devel branch.

Yeah I'm inclined to spend time on moving to cibuildwheel instead. Long term, multibuild updates are lost dev time.

@tylerjereddy
Copy link
Collaborator Author

Alright, well let me see if I hit a wall with at least trying to build a regular x86_64 3.10 wheel for Mac, probably in a separate trimmed down debug PR and/or with some local testing..

@tylerjereddy
Copy link
Collaborator Author

Something, I assume multibuild, is really insisting on the dual -arch arm64 -arch x86_64 with clang for Python 3.10. I tried gently circumventing with a multibuild hack but now I'm just trying to delete everything mac-related that uses arm64 in a debug multibuild branch for sanity..

@mattip
Copy link
Contributor

mattip commented Oct 30, 2021

Maybe worth pinging @matthew-brett on how to make x86_64 wheels for macOS

@tylerjereddy
Copy link
Collaborator Author

Ok, it is not looking like multibuild is injecting it directly on my test branch (I purged every opportunity to add it from multibuild). I've read about Xcode 12/the system Python itself automatically injecting the flag, which is what I feared, so will see if one of the described workarounds is viable.. export ARCHFLAGS="-arch x86_64" seems the best bet if I can find just the rigth way to add it (hopefully no prepend vs. append flag nightmares).

@tylerjereddy
Copy link
Collaborator Author

tylerjereddy commented Oct 30, 2021

The Python MacOS 3.10 wheel build appears to succeed in gh-136 with x86_64 arch forced, but there are quite a few test failures in the next stage, especially related to float128 and complex256 array results dtype comparisons in sparse. I'll relay similar info in the main repo to get a few more eyes on it.

universal2 is still incorrectly used for some path names when building the bdist, but seems like an x86_64 build otherwise.

@charris
Copy link
Contributor

charris commented Oct 30, 2021

especially related to float128 and complex256

Interesting. Those both use doubles on M1, no extended/quad precision, and numpy ran into problems with c++ there, but that shouldn't be the case on Intel.

I note that cibuildwheels claims to build Python 3.10 macosx wheels for Intel.

* switch to `x86_64` thin wheel builds
for Mac OS Python `3.10` following upstream
changes/releases in multibuild/NumPy that
support this

* point to the new `multibuild` repo and sync
to latest `devel` (for thin MacOS Python 3.10
x86_64 thin wheel support, etc.)

* backport the full set of `master` branch changes
to `config.sh` based on feedback from Isuru
@tylerjereddy
Copy link
Collaborator Author

Following various upstream updates the following changes have been pushed in here (from the commit message):

    * switch to `x86_64` thin wheel builds
    for Mac OS Python `3.10` following upstream
    changes/releases in multibuild/NumPy that
    support this

    * point to the new `multibuild` repo and sync
    to latest `devel` (for thin MacOS Python 3.10
    x86_64 thin wheel support, etc.)

    * backport the full set of `master` branch changes
    to `config.sh` based on feedback from Isuru

@tupui
Copy link
Contributor

tupui commented Nov 5, 2021

For the failures on macOS, I've seen the same in my PR to update our CI scipy/scipy#14832. But seems like we don't have it anymore.

@tylerjereddy
Copy link
Collaborator Author

@tupui @charris @rgommers I suspect if we're going to use oldest-support-numpy we're either going to have update that repo and do another release there because we need absolute bleeding edge NumPy wheels for MacOS 3.10; alternatively, at least for the backport PR here, might we consider just putting the actual NumPy version we need for that one MacOS Python 3.10 job?

At the moment those failures are identical to before, and almost certainly the result of not using the latest NumPy with the quad/double fixes.

@tupui
Copy link
Contributor

tupui commented Nov 5, 2021

@tupui @charris @rgommers I suspect if we're going to use oldest-support-numpy we're either going to have update that repo and do another release there because we need absolute bleeding edge NumPy wheels for MacOS 3.10; alternatively, at least for the backport PR here, might we consider just putting the actual NumPy version we need for that one MacOS Python 3.10 job?

@tylerjereddy Ralf just updated it so it serves latest 1.21.4. So maybe just relaunch the CI?

@tylerjereddy tylerjereddy reopened this Nov 5, 2021
@tylerjereddy
Copy link
Collaborator Author

Ok, let's just re-flush the CI then. I'd try to restart just that one job, but let's just check across the board..

@tylerjereddy
Copy link
Collaborator Author

So close, just this issue left it seems: scipy/scipy#14985

* try importing multiprocessing in `run_scipy_tests.py` to
address this:
scipy/scipy#14985
@tylerjereddy
Copy link
Collaborator Author

Not sure if it is good fortune or the import added to run_scipy_tests.py allowing appveyor to pass this time, but let's not dwell on that and move forward with the release process..

@tylerjereddy tylerjereddy merged commit 200b295 into MacPython:v1.7.x Nov 5, 2021
@tylerjereddy tylerjereddy deleted the treddy_172_backports branch November 5, 2021 21:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants