Add non-repro, simple CI checks all relevant config PRs

[CodeGat]

References this comment: #23 (comment)

We don't need to go through the rigamarole of testing reproducibility for all PRs (only test that for dev-* into release-*) but we should still do some checks. See the referenced comment for the types of checks we want to do. These would run on all config-related PRs (<some feature> into dev-*, and dev-* into release-*).

[aidanheerdegen]

Some checks

• check project is not defined
• check shortpath is not defined
• absolute input paths
• all input paths in vk83 (allow exceptions for jra55 data in IAF configs)
• all input paths point to files (allow exceptions for IAF forcing data)
• exe paths absolute. test for consistency of spack hash?
• mppnccombine-fast collate exe for high res configs
• test no storage flags in qsub flags
• check no scripts in top level directory (should be in tools)
• test runlog is on
• restart_freq is date based. Maybe define some acceptable values?
• sync not enabled, sync path not set
• metadata.yaml json schema validation
• metadata.yaml fields present and correct (description, notes, keywords, nominal resolution, version, reference, license, url, model, realm = ['ocean', 'seaIce'] or realm = ['ocean', 'seaIce', 'ocnBgchem']?)
• manifests updated and consistent with executables and inputs

Many of these are available in an existing CI checking "library"

https://github.com/COSIMA/cleanconfig/blob/master/test.py

[aidanheerdegen]

Could use pytest markers to mark tests to assist in running only when required. e.g. highres marker could be used to switch on/off the mppnccombine-fast check

https://pytest-with-eric.com/pytest-best-practices/pytest-markers/

[aidanheerdegen]

@dougiesquire do you have any comments about the checks suggested above? Particularly the metadata.yaml fields? Should access-om2-bgc have an ocean and ocnBgchem realm?

And off topic, but hooo boy, there should be a special place in controlled vocabulary hell for the committee that thought ocnBgchem was better than oceanbgc or oceanbiogeochem or pretty much anything. Eeeuuwww.

[dougiesquire]

Should access-om2-bgc have an ocean and ocnBgchem realm?

Seems sensible to me. FYI, this choice doesn't impact the catalog, which gets the realm per file. Currently there's no easy way to determine whether an ACCESS-OM2 MOM output file is ocean or ocnBgchem so everything is classified as ocean.

[aidanheerdegen]

Thanks.

I should have been more specific (as usual), I wasn't sure if ocnBgchem (feels awful just typing it) implied ocean.

If we assume not, that both are required, we'll have the situation that the experiment specifies both realms, but none of the "file assets" (data files) have the ocnBgchem realm. I think that's fine, but we'll probably be first up against the wall if we stumble into an angry mob of ontologists (is there any other kind?).

[dougiesquire]

I wasn't sure if ocnBgchem (feels awful just typing it) implied ocean

I see - I don't think we should only include ocnBgchem. I think what's in the realm property of the metadata.yaml file should be the union of the realms for all the files in the experiment. access-om2-bgc has seaIce (CICE output) and ocean (most of MOM output) and ocnBgchem (MOM WOMBAT output).

we'll have the situation that the experiment specifies both realms, but none of the "file assets" (data files) have the ocnBgchem realm.

I can't see this being a huge problem anytime soon. The only issue I can think of off the top of my head is if you are doing/planning something like writing code snippets to load data from the catalog based on the metadata.yaml file. Then its probably safest to just have realm = ['ocean', 'seaIce'].

[CodeGat]

Could use pytest markers to mark tests to assist in running only when required. e.g. highres marker could be used to switch on/off the mppnccombine-fast check

https://pytest-with-eric.com/pytest-best-practices/pytest-markers/

@aidanheerdegen if we want to run different checks on different branches, we could add another .json file to the config folder on main that informs the CI (I seem to love those) regarding what markers to use. For example:

{
  "markers": {
    "branches": {
      "release-1deg_jra55_ryf": "highres and config and metadata",
      "release-some_other_config": "config and metadata"
    }
  }
}

What do you think? (thanks for bringing it to my attention @jo-basevi

[aidanheerdegen]

if we want to run different checks on different branches, we could add another .json file to the config folder on main that informs the CI (I seem to love those) regarding what markers to use

We could .. but equally we could also just interrogate the branch name

#32 (comment)

If the latter makes the CI too access-om2 specific, if otherwise it could be used more generically, then I'd be ok with a config file.

[CodeGat]

We could interrogate the branch, and do it all in a step, but that assumes that we will not really have many branches (if it's this branch or this one or ... then use these markers), or there is a certain pattern to branches that makes them all definitely config or metadata or highres.
I've done up the config file in this PR, anyway - what do you think?

[aidanheerdegen]

I forgot a check! We also want this set in config.yaml

manifest:
   reproduce:
      exe: True

Arguably we probably want input: True as well, but still chewing over the implications for users.

[aidanheerdegen]

Another thought popped into my head because I had already made this mistake

https://github.com/ACCESS-NRI/access-om2-configs/blob/release-1deg_jra55_ryf/accessom2.nml#L19

It is common practice to reduce the restart_period (model run time) when testing a configuration, so we should check it is a "reasonable" value.

Now .. what is reasonable?

The one degree has a 5 year run time for both physics only and BGC:

https://github.com/COSIMA/1deg_jra55_ryf/blob/master/accessom2.nml#L19
https://github.com/COSIMA/1deg_jra55_ryf/blob/master%2Bbgc/accessom2.nml#L19

The quarter degree typically runs for 2 years at a time

https://github.com/COSIMA/025deg_jra55_ryf/blob/master/accessom2.nml#L20

but the BGC version is 1 year

https://github.com/COSIMA/025deg_jra55_ryf/blob/master%2Bbgc/accessom2.nml#L20

The tenth degree only runs 3 months at a time

https://github.com/COSIMA/01deg_jra55_ryf/blob/master/accessom2.nml#L22

and the BGC version is 1 month

https://github.com/COSIMA/01deg_jra55_ryf/blob/master%2Bbgc/accessom2.nml#L22

[aidanheerdegen]

It would be good to control (limit) the acceptable keywords in the configurations, partly to provide guidance on what should be used because everyone will come up with their own.

Keywords are a bit of a wild-west and it doesn't seem there are many definitive vocabularies of keywords available, but the Global Change Master Directory (GCMD) Keywords might be one. It might be overkill but it is a great resource, e.g.

https://gcmd.earthdata.nasa.gov/KeywordViewer/scheme/all/10a128a6-12d4-4bce-b25d-2ffc464182f4?gtm_keyword=POLYNYAS&gtm_scheme=Earth%20Science

So .. a list of acceptable keywords (please others add your opinions):

Topic	Keywords (mutually exclusive)
Spatial extent	global, regional
Forcing product	JRA55, ERA5
Forcing mode	repeat-year, ryf, repeat-decade, rdf, interannual, iaf
Model	access-om2, access-om2-025, access-om2-01

I've intentionally left off organisational keywords because that should be covered in the metadata right? Also it might not be mutually exclusive. We should definitely credit COSIMA because they developed these configurations, but also ACCESS-NRI is supporting them now.

[aidanheerdegen]

Speaking of controlled vocabularies, we need to populate the nominal resolution field in the metadata.yaml with something useful.

From this CMIP6 example it seems like a distance measure is the normal approach and the respective values should be

Config resolution	Nominal Resolution
1deg	100 km
025deg	25 km
01deg	10 km

Does that look right to you @aekiss ?

[dougiesquire]

Note the CMIP6 controlled vocab is here. Most are distance measures as you said.

[aekiss]

I think that's close enough and would be familiar to people. Of course it depends on latitude etc. eg see table 6 and fig 5 of the tech report

[aidanheerdegen]

Note the CMIP6 controlled vocab is here. Most are distance measures as you said.

Thanks. WCRP "Controlled" vocabulary I have a 1x1 degree bone to pick with you.

https://github.com/WCRP-CMIP/CMIP6_CVs/blob/main/CMIP6_nominal_resolution.json#L9

[aidanheerdegen]

I think that's close enough and would be familiar to people. Of course it depends on latitude etc. eg see table 6 and fig 5 of the tech report

Absolute distance is a terrible measure eh? "ballpark resolution" might be a better name.

Should we use 1x1 degree, 0.25x0.25 degree and 0.1x0.1 degree?

We don't have to follow CMIP6.

Ignore me. I have already changed the CI goalposts too many times today. So sorry @jo-basevi!

[aekiss]

Absolute distance is worse than degrees, but degrees are also not great (meridional resolution in degrees is also latitude-dependent due to Mercator grid, and then there's the tripolar region). I guess that's why they hedge it with "nominal".

[aidanheerdegen]

And at least distance has some vague meaning for normal people I guess.