Skip to content

CMake build system overhaul#39

Merged
dougiesquire merged 5 commits intomasterfrom
update_cmake
May 23, 2025
Merged

CMake build system overhaul#39
dougiesquire merged 5 commits intomasterfrom
update_cmake

Conversation

@dougiesquire
Copy link
Collaborator

@dougiesquire dougiesquire commented May 8, 2025

This PR overhauls the existing CMake build system to allow it to be used to build ACCESS models.

Closes #38

Changes

  • Added support for ACCESS build types: ACCESS-OM, ACCESS-OM-BGC, ACCESS-ESM, ACCESS-CM. These use external FMS and generic tracers.
  • Removed support for unneeded build types: CM2M, EBM, ESM2M, ICCM
  • Retained support for potentially-useful build types: MOM_solo, MOM_SIS. These use internal FMS and generic tracers.
  • The CMake build does not include the option to set the REPRO build flags. These are now always set. These were set by default in the mom5 SPR.
  • The CMake build does not include the option to set -DCOSIMA_VERSION. It is now always set. UPDATE: It is now always unset.
  • The CMake build does not include the option to set -DUNIT_TESTING. It is now always unset. We can add the option later if we find it is needed.
  • The CMake build does not include the option to generate the optimization report.

A few other small changes are included in this PR to ensure that:

  • The old mkmf build still works (fca88c2), without support for building ACCESS models with generic tracer WOMBAT (4e76671)
  • The legacy ACCESS-OM-BGC build type can still be used (214fe0c)
  • Additional compiler flags added by @harshula to the MOM5 spack package are included in bin/mkmf.template.ubuntu (a4b3ae3)

Dependencies (ticked if merged)

This PR requires the following changes:

Corresponding changes to the mom5 Spack package are here:

Questions

  • I haven't included these FPPFLAGS. Should I? I can reproduce old answers without them.
  • Previously, we built mppnccombine as part of the MOM5 build using outdated code included in the MOM5 repo. Do we want to continue to support this, or should we deploy mppnccombine through FRE-NCtools.

@dougiesquire dougiesquire self-assigned this May 8, 2025
@dougiesquire dougiesquire force-pushed the update_cmake branch 2 times, most recently from c1ab829 to 1fdf6ff Compare May 12, 2025 01:25
dougiesquire added a commit to ACCESS-NRI/ACCESS-OM2 that referenced this pull request May 12, 2025
dougiesquire added a commit to ACCESS-NRI/ACCESS-OM2 that referenced this pull request May 12, 2025
dougiesquire added a commit to ACCESS-NRI/ACCESS-OM2 that referenced this pull request May 12, 2025
dougiesquire added a commit to ACCESS-NRI/ACCESS-OM2-BGC that referenced this pull request May 12, 2025
dougiesquire added a commit to ACCESS-NRI/ACCESS-OM2-BGC that referenced this pull request May 12, 2025
dougiesquire added a commit to ACCESS-NRI/ACCESS-ESM1.6 that referenced this pull request May 12, 2025
dougiesquire added a commit to ACCESS-NRI/ACCESS-ESM1.6 that referenced this pull request May 12, 2025
@dougiesquire
Copy link
Collaborator Author

Testing is documented here: ACCESS-NRI/access-spack-packages#238 (comment)

This is now ready for review @harshula, @micaeljtoliveira

@dougiesquire dougiesquire marked this pull request as ready for review May 13, 2025 01:44
@dougiesquire
Copy link
Collaborator Author

dougiesquire commented May 13, 2025

This is now ready for review @harshula, @micaeljtoliveira

Actually, probably best to hold off a little sorry. Requirements have evolved slightly.

@dougiesquire
Copy link
Collaborator Author

Okay, this is ready for review @harshula, @micaeljtoliveira

I've tested the most recent changes pretty extensively using Spack - results are reported here

@aidanheerdegen
Copy link
Member

aidanheerdegen commented May 16, 2025

  • Additional compiler flags added by @harshula to the MOM5 spack package are included in bin/mkmf.template.ubuntu (a4b3ae3)

I assume this was to get the mkmf build equivalent to the CMake build?

Doesn't worry me. I assume if one of the optimisation team want to do this they can pass the appropriate flags through from the spack.yaml?

  • Do we want to continue to support this, or should we deploy mppnccombine through FRE-NCtools.

Deploy through FRE-NCtools is better.

Edit: very comprehensive PR, exceptionally well documented. Thanks. Made it a lot easier to understand.

@dougiesquire
Copy link
Collaborator Author

I assume this was to get the mkmf build equivalent to the CMake build?

It's actually to make sure that the mkmf build works with new versions of the GNU compilers without using Spack (I have no idea why anyone would want this, but no harm I suppose)

Deploy through FRE-NCtools is better.

Ta. We'll need to do a proper deployment of FRE-NCtools then (we currently only have a prerelease deployed through the ACCESS-OM3 deployment repo).

@harshula
Copy link

I assume this was to get the mkmf build equivalent to the CMake build?

It's actually to make sure that the mkmf build works with new versions of the GNU compilers without using Spack (I have no idea why anyone would want this, but no harm I suppose)

No, that was not the reason. It was because the MOM5 SPR had # Copied from bin/mkmf.template.ubuntu and @dougiesquire noticed that we had appended -fallow-argument-mismatch -fallow-invalid-boz to FFLAGS in the MOM5 SPR. Hopefully, FFLAGS in both locations are consistent.

@harshula harshula requested review from aidanheerdegen and removed request for harshula May 19, 2025 05:02
@harshula
Copy link

I've added @aidanheerdegen as a reviewer and removed myself.

@dougiesquire
Copy link
Collaborator Author

we had appended -fallow-argument-mismatch -fallow-invalid-boz

I thought these were added in the SPR to allow compilation with newer version of GNU compilers?

Copy link
Member

@micaeljtoliveira micaeljtoliveira left a comment

Choose a reason for hiding this comment

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

Just a couple of questions, but nothing blocking.

Copy link
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

LGTM

@aidanheerdegen
Copy link
Member

Build failing though .. it is one of those pesky spack version things. Why would it fail now though? Is it worth just re-running in case it is an intermittent problem?

@dougiesquire
Copy link
Collaborator Author

dougiesquire commented May 22, 2025

I think this is because the CI uses the latest tag of spack-packages and there are changes in this PR that are incompatible with that version of spack-packages. We could merge/tag ACCESS-NRI/access-spack-packages#238 first to be sure?

UPDATE: actually I'm not sure I've thought that through. Let me look deeper...

@dougiesquire
Copy link
Collaborator Author

dougiesquire commented May 22, 2025

What I said above is wrong. I'm guessing the build failure is because the CI is trying to install mom5-git.update_cmake without setting the spack version? This is concretizing to mom5-git.update_cmake=2025.03.001-git.8 which isn't a valid spack version in the SPR (it needs to be one of access-om2, legacy-access-om2-bgc, access-esm1.5, access-esm1.6).

I can recreate by trying to install:

spack -d install mom5@git.update_cmake%intel@2021.10.0

which fails with the same error. However:

spack -d install mom5@git.update_cmake=access-om2%intel@2021.10.0

succeeds.

@harshula, @CodeGat

@dougiesquire
Copy link
Collaborator Author

dougiesquire commented May 22, 2025

I think the CI used to work because the depends on logic in the mom5 SPR used to be:

    with when("@:access-esm0,access-esm2:"):
        depends_on("netcdf-c@4.7.4:")
        depends_on("netcdf-fortran@4.5.2:")
        # Depend on virtual package "mpi".
        depends_on("mpi")
        depends_on("datetime-fortran")
        depends_on("oasis3-mct+deterministic", when="+deterministic")
        depends_on("oasis3-mct~deterministic", when="~deterministic")
        depends_on("libaccessom2+deterministic", when="+deterministic")
        depends_on("libaccessom2~deterministic", when="~deterministic")
    with when("@access-esm1.5:access-esm1.6"):
        depends_on("netcdf-c@4.7.1:")
        depends_on("netcdf-fortran@4.5.1:")
        # Depend on "openmpi".
        depends_on("openmpi")
        depends_on("oasis3-mct")

and so a numerical Spack version would satisfy the first with clause (i.e. build like access-om2).

However, with this recent change we moved to only allowing versions access-om2, legacy-access-om2-bgc, access-esm1.5 or access-esm1.6

@harshula
Copy link

Hi @dougiesquire , Ideally, CI should build a set of 'versions' that we consider important.

@dougiesquire
Copy link
Collaborator Author

@CodeGat,

If yes to both, @aidanheerdegen are you okay for me to merge with the failing check?

I've tested these changes pretty extensively as part of the related mom5 SPR update - see here

@aidanheerdegen
Copy link
Member

I don't know enough about why or why not, so I trust it's ok and am happy for you to merge.

@CodeGat
Copy link
Member

CodeGat commented May 23, 2025

Hi @dougiesquire, yes to both. I'd merge it - this shouldn't be a problem in the second iteration of build-ci.

…e.ubuntu

These were added by Harshula Jayasuriya in the MOM5 spack package but not propagated back
…c tracer WOMBAT

This is supported properly through the CMake build system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overhaul CMake build system

5 participants