-
Notifications
You must be signed in to change notification settings - Fork 200
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: Ubuntu 22.04/24.04 in GitHub Actions, CUDA 11.7+ #5731
base: development
Are you sure you want to change the base?
Conversation
I see this error message from the
Is there a specific reason why we want to keep testing builds with ICC, even though it is deprecated since 2023? |
Do you have insight on my question above, #5731 (comment)? I don't see ICC being used in AMReX CI workflows, if I'm not mistaken. If this is indeed obsolete, we can migrate this last ICC build in WarpX to ICX (DPC++) as well, or remove it if redundant. |
Examples/Tests/open_bc_poisson_solver/inputs_test_3d_open_bc_poisson_solver
Outdated
Show resolved
Hide resolved
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.
Looks good to me! Thanks!
.github/workflows/ubuntu.yml
Outdated
@@ -101,7 +101,7 @@ jobs: | |||
|
|||
build_3D_sp: | |||
name: GCC 3D & RZ w/ MPI, single precision | |||
runs-on: ubuntu-22.04 |
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 admittedly had intentionally a few 22.04 runners because it will cover older g++ and clang++ compilers that way.
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.
Please revert this one.
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.
Done in dac25b8.
.github/workflows/intel.yml
Outdated
@@ -18,7 +18,7 @@ jobs: | |||
# intel-basekit intel-hpckit are too large in size | |||
build_icc: | |||
name: oneAPI ICC SP&DP | |||
runs-on: ubuntu-20.04 | |||
runs-on: ubuntu-24.04 |
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.
Please remove the "old" ICC, it is EOL since mid 2023 and I already removed it from pyAMReX.
AMReX-Codes/pyamrex#409
Intel only supports ICX (oneAPI compilers) going forward. (The two we have below in this script.)
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.
Done in 43e0a32.
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.
Now that the oneAPI ICC SP&DP
workflow job has been removed, it should aslo be removed from the list of "required" workflows in the repository settings by one of the owners.
.github/workflows/post-pr.yml
Outdated
@@ -10,7 +10,7 @@ on: | |||
|
|||
jobs: | |||
noop: | |||
runs-on: ubuntu-latest |
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 would keep -latest
for this, because it will never need an update that way.
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.
Please revert this one.
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.
Done in 49a2603.
.github/workflows/cuda.yml
Outdated
name: NVCC 11.3 SP | ||
runs-on: ubuntu-20.04 | ||
name: NVCC SP | ||
runs-on: ubuntu-24.04 |
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 would not go over 22.04, because that way we do not cover older CUDA compilers anymore and will likely loose compatibility.
Instead, please do something like this BLAST-ImpactX/impactx#869 and update our documentation that we only support CUDA 11.7+ now.
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.
Okay, done in dd2fe9b.
The documentation does not seem to need any update, it already states 11.7+, if I'm not mistaken.
I initially chose to go for 12.6+ (and Ubuntu 24.04) because I saw that this is what they did in AMReX-Codes/amrex#4330 in AMReX.
In general isn't it a risk to mix Ubuntu 22.04 and Ubuntu 24.04 support in our CI coverage, i.e., don't we risk to have potentially holes in how we test compatibility? Shoudn't we then simply cover only Ubuntu 22.04 until it reaches EOL? I'm just trying to understand which of the two strategies, the mixed one we are setting up here or the single-version one they set up in AMReX, makes more sense, if any.
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.
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.
Please see inline comments and needed documentation changes for CUDA supported versions.
The Ubuntu 20.04 runner image will be deprecated soon: actions/runner-images#11101.
Only support CUDA 11.7+ going forward.
To-do:
Azure DevOps workflows(transfered to [WIP]Update constants to use values from CODATA2022 #5661)Notes
I renamed the following workflows, with the aim of not having to rename them again in the future when we upgrade version numbers:
NVCC 11.3 SP
, nowNVCC SP
NVCC 11.8.0 GNUmake
, nowNVC GNU Make
NVHPC@24.1 NVCC/NVC++ Release [tests]
, nowNVHPC
If you are okay with the renaming, the list of "required" workflows should be updated to reflect the new names, and I don't have the necessary access to do so.