Skip to content

Move to manylinux_2_28 base image for wheels#3306

Merged
JCGoran merged 10 commits into
masterfrom
jelic/manylinux_2_28_migration
Jan 28, 2025
Merged

Move to manylinux_2_28 base image for wheels#3306
JCGoran merged 10 commits into
masterfrom
jelic/manylinux_2_28_migration

Conversation

@JCGoran

@JCGoran JCGoran commented Jan 10, 2025

Copy link
Copy Markdown
Collaborator
  • use new Docker image tags (manylinux_2_28_x86_64 and manylinux_2_28_aarch64) in CI for building wheels
  • use image-provided flex, mpich, and openmpi instead of building from source (still need custom readline and ncurses as the *-static versions do not use -fPIC)
  • unify MPI headers when building wheels
  • remove BBP-specific tests for wheels
  • show warning in Python wrapper if host machine is using GCC<10

@JCGoran JCGoran changed the title [WIP] Move to manylinux_2_28 base image [WIP] Move to manylinux_2_28 base image for wheels Jan 10, 2025
@pramodk

pramodk commented Jan 10, 2025

Copy link
Copy Markdown
Member

I didn’t check the details but just asking for clarity - moving to 2.28 changed anything about old platforms supported?

@JCGoran

JCGoran commented Jan 10, 2025

Copy link
Copy Markdown
Collaborator Author

I didn’t check the details but just asking for clarity - moving to 2.28 changed anything about old platforms supported?

This is mostly in reference to #1963 (comment) to assess how much work is needed.
The change would only have an impact on users who have older systems, but would like to run a newer version of NEURON (9 and above), and only via wheels (source/Spack builds would remain unaffected), i.e. personal machines, since cluster deployments should anyway use a Spack/source install (for performance reasons).

The new min. system requirements would be:

  • Debian 10+ (10 reached EOL in July 2024)
  • Ubuntu 18.10+ (the previous LTS version, 18.04.6, reached EOL in May 2023)
  • Fedora 29+ (reached EOL years ago)
  • CentOS/RHEL 8+ (CentOS 7 reached EOL July 2024)

@azure-pipelines

Copy link
Copy Markdown

✔️ 952d3d9 -> Azure artifacts URL

@codecov

codecov Bot commented Jan 10, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.07%. Comparing base (74dcdd5) to head (4ffe46c).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3306   +/-   ##
=======================================
  Coverage   67.07%   67.07%           
=======================================
  Files         571      571           
  Lines      111055   111055           
=======================================
  Hits        74488    74488           
  Misses      36567    36567           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pramodk

pramodk commented Jan 10, 2025

Copy link
Copy Markdown
Member

Thanks for checking. Those all looks good! 👍

@JCGoran

JCGoran commented Jan 13, 2025

Copy link
Copy Markdown
Collaborator Author

Okay, some additional findings about this:

  • GCC major versions are not backwards compatible, i.e. we cannot reliably compile and link mod files w/GCC vOlder if we compiled a manylinux wheel w/GCC vNewer (if we didn't have to compile and link anything on the host machine, but just use it, we'd most likely be fine)
  • the version of the compiler in the manylinux image is actually too new for us (GCC 14); fortunately, we can install GCC 10 from the repos and use that as the compiler toolchain
  • why GCC 10? Because it's available for installation (may not be default) on at least Ubuntu 20.04 LTS, Debian 11, and (trivially) Almalinux 8, which are all rather old, but best be safe than sorry
  • I've also changed the min. version of GCC checked by the _binwrapper.py script (from 9 to 10) because we used to build things with GCC 10 anyway, and due to the first point above, there is no guarantee that NEURON would compile and link mod files properly w/ GCC 9
  • with the above in mind, we should also be more explicit about the min. version of GCC needed on the user system in the docs

In summary, I think the move to manylinux_2_28 without too many changes is more than feasible.

@azure-pipelines

Copy link
Copy Markdown

✔️ 44ef470 -> Azure artifacts URL

@JCGoran JCGoran changed the title [WIP] Move to manylinux_2_28 base image for wheels Move to manylinux_2_28 base image for wheels Jan 13, 2025
@JCGoran JCGoran marked this pull request as ready for review January 13, 2025 18:53
@azure-pipelines

Copy link
Copy Markdown

✔️ 4ffe46c -> Azure artifacts URL

@sonarqubecloud

Copy link
Copy Markdown

Comment thread packaging/python/README.md
@azure-pipelines

Copy link
Copy Markdown

✔️ fae53da -> Azure artifacts URL

@nrnhines nrnhines left a comment

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.

Many details I'm not familiar with. But if it works...

@JCGoran JCGoran merged commit 942642a into master Jan 28, 2025
@JCGoran JCGoran deleted the jelic/manylinux_2_28_migration branch January 28, 2025 22:20
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.

Python wheels impose vague and undocumented restrictions on the use of C++ features

3 participants