Skip to content
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.

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

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

Open
aekiss opened this issue Mar 12, 2024 · 14 comments
Open

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

aekiss opened this issue Mar 12, 2024 · 14 comments
Assignees
Labels
documentation Improvements or additions to documentation mom6 Related to MOM6 priority:high

Comments

@aekiss
Copy link
Contributor

aekiss commented Mar 12, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

aekiss commented Mar 14, 2024

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?

@aekiss aekiss added the documentation Improvements or additions to documentation label Mar 14, 2024
@micaeljtoliveira
Copy link
Contributor

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
Copy link
Contributor Author

aekiss commented Mar 14, 2024

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
Copy link
Contributor Author

aekiss commented Mar 14, 2024

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 added a commit to ACCESS-NRI/access-om3-configs that referenced this issue Mar 14, 2024
@aekiss
Copy link
Contributor Author

aekiss commented Mar 14, 2024

I added MOM_parameter_doc.* manually in this PR ACCESS-NRI/access-om3-configs#45

@aekiss
Copy link
Contributor Author

aekiss commented Mar 14, 2024

Note that they are incomplete due to this bug #121

@aekiss aekiss added the mom6 Related to MOM6 label Mar 14, 2024
@aekiss
Copy link
Contributor Author

aekiss commented Mar 27, 2024

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

anton-seaice commented Aug 1, 2024

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
Copy link
Contributor Author

aekiss commented Sep 11, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

aekiss commented Sep 11, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation mom6 Related to MOM6 priority:high
Projects
Status: No status
Status: No status
Development

No branches or pull requests

5 participants