COSIMA / access-om3

ACCESS-OM3 global ocean-sea ice-wave coupled model
13 stars 6 forks source link

CI to update MOM_parameter_doc.* and add to PRs #117

Open aekiss opened 6 months ago

aekiss commented 6 months ago

When MOM6 runs, it outputs these files detailing every parameter actually used by the model, as described here

MOM_parameter_doc.layout
MOM_parameter_doc.debugging
MOM_parameter_doc.all

These provide comprehensive parameter documentation, so even though they are output files they would be useful to commit to the repo, as is done in MOM6-examples, e.g. ocean_only/global.

For this to be useful rather than misleading, MOM_parameter_doc.* would need to be kept in sync with MOM_input (and the executable), e.g. via a short MOM6 run as part of CI on PRs. Does that sound feasible?

micaeljtoliveira commented 6 months ago

For this to be useful rather than misleading, MOM_parameter_doc.* would need to be kept in sync with MOM_input (and the executable), e.g. via a short MOM6 run as part of CI on PRs. Does that sound feasible?

Do you mean a CI workflow that would run MOM6 to check if the files are up to date, or a CI workflow that would run MOM6 and then update the files?

aekiss commented 6 months ago

I meant the latter. MOM6 will re-generate MOM_parameter_doc.* when run (probably just the init is needed, so 1 timestep is more than enough). I thought we could then do

git add MOM_parameter_doc.*
git commit -m "update MOM_parameter_doc.*"

and add this to the PR. I'm guessing this would do nothing for unchanged files?

Perhaps something like this is already done for MOM6-examples?

micaeljtoliveira commented 6 months ago

Unfortunately that kind of approach is problematic for a couple of reasons.

First, the CI needs to push the new commit to the remote, otherwise the commit will only exist in the github runner. Pushing to the remote will in turn trigger a new CI run, that will in turn add another commit, etc.

The fact that the CI is adding a commit to the remote git repository can also confuse developers and create conflicts with their local branch (e.g., when doing a git push or git pull).

The usual approach is instead to mark the CI as failed if the files in question are not up to date. Fixing it will then require some extra work from the developer (download the new version of the files + commit them), but avoids all of the above issues.

aekiss commented 6 months ago

The usual approach is instead to mark the CI as failed if the files in question are not up to date. Fixing it will then require some extra work from the developer (download the new version of the files + commit them), but avoids all of the above issues.

OK that sounds like a better approach then - have CI do a short run to update MOM_parameter_doc.* and fail if they change.

aekiss commented 6 months ago

NB runs of 0 or 1 timesteps don't work, but 2 does, i.e.

     stop_n = 2
     stop_option = nsteps

in nuopc.runconfig

aekiss commented 6 months ago

I added MOM_parameter_doc.* manually in this PR https://github.com/COSIMA/MOM6-CICE6/pull/45

aekiss commented 6 months ago

Note that they are incomplete due to this bug https://github.com/COSIMA/access-om3/issues/121

aekiss commented 5 months ago

Does anyone know if there is an analogous way to save all the parameters for CICE6, including the defaults that weren't set explicitly?

anton-seaice commented 5 months ago

There is a log file section "Overview of model configuration with relevant parameters" which should capture all the configuration options. I think that whole section (up to 'istep1') should be basically unchanged between runs unless the configuration / build changes

access-hive-bot commented 2 months ago

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

https://forum.access-hive.org.au/t/cosima-twg-meeting-minutes-2024/1734/12

anton-seaice commented 1 month ago

We think this can be implemented as a test in CI-workflows, which fails if any of the fours docs files has changed. And then have a comment trigger to update the docs when desired by the developer (e.g. !updatedocs in a PR comment)

@codegat and I will look at this after #41

aekiss commented 1 week ago

Ideally payu would also copy the most recent archive/output*/MOM_parameter_doc.* to the docs directory after each run so that the git runlog would record any changes.

anton-seaice commented 1 week ago

Ideally payu would also copy the most recent archive/output*/MOM_parameter_doc.* to the docs directory after each run so that the git runlog would record any changes.

I think the git runlog commits at the start of run, so it would only get captured if it was run again ?

aekiss commented 1 week ago

hmm, good point. Would it be confusing to commit it at the start of the subsequent run?

Maybe this would best be done via a script that could be put in the userscripts archive in config.yaml. The script could do the commit, with a message that the change applied to the previous run.