Update mom5 SPR for new CMake build system

[dougiesquire]

This PR updates the mom5 SPR for the new CMake build system.

Currently I have completely dropped support for the access-esm1.5 version. We need to discuss how we want to handle this version since it is built from a different branch and is not built with CMake. UPDATE: no longer true. This SPR now supports two build systems:

Makefile build supporting:

• access-esm1.5
• access-om2
• legacy-access-om2-bgc

CMake build supporting:

• mom_solo
• mom_sis
• access-om2
• legacy-access-om2-bgc
• access-esm1.6

Build will default to CMake where possible

[dougiesquire]

@harshula, what would you like to do about the access-esm1.5 version? Should I create a separate SPR for it?

[harshula]

Hi @dougiesquire ,

Have a look at: https://github.com/spack/spack/blob/develop/var/spack/repos/spack_repo/builtin/packages/libxml2/package.py

Spack documents how to do this: https://spack.readthedocs.io/en/latest/packaging_guide.html#multiple-build-systems

[dougiesquire]

Testing

Uses:

• This PR @ 2a7949b
• CMake build system overhaul MOM5#39
• Changes to allow using gtracers in another CMake project GFDL-generic-tracers#28
• Update libaccessom2.pc.in libaccessom2#15
• access-fms: set variant gfs_phys default to False #237
• Add MOM5 REPRO flags to preserve answers with MOM5 internal FMS FMS#12

Summary of results:

Model	Prerelease deployment PR	Configuration test PR	Historical reproducibility?	Expected outcome?
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#106	dev-1deg_jra55_ryf dougiesquire/1deg_jra55_ryf_wombatlite	✅ see here ❌ see here	✅ ✅ see here
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#24	dev-1deg_jra55_ryf_bgc	✅ see here	✅
ACCESS-ESM1.6	ACCESS-NRI/ACCESS-ESM1.6#86	dev-preindustrial+concentrations	❌ see here	✅ see here

[harshula]

Hi @dougiesquire , Thanks for implementing the class Mom5(CMakePackage, MakefilePackage) approach. Please keep support for access-om2, access-om2-bgc and access-esm1.5 in the MakefilePackage section because we released them with the mkmf build system. I would also keep access-esm1.6 support in the MakefilePackage section for easy comparison runs, if we need to do any debugging. However, your call on access-esm1.6.

[dougiesquire]

Testing

Uses:

• This PR @ 1dcfbf4
• CMake build system overhaul MOM5#39
• Changes to allow using gtracers in another CMake project GFDL-generic-tracers#28
• Update libaccessom2.pc.in libaccessom2#15
• access-fms: set variant gfs_phys default to False #237
• Add MOM5 REPRO flags to preserve answers with MOM5 internal FMS FMS#12

Summary of results:

Model	Prerelease deployment PR	Configuration test PR	Historical reproducibility?	Expected outcome?
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#106	dev-1deg_jra55_ryf dougiesquire/1deg_jra55_ryf_wombatlite	✅ see here ❌ see here	✅ ✅ see here
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#24	dev-1deg_jra55_ryf_bgc	✅ see here	✅
ACCESS-ESM1.6	ACCESS-NRI/ACCESS-ESM1.6#86	dev-preindustrial+concentrations	❌ see here	✅ see here
ACCESS-ESM1.5	ACCESS-NRI/ACCESS-ESM1.5#37	dev-preindustrial+concentrations	✅ see here	✅

Also:

$ spack install mom5@51f67bf4a554d62f7b1ce99caf2f39dc1ef6b7b3=mom_solo

and

$ spack install mom5@51f67bf4a554d62f7b1ce99caf2f39dc1ef6b7b3=mom_sis

both build successfully.

[dougiesquire]

Please keep support for access-om2, access-om2-bgc and access-esm1.5 in the MakefilePackage section because we released them with the mkmf build system. I would also keep access-esm1.6 support in the MakefilePackage section for easy comparison runs, if we need to do any debugging.

@harshula really? Can we not just use an earlier tag of spack-packages if we ever need to do this in the future (though I don't see why we'd need mkmf again for these types given the CMake system gives the same answers - see above)? I thought the whole point in me doing all this work was to remove the burden of having to support mkmf?

[harshula]

Hi @dougiesquire , As stated during the meeting, the point of adding CMake support to MOM5 was for long-term ACCESS-ESM1.6 support. We can't rely on using an older version of spack-packages because we may need to make changes to SPRs. e.g. When Spack v1.0 releases, we anticipate having to make Spack related changes to our SPRs.

[dougiesquire]

Why do we need mkmf support when I can reproduce the released models with the CMake build system?

[harshula]

Hi @dougiesquire , Can you reproduce the binaries?

[dougiesquire]

Hi @dougiesquire , Can you reproduce the binaries?

I'm guessing no because they were released with ~deterministic. But neither could the mkmf build, right? Or have I misunderstood the question?

[harshula]

Hi @dougiesquire , +deterministic removes attributes like timestamps to make it easier to compare binaries. However, that doesn't mean the binaries are identical. IIRC, for MOM5, I don't think I was able to get the COSIMA and ACCESS-NRI binaries to be identical.

If you don't have time to revert back the support for Makefile builds of released models, just rename your MOM5 SPR (e.g. Mom5Cmake) and continue work. We can then integrate your CMake changes back to the MOM5 SPR without breaking the existing Makefile support.

[dougiesquire]

There are a number of nuances to what you're asking. Can we chat about this?

[harshula]

Summary: We should have a workable build_system() call with the appropriate conditional() calls.

[dougiesquire]

Notes from chat:

• Makefile build will support:
  • access-esm1.5
  • access-om2
  • legacy-access-om2-bgc
• CMake build will support:
  • mom_solo
  • mom_sis
  • access-om2
  • legacy-access-om2-bgc
  • access-esm1.6
• Build will default to CMake where possible
• The old implementation of external gtracer/fms support will be dropped from Makefile builds (this was never released)

[dougiesquire]

Testing

Uses:

• This PR @ e7c653b
• CMake build system overhaul MOM5#39
• Add CMake build system and Pkgconf pc file GFDL-generic-tracers#17
• Allow libaccessom2 to be used in other CMake projects using find_package libaccessom2#17
• access-fms: set variant gfs_phys default to False #237
• Add MOM5 REPRO flags to preserve answers with MOM5 internal FMS FMS#12

CMake build system:

Model	Prerelease deployment PR	Configuration test PR	Historical reproducibility?	Expected outcome?
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#106	dev-1deg_jra55_ryf dougiesquire/1deg_jra55_ryf_wombatlite	✅ see here ❌ see here	✅ ✅ see here
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#24	dev-1deg_jra55_ryf_bgc	✅ see here	✅
ACCESS-ESM1.6	ACCESS-NRI/ACCESS-ESM1.6#86	dev-preindustrial+concentrations	❌ see here	✅ see here

Makefile build system:

Model	Prerelease deployment PR	Configuration test PR	Historical reproducibility?	Expected outcome?
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#106	dev-1deg_jra55_ryf	✅ see here	✅
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#24	dev-1deg_jra55_ryf_bgc	✅ see here	✅
ACCESS-ESM1.5	ACCESS-NRI/ACCESS-ESM1.5#37	dev-preindustrial+concentrations	✅ see here	✅

Also:

$ spack install [email protected]=mom_solo

and

$ spack install [email protected]=mom_sis

both build successfully with CMake.

[dougiesquire]

This is ready for review @harshula

[harshula]

Hi @dougiesquire , The changes are progressing well! Can you please put back the comments related to the setup() function and revert back to __builds variable name for the data structure because we want to use this SPR as a template that can be copied by others. Also, put back the copyright comment and undo unnecessary whitespace changes that make the diff even larger. In general, when making such a significant and important functional change, please don't make unnecessary changes.

[dougiesquire]

Can you please put back the comments related to the setup() function and revert back to __builds variable name for the data structure because we want to use this SPR as a template that can be copied by others

I changed the name to __types because this used to be a nested dictionary with values describing the builds, but it's now a single-level dictionary with values containing the types. But sure.

undo unnecessary whitespace changes that make the diff even larger.

Apologies, this must have been my editor

[dougiesquire]

Done in 1517db9

[access-hive-bot]

This pull request has been mentioned on ACCESS Hive Community Forum. There might be relevant details there:

https://forum.access-hive.org.au/t/coordinating-development-threads-for-next-phase-of-control-run/4523/1

[harshula]

Looks good! Are you intentionally removing restart_repro variant for CMake builds?

[dougiesquire]

Are you intentionally removing restart_repro variant for CMake builds?

Yes that's intentional. The CMake build always builds with the REPRO flags. It's mentioned here but should have also mentioned in this PR sorry.

[dougiesquire]

@harshula once you're happy I'll do one final set of tests with the most recent version before merging

[harshula]

Thanks for making these changes! Please remember to squash merge.

[dougiesquire]

Testing

Uses:

• This PR @ ed25bd4
• CMake build system overhaul MOM5#39
• Add CMake build system and Pkgconf pc file GFDL-generic-tracers#17
• Allow libaccessom2 to be used in other CMake projects using find_package libaccessom2#17
• access-fms: set variant gfs_phys default to False #237
• Add MOM5 REPRO flags to preserve answers with MOM5 internal FMS FMS#12

CMake build system:

Model	Prerelease deployment PR	Configuration test PR	Historical reproducibility?	Expected outcome?
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#106	dev-1deg_jra55_ryf dougiesquire/1deg_jra55_ryf_wombatlite	✅ see here ❌ see here	✅ ✅ see here
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#24	dev-1deg_jra55_ryf_bgc	✅ see here	✅
ACCESS-ESM1.6	ACCESS-NRI/ACCESS-ESM1.6#86	dev-preindustrial+concentrations	❌ see here	✅ see here

Makefile build system:

Model	Prerelease deployment PR	Configuration test PR	Historical reproducibility?	Expected outcome?
ACCESS-OM2	ACCESS-NRI/ACCESS-OM2#106	dev-1deg_jra55_ryf	✅ see here	✅
ACCESS-OM2-BGC (legacy)	ACCESS-NRI/ACCESS-OM2-BGC#24	dev-1deg_jra55_ryf_bgc	✅ see here	✅
ACCESS-ESM1.5	ACCESS-NRI/ACCESS-ESM1.5#37	dev-preindustrial+concentrations	✅ see here	✅

Also:

$ spack install [email protected]==mom_solo

and

$ spack install [email protected]==mom_sis

both build successfully with CMake.

[harshula]

Hi @dougiesquire , You should have cleaned up the commit message.