Skip to content

GitHub Actions CI: CPU build tests for GNU, Intel OneAPI, and NVHPC - #1524

Open
cenamiller wants to merge 1 commit into
MPAS-Dev:developfrom
NCAR:feature/ci-cpu-testing
Open

cenamiller wants to merge 1 commit into
MPAS-Dev:developfrom
NCAR:feature/ci-cpu-testing

Conversation

@cenamiller

@cenamiller cenamiller commented Oct 5, 2026 •

Copy link
Copy Markdown

Replaces #1469 (closed prematurely).

Adds initial GitHub Actions CI for MPAS-A: build tests for the
atmosphere and init_atmosphere cores with GNU, Intel OneAPI, and
NVHPC compilers and MPICH on CPU. This is a subset of the testing
developed on the NCAR/MPAS-Model-CI fork.

Each workflow:

  • Builds atmosphere, then init_atmosphere in the same source tree
    (skipped if atmosphere fails), in NCAR hpcdev containers on
    GitHub-hosted runners. Double precision and SMIOL are the defaults.
  • Runs on new PRs against develop (or when someone pushes a new
    commit to an open PR) and on push to master.
  • Prints compiler and library versions and the build command in the
    run summary, and uploads the build logs as an artifact
    (build-logs-<sha>-<compiler>-<mpi>-<io>-<precision>, kept 7 days).

More tests and build options are available on the NCAR/MPAS-Model-CI
fork and can follow in future PRs.

Files:

  • .github/ci-config.env: container images and make targets
  • .github/actions/build-mpas/: builds an MPAS core
    (print-build-env.sh prints versions and the build command)
  • .github/actions/resolve-container/: resolves the container image
  • .github/workflows/_build-compiler.yml: reusable build workflow
  • .github/workflows/build-{gcc,intel,nvhpc}-mpich.yml: 3 callers

Developed in NCAR/MPAS-Model-CI. Claude assistance.

@cenamiller

Copy link
Copy Markdown
Author

Depending on what science changes have been merged into develop at the same time, we shouldn't expect the ect to pass until I generate a new summary file. But let's see!

@mgduda

mgduda commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@cenamiller Thanks for this new PR!

@mgduda
mgduda requested review from abishekg7 and mgduda October 5, 2026 22:04
@mgduda mgduda added Atmosphere CI only Changes only affect CI, not the code or documentation labels Oct 5, 2026
@mgduda

mgduda commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

@cenamiller @abishekg7 Following up on the comments from PR #1469 (which this PR replaces), I agree with most of them, and I'm fine with addressing some of them (e.g., adding build tests for the init_atmosphere core, and adding a CMake build test) with a future PR.

Is it the case as of now that all of the tests will run with every push to a PR branch?

@abishekg7

Copy link
Copy Markdown
Collaborator

@cenamiller @abishekg7 Following up on the comments from PR #1469 (which this PR replaces), I agree with most of them, and I'm fine with addressing some of them (e.g., adding build tests for the init_atmosphere core, and adding a CMake build test) with a future PR.

Is it the case as of now that all of the tests will run with every push to a PR branch?

It is now a pared down set of tests, but yes, they will be triggered with every push to a PR branch.

@cenamiller

Copy link
Copy Markdown
Author

Hi, yes, agreed with Abishek. I've been thinking about this. It may be nice to have the "runs on every push to a PR" branch feature for a bit, because it makes it easier to trigger tests on old PRs, with just a push. Otherwise, contributors would need to close/reopen the PR in order to trigger tests. But once we've run them on some old PRs, we can very easily turn that trigger off. And anyway, if the tests make it to the default branch, you'll be able to manually dispatch tests when you think it's appropriate and we won't be so beholden to an automatic trigger.

@mgduda

mgduda commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@cenamiller @abishekg7 What do you all think about (1) removing the ECT tests from this PR, and (2) adding build tests for the init_atmosphere core to this PR? My thinking is that build tests for both the init_atmosphere and atmosphere cores are probably of greatest value right now, and omitting the ECT tests would help to reduce the amount of time all of the checks will typically take to complete.

@cenamiller

Copy link
Copy Markdown
Author

@mgduda Yes, we can do that! It won't take very long to set up, but I don't have those tests ready as stand-alones. I can submit them before lunch tomorrow though. Would you like them to run on the same triggers as these? And which compilers for each? The options for 'just build' would be GNU, Intel OneAPI, NVHPC (for CPU) and NVHPC+OpenACC. And for sides we have PIO/SMIOL and MPICH/OpenMPI.

@mgduda

mgduda commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

@mgduda Yes, we can do that! It won't take very long to set up, but I don't have those tests ready as stand-alones. I can submit them before lunch tomorrow though. Would you like them to run on the same triggers as these? And which compilers for each? The options for 'just build' would be GNU, Intel OneAPI, NVHPC (for CPU) and NVHPC+OpenACC. And for sides we have PIO/SMIOL and MPICH/OpenMPI.

In the interest of keeping everything as simple as possible for now, would it be reasonable to add an extra step to the build workflows to compile the init_atmosphere core before or after we compile the atmosphere core? When building a second core within an MPAS-Model source tree that already has another core (along with the shared infrastructure) compiled, the incremental cost of building the second core is not so bad. Going this route would suggest we use the same build matrix as for the atmosphere core: GNU+MPICH for Intel oneAPI, NVHPC, and GNU.

@abishekg7

Copy link
Copy Markdown
Collaborator

I think keeping the existing build matrix and triggers, and just adding the build of init_atmosphere core as a step sounds good. Perhaps init_atmosphere core can be built before the build of atmosphere?

@mgduda

mgduda commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

I think keeping the existing build matrix and triggers, and just adding the build of init_atmosphere core as a step sounds good. Perhaps init_atmosphere core can be built before the build of atmosphere?

Adding a build of the init_atmosphere core before the build of the atmosphere core sounds fine to me. (Perhaps it's over-optimizing, but my suspicion is that more code is changed in the atmosphere core than in the init_atmosphere core, so building the atmosphere core first may help us to catch build errors introduced by a PR a bit sooner. Ultimately, though, the time savings either way may not be significant enough to influence our decision on which core to build first.)

Build the atmosphere and init_atmosphere cores in NCAR hpcdev containers
on GitHub-hosted runners, atmosphere first, in one source tree. Each run
saves compiler and library versions, the build command, and build logs.

Runs on PRs against develop and on push to master.
@cenamiller
cenamiller force-pushed the feature/ci-cpu-testing branch from 2268943 to 96b9518 Compare October 16, 2026 15:21
@cenamiller cenamiller changed the title GitHub Actions CI: CPU compilers with ECT validation GitHub Actions CI: CPU build tests for GNU, Intel OneAPI, and NVHPC Oct 16, 2026
@cenamiller

Copy link
Copy Markdown
Author

Squashed to a single commit and rebased onto the current develop, as requested. No file changes.


# Set up I/O configuration
if [ "${{ inputs.use-pio }}" = "true" ]; then
export PIO_ROOT=${PIO_ROOT:-/container/pio}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be PIO, not PIO_ROOT. According to the MPAS Makefile

persist-credentials: false

# atmosphere first: it is the more likely to fail
- name: Build atmosphere (${{ inputs.precision }} precision, ${{ inputs.io }})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to do the init_atmosphere CORE step first and then atmosphere? I'm thinking about keeping the atmosphere executable to run ECTs later on, etc. Or are these executables cached anyway, and the make clean step doesn't matter as much?

echo "::error::${PRINT_ENV} not found"
exit 1
fi
export COMPILER_FAMILY MAKE_TARGET ARCH CLEAN_CMD MAKE_CMD

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Claude suggested this issue. I haven't double checked it myself.

make-flags gets split on spaces. ${MAKE_CMD} and ${CLEAN_CMD} run unquoted. A value like FFLAGS_OPT="-O2 -g" reaches make as two broken words, and any * or ? gets expanded as a filename pattern. Building the command as a bash array would fix it.

build-timeout:
description: 'Build timeout in minutes'
required: false
default: '20'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could this be bumped up a bit? As we're building two CORES now?

export USE_PIO2=true
else
unset PIO
export USE_PIO2=false

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

USE_PIO2 just needs to be unset I think

@abishekg7

Copy link
Copy Markdown
Collaborator

I'm adding a bunch of tasks as issues (to my fork) that can be addressed in the second iteration of the CI. Feel free to ignore them for now.

This branch has not been deployed

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

Labels

Atmosphere CI only Changes only affect CI, not the code or documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants