Skip to content

Portable MOM6-examples testing #515

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Mar 20, 2025
Merged

Portable MOM6-examples testing #515

merged 12 commits into from
Mar 20, 2025

Conversation

@marshallward
Copy link
Member

This PR modifies the ocean_only Makefile to support automated running and testing of the MOM6-examples test suite.

Currently it only replicates the existing Gitlab testing, which validates the input parameter (MOM_parameter_doc.*) and diagnostics (available_diags*) output. But the testing the

Two support scripts have been added:

  • parse_mom_input.py: Command line tool that returns the value of a MOM_input parameter. It can also be used to convert the parameter file to a Python dict.
  • get_input_manifest.py: Generate a list of input files used by

The f90nml Python module has also been added as a git submodule for get_input_manifest.py.

This was set up for "normal" systems, so it does not work out-of-the-box on Gaea. Additional MPI configuration is required.

@marshallward marshallward force-pushed the update_m6e_test branch 2 times, most recently from de19e5c to 9160a2f Compare March 6, 2025 18:58
@marshallward
Copy link
Member Author

I extended this to the ice_ocean_SIS2 tests, and also fixed a few issues exposed by those tests. Let me try to extend this to the coupled AM2 tests before reviewing.

@aekiss aekiss mentioned this pull request Mar 6, 2025
Open
@marshallward
Copy link
Member Author

marshallward commented Mar 7, 2025 •
edited
Loading

I added some more:

  • testing rules for the AM2-coupled builds

  • make test rule in the top Makefile

  • A few out-of-date tests were added to EXCLUDE so that the tests would pass:

    • idealized_hurricane

    • global

    • AM2_SIS2B_MOM6i_1deg

    But they could be included if we wanted to keep those params up to date.

I think this is ready now, although I always end up finding surprises.

@adcroft
Copy link
Member

adcroft commented Mar 10, 2025 •
edited
Loading

I'll try this out shortly, but I notice we have a test.all target in both the top and ocean_only Makefiles, but we have no run.all even though we do have run.<EXPT> in ocean_only. I think we would need run.all when the doc files are not the output being "tested" from other pipelines.

@marshallward
Copy link
Member Author

Each of the submakes has a run.all but I can add a run (or run.all) to the top Makefile which parallels its test rule.

@marshallward
Copy link
Member Author

I added a top-level run rule which does run.all in each model. (I could rename run and test to run.all and test.all, might be more clear?)

TESTDIR is currently set to an external directory, scratch/. Do we want to change this to in-place testing (TESTING = .) ? It would make it easier to do update the parameters with git. If so, maybe TESTDIR should be set to in-place for each model as well.

@marshallward
Copy link
Member Author

marshallward commented Mar 11, 2025 •
edited
Loading

There are issues with the BUILD macro incorrectly applied to the dependencies, e.g.

  1. BUILD=build2
  2. FMS builds to shared/fms/build2
  3. Parallel submake into ocean_only and ice_ocean_SIS2, but they use FMS_BUILD=../shared/build (i.e. the original path) and then one fails as they both try to build FMS in parallel. And they don't even use build2!

There are a few fixes, but one possibility is to define top-level FMS_BUILD, ATMOS_BUILD, etc macros to satisfy everything.

(Also remember AM2-coupled needs FMS1, so both FMS libs should have been listed as dependencies. This was a flat-out error in the top Makefile.)

Other minor stuff:

  • Old attempts at run rules in MOM6-Examples Makefile should probably be removed!
  • Explore a common "API" in the top-level and submakes.
  • ocean_only has a big doc header but the others are empty. Huh??

@marshallward
Copy link
Member Author

The top Makefile was updated to support a BUILD argument. If unset, it respects the defaults of the submakes. If set, it places each component in $(BUILD)/fms2, $(BUILD)/ocean_only, etc. It also repairs the FMS1/FMS2 issues that were not being handled in the Makefile.

I did this by adding export ... decorators to each rule. Could be crazy... but it worked. Should be safe as long as these are explicitly defined in each subsequent rule. But I could see it growing into a bit of a mess in the clean rules.

But overall this appears to work now, and no changes to the submakes was needed.

@marshallward
Copy link
Member Author

New updates to run and test rules, to use the new dependency decorators.

Also pretty colors to the test diff output.

@marshallward
Copy link
Member Author

Yet another issue: This works if BUILD is set with environment variables (BUILD=foo make) but not as a command-line macro value (make BUILD=foo).

Seems like rules (and the funny export conditions) are built before command-line macros, but environment variables are available when rules are built. Oops.

Taking one more crack at the problem...

@marshallward
Copy link
Member Author

New version creates BUILDFLAG = BUILD=$(BUILD)/model macros which are used in the rules. A bit more bloaty but it appears to work. (Testing would be very welcome though.)

@marshallward marshallward force-pushed the update_m6e_test branch 2 times, most recently from 82e7b95 to d9bcd82 Compare March 18, 2025 20:05
@marshallward
parse_mom_input.py: MOM parameter doc file parser
ba3495b 
This script parses a MOM parameter file and can be used as either a
Python module or a command line tool.

If used as a module, it returns a dict of all Python parameters.  AFAIK
nearly everything is supported, including blocks and repeat tokens (e.g.
x = 5*1).  Only A%B%C = 1 is not yet supported.

As a command line tool, you can provide the file and a parameter and it
will return the value.  Probably won't work for blocks.  I don't think
there are any plans to use it as a CLI tool.

The f90nml Python module was added as a submodule to support this and
future support scripts.
@marshallward
get_nprocs.py: Replacement CPU parser
958472a 
The get_nprocs.sh script has been replaced with get_nprocs.py, a Python
script which parses both MOM_parameter_doc.layout and input.nml to
determine the total number of CPUs for a given experiment.

The "maxiumum CPU limit" is now determined by the psutils Python module.
It will only be used if the module is available.  There is no true
platform-independent or even POSIX way to determine the total CPU count,
but psutil provides a way to do this on a variety of platforms - if it's
available.
@marshallward
gen_input_manifest.py: Experiment input file list
9f64735 
This script is a command line too which attempts to generate the list of
input files for a given experiment.

Information is taken from both input.nml and MOM parameter files to
reconstruct the list of input files, along with any expected FMS files.
Any information in INPUT (or perhaps the param's INPUTDIR) is assumed to
be a symbolic link and is moved over by the associated Makefile.

This script is far from perfect, and it is perhaps impossible to ever
create such a script, but it should be sufficient for this repo.
@marshallward
ocean_only: Makefile rules to run and test params
8b9d855 
This adds multiple rules to the ocean_only Makefile for running
experiments and testing parameter output.  This reflects existing
testing of MOM6-examples in the

See the Makefile header for a breakdown of the rules.  The basic rules
are

    make
        Build the model
    make run EXPT=exp RUNDIR=scratch
    make exp.run
        Run experiment `exp` in directory `scratch`.
        If RUNDIR is omitted, run in-place.
    make test EXPT=exp
        Test the parameter output of `exp` against reference output.
    make run.all
    make test.all
        Run all tests (with some exception)
@marshallward
ice_ocean_SIS2: New rules to run and test model
49ac370 
Same as ocean_only, see prior commit.
@marshallward
coupled_AM2_LM3_SIS2: Rules to run and test model
2e507aa 
Same as ocean_only, see prior commit.
@marshallward
Top-level test rule in MOM6-examples Makefile
d958c24 
This patch adds a `test` rule to the top Makefile, which runs `test.all`
for each of the submodules.

The build rules for each model are added as dependencies for the test
rule, so that there is no attempt to concurrently build the identical
components in separate submake processes.
@marshallward
Update Gitlab config to new Makefile testing
b3f86f3 
The Gitlab configuration file is updated to use the new Makefile-based
testing.  Multiple tasks which were previously defined in
tools/MRS/Makefile are moved into .gitlab-ci.yml.

While still not a platform-independent setup, there is at least now a
logical separation of the environment from the building and running of
the models.
@marshallward
Explicitly run scripts through Python
34cfe68 
Replace Python script shell invocation and pass the scripts directly to
the Python parser.  This prevents reliance on the /usr/bin/env syntax.
(For example, `python3` is the only available parser in some systems.)

The PYTHON environment variable is also used to permit the environment
to define the Python parser.
adcroft added 3 commits March 20, 2025 15:06
@adcroft @marshallward
Removed "complete clean" after incomplete builds
42d0ae2 
An incomplete build (e.g. interrupted, or due to source code error) would
lead to a rm *.o before re-building. Apparently I said this was ok, but
I don't agree with my past self.
@adcroft @marshallward
Replaced FORCE with wildcard to avoid unnecessary invokation of sub-make
f428779 
The use of a FORCE target was causing invocation of sub-make 4 levels deep
all the way through to the build/fms Makefile which was invoked for every
other sub-make target. This replaces FORCE with appropriate wildcard based
on CODEBASE for each target. It works fine. While the wildcard might
capture source code that is not actually compiled, it does no harm making
it a dependency in the higher level Makefiles.
@adcroft @marshallward
Output from make test.all was hidden
e363928 
- Changed `make -j test.all` to `make -k test.all` to get all changes
- Changed dependency for test.% to run.% to help with one-stop-shop
  - The test otherwise depended on just the executable
@marshallward
Copy link
Member Author

Improvements from @adcroft

  • Some problematic dependency rules have been replaced.
  • FORCE rules have been removed and replaced with wildcard dependencies to dependency source
  • Gitlab testing now (correctly) prints diff output to CI log.

@adcroft adcroft merged commit b10a27a into NOAA-GFDL:dev/gfdl Mar 20, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marshallward @adcroft