Skip to content

DOC: update toolchain.rst after EOL of manylinux_2_24; allow C++17#16589

Merged
rgommers merged 3 commits intoscipy:mainfrom
h-vetinari:toolchain
Jul 23, 2022
Merged

DOC: update toolchain.rst after EOL of manylinux_2_24; allow C++17#16589
rgommers merged 3 commits intoscipy:mainfrom
h-vetinari:toolchain

Conversation

@h-vetinari
Copy link
Copy Markdown
Member

Here's the promised toolchain update now that manylinux_2_24 has reached EOL. For the first time in a very long time (ever?), this means the C/C++ support is not restricted by the compiler versions on the main platforms (linux, osx, windows). I've tried to make a best effort to document the state of affairs on more niche platforms as well.

Big question here is what compiler version we want to raise the minimum to, especially for GCC but also LLVM (for the latter, I wonder if we want to specify a minimum at all, because we'd have to add a CI job to ensure it's not a lie). I believe it would be fine to bump to GCC 8.x, though arguably anything between 7 & 10 (inclusive) is fair game based on the compiler versions available on non-EOL platforms (and how much weight to ascribe to certain niche platforms / uses). Any bump beyond GCC 6 would allow us to use C++17, and fix things like #15136, so I think 7 is the barest minimum.

I expect this to need some hashing out (especially the rationale, and how much to document it), so this is only a draft for now.

CC @rgommers

Comment thread doc/source/dev/toolchain.rst
Comment thread doc/source/dev/toolchain.rst Outdated
Comment on lines +286 to +291
Finally there is the question of which machines are widely used by people
needing to compile SciPy from source for other reasons (e.g. SciPy developers,
or people wanting to compile for themselves or performance reasons). The oldest
relevant distributions (without RHEL-style backports) are Ubuntu 18.04 LTS
(which has GCC 7 but also has a backport of GCC 8; Ubuntu 20.04 LTS has GCC 9)
and Debian Buster (with GCC 8; Bullseye has GCC 10).
Copy link
Copy Markdown
Member Author

@h-vetinari h-vetinari Jul 12, 2022

Choose a reason for hiding this comment

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

I'm hesitant how much weight to ascribe to this use case, but thought I'd document it. IMO, developers and power users can be expected to use backports and/or up-to-date software.

In particular, Ubuntu 18.04 and Debian Buster can use manylinux_2014 wheels, and all other platforms (see paragraph above) at least have GCC 10 installable through the distro.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed. The key thing we should add is clear error messages and bail out immediately when a compiler is too old, rather than just getting a cryptic error halfway through. This is easy to do now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll keep this unresolved for now, because this is the most likely point to come up when we merge the GCC >= 8 version check. It shows that we did consider this, and what the solution is.

Comment thread doc/source/dev/toolchain.rst Outdated
@h-vetinari h-vetinari added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org C/C++ Items related to the internal C/C++ code base labels Jul 12, 2022
Copy link
Copy Markdown
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.

This is excellent @h-vetinari. I don't have changes to suggest; just holding off on hitting Approve since the PR is still draft and we should leave this open for discussion for a little while.

Some thoughts after reading through pypa/manylinux#1332:

  • thanks for pushing for manylinux_2_24 EOL @h-vetinari
  • either way don't have to care, since we now have manylinux2014 and manylinux2010 wheels only (see https://pypi.org/project/scipy/1.9.0rc2/#files), have no reason to add manylinux_2_24 in the first place, so the next thing we'll add is manylinux_2_28.

Cc @r-devulap this may interest you, since we talked about GCC versions for NumPy wheels yesterday. Just short of GCC 12 here it looks like, but we're getting close - and lots of info in this PR about relevant constraints. I think to get GCC 12, what is needed is add new Docker images in https://github.com/pypa/manylinux that effectively define what the next manylinux_2_xx version is.

Comment thread doc/source/dev/toolchain.rst
Comment thread doc/source/dev/toolchain.rst Outdated
Comment on lines +286 to +291
Finally there is the question of which machines are widely used by people
needing to compile SciPy from source for other reasons (e.g. SciPy developers,
or people wanting to compile for themselves or performance reasons). The oldest
relevant distributions (without RHEL-style backports) are Ubuntu 18.04 LTS
(which has GCC 7 but also has a backport of GCC 8; Ubuntu 20.04 LTS has GCC 9)
and Debian Buster (with GCC 8; Bullseye has GCC 10).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed. The key thing we should add is clear error messages and bail out immediately when a compiler is too old, rather than just getting a cryptic error halfway through. This is easy to do now.

@rgommers rgommers added this to the 1.10 milestone Jul 13, 2022
@rgommers
Copy link
Copy Markdown
Member

@mckib2's closing of MacPython/scipy-wheels#111 just reminded me of that effort/discussion. I don't see a hole in the much bigger move proposed here right now; it's also easier to do now with sane compiler detection so we can error out cleanly.

That said, we do need to finish moving to cibuildwheel, more messing with multibuild is going to be quite a waste of time.

@h-vetinari
Copy link
Copy Markdown
Member Author

I think to get GCC 12, what is needed is add new Docker images in https://github.com/pypa/manylinux that effectively define what the next manylinux_2_xx version is.

Not even that, just the equivalent of pypa/manylinux#1269 to do GCC 11-->12 for manylinux_2_28. once the respective devtoolset becomes available for RHEL (and then makes its way through to AlmaLinux).

Pushing for the next manylinux standard is probably premature, though of course possible if required for a specific usecase (though even RHEL 9 does not have GCC 12 backports yet).

@rgommers rgommers modified the milestones: 1.10, 1.10.0 Jul 14, 2022
@h-vetinari
Copy link
Copy Markdown
Member Author

h-vetinari commented Jul 15, 2022

This is excellent @h-vetinari. I don't have changes to suggest; just holding off on hitting Approve since the PR is still draft and we should leave this open for discussion for a little while.

Thanks! I've sent a recap to the mailing list asking for feedback. :)

The last commit now proposes the lower bounds of GCC 8 and LLVM 12. For the latter I've just taken what we use in the wheelbuilder repo as a baseline, though we could weaken this to say that anything below what's in our CI is not tested directly.

@tylerjereddy
Copy link
Copy Markdown
Contributor

Sounds good. Will C++17 parallel execution policies be allowed/encouraged? I discussed this a bit with Thomas today re: the "level" at which parallelism happens/gets controlled in the ecosystem. Perhaps for a separate discussion though.

One thing to like about being able to use the parallel-native options for a given compiled language is that some of them (i.e., Rust) actually provide substantial correctness checking for concurrent logic, but there's also a balance to strike with over-subscription/control of parallelism (I think as long as you can feed in a keyword switch in the Python API that gets passed down you're probably not causing too much trouble?).

@h-vetinari
Copy link
Copy Markdown
Member Author

Will C++17 parallel execution policies be allowed/encouraged?

That's actually the last really major piece where LLVM is not yet ready. They've included Intel's pstl in the repo now, but it's not production-ready yet, and won't be for the upcoming LLVM 15. So still a ways away unfortunately.

@h-vetinari h-vetinari marked this pull request as ready for review July 18, 2022 07:43
Comment thread doc/source/dev/toolchain.rst Outdated
rgommers added a commit to rgommers/scipy that referenced this pull request Jul 23, 2022
This follows up on scipygh-16589, where we agreed on a new minimum
GCC version.

I don't think we should implement checks for Clang or other compilers
yet, let's first see this is robust.
rgommers added a commit to rgommers/scipy that referenced this pull request Jul 23, 2022
This follows up on scipygh-16589, where we agreed on a new minimum
compiler versions.

I don't think we should implement checks for Clang or other compilers
yet, let's first see this is robust.
Copy link
Copy Markdown
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 now, in it goes. Thanks @h-vetinari!

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

Labels

C/C++ Items related to the internal C/C++ code base Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants