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

Automate parameter sync across configs #119

Open
aekiss opened this issue Mar 13, 2024 · 21 comments
Open

Automate parameter sync across configs #119

aekiss opened this issue Mar 13, 2024 · 21 comments
Assignees

Comments

@aekiss
Copy link
Contributor

aekiss commented Mar 13, 2024

As the number of config branches and repos increases we will need easy ways to

  1. check for parameter differences across the configs (this is also useful for documentation) and
  2. sync them as necessary (noting that some differences will be necessary) so that improvements in one config can find their way into the rest.

I've made a small foray into 1. with https://github.com/COSIMA/access-om3-doc which has all the config branches as separate submodules in https://github.com/COSIMA/access-om3-doc/tree/main/configs and then auto-generates comparison tables in https://github.com/COSIMA/access-om3-doc/tree/main/tables, e.g. tables/*/*diff.md show differences between particular configs. Not sure if this is the best way to go, and it would need modification to work with non-namelist files.

Item 2. might be implementable via a cherry-pick script from one config to all the rest for improvements that all configs should have (e.g. bug fixes).

Thoughts, anyone?

@micaeljtoliveira
Copy link
Contributor

micaeljtoliveira commented Mar 13, 2024

Item 2. might be implementable via a cherry-pick script from one config to all the rest for improvements that all configs should have (e.g. bug fixes).

I think that's doable. The only thing I would be careful about is that anything generated by a such a script needs to be reviewed by a human before being added to the configurations.

Regarding 1., it might be worth using the IO routines from OM3-Utils to parse the different configurations.

@aekiss
Copy link
Contributor Author

aekiss commented Mar 13, 2024

Of course if we keep the parameter file layouts very consistent (and I think we should!), we can get away with diff for 1, at least for comparisons between our own configs rather than others.

@anton-seaice
Copy link
Contributor

  1. check for parameter differences across the configs (this is also useful for documentation) and

Could this be automated too? I think it gets hard to automate with a github action because there are multiple repositories (i.e. MOM6-CICE6 and MOM6-CICE6-WW3) and branches but will only be one set of docs?

@anton-seaice
Copy link
Contributor

Item 2. might be implementable via a cherry-pick script from one config to all the rest for improvements that all configs should have (e.g. bug fixes).

I think that's doable. The only thing I would be careful about is that anything generated by a such a script needs to be reviewed by a human before being added to the configurations.

Regarding 1., it might be worth using the IO routines from OM3-Utils to parse the different configurations.

I guess we could start with a script to run after the first PR is merged, which then cherry-picks the changes to the other relevant repos + branches ? I think the script would need to be triggered manually and need input for which branches to make PRs for?

@aekiss
Copy link
Contributor Author

aekiss commented Mar 14, 2024

  1. check for parameter differences across the configs (this is also useful for documentation) and

Could this be automated too? I think it gets hard to automate with a github action because there are multiple repositories (i.e. MOM6-CICE6 and MOM6-CICE6-WW3) and branches but will only be one set of docs?

I'm not sure of the technical limitations of GH Actions being triggered across repos, but the actual submodule updating and table regeneration are already scripted with update_configs.sh and make_tables.py respectively

@aekiss
Copy link
Contributor Author

aekiss commented Mar 14, 2024

Item 2. might be implementable via a cherry-pick script from one config to all the rest for improvements that all configs should have (e.g. bug fixes).

I think that's doable. The only thing I would be careful about is that anything generated by a such a script needs to be reviewed by a human before being added to the configurations.

I guess we could start with a script to run after the first PR is merged, which then cherry-picks the changes to the other relevant repos + branches ? I think the script would need to be triggered manually and need input for which branches to make PRs for?

That's what I was thinking. There could be a file supplying the repos and branches to sync. A .gitmodules file would do the trick, e.g. https://github.com/COSIMA/access-om3-doc/blob/main/.gitmodules

@anton-seaice
Copy link
Contributor

Item 2. might be implementable via a cherry-pick script from one config to all the rest for improvements that all configs should have (e.g. bug fixes).

I think that's doable. The only thing I would be careful about is that anything generated by a such a script needs to be reviewed by a human before being added to the configurations.

I guess we could start with a script to run after the first PR is merged, which then cherry-picks the changes to the other relevant repos + branches ? I think the script would need to be triggered manually and need input for which branches to make PRs for?

That's what I was thinking. There could be a file supplying the repos and branches to sync. A .gitmodules file would do the trick, e.g. https://github.com/COSIMA/access-om3-doc/blob/main/.gitmodules

Oh yes - but also some way to specify it on a PR only basis - e.g. to copy only IAF changes to IAF configs, or BGC changes only to BGC etc ...

@aekiss
Copy link
Contributor Author

aekiss commented Mar 14, 2024

Would it be simpler to make draft PRs for all configs, and then cancel the inapplicable ones and manually tweak the rest as needed before merging?

@aekiss
Copy link
Contributor Author

aekiss commented Mar 14, 2024

We may face a similar problem with documentation #120 (comment)

@micaeljtoliveira
Copy link
Contributor

After some thinking and some further discussions with @aekiss, here is a new proposal about how to automate the parameter synchronisation across repos.

The idea would be following. When a developer opens a PR in one of the config repos, they would have the option to specify in a comment a list of commits to be cherry-picked and a target repo/branch. The comment would look like this (exact syntax to be determined later):

cherry-pick <commit_hash_1> <commit_hash_2> <repo_a/branch> 
cherry-pick <commit_hash_1> <repo_b/branch>

Each line in such comment would in turn trigger a github actions pipeline that would perform the following actions:

  1. clone the git repo of the original PR
  2. add the target repo as a git remote
  3. create a new branch from the target branch
  4. cherry-pick the target commits into the new branch
  5. push the new branch to the target repo
  6. open a PR to merge the new branch into the target branch

There are a few advantages of this approach:

  • it does not create unnecessary PRs
  • uses git cherry-pick, which, by doing a proper three-way merge, is much more robust than a simple diff-based approach
  • gives full control to developers about which commits to cherry-pick and what repos/branches to target.

@aekiss
Copy link
Contributor Author

aekiss commented Mar 28, 2024

Could also have a bot provide this list for all possibly relevant repos and branches so we don't miss any.

@anton-seaice
Copy link
Contributor

This sounds very promising :)

Could also have a bot provide this list for all possibly relevant repos and branches so we don't miss any.

Maybe this could be a checklist with on/off tickboxes?

to specify in a comment

Also, Dougie at some point suggested we could use the "Labels" feature on github to mark which branches to cherry-pick across to, instead of using the comment?

@micaeljtoliveira
Copy link
Contributor

Maybe this could be a checklist with on/off tickboxes?

This might be possible. We just need to figure out how to make that work in a github action.

Also, Dougie at some point suggested we could use the "Labels" feature on github to mark which branches to cherry-pick across to, instead of using the comment?

That might also be possible, but I suspect the comment will be easier to implement. But worth exploring if people think this would be a good alternative. Note however that one would still need a way of providing a list of commit to cherry-pick, because sometimes it might be desirable to only pick a few selected commits from a branch.

@aidanheerdegen
Copy link

The idea would be following. When a developer opens a PR in one of the config repos,

This would require creating a branch and commit a modification (or an empty commit) to be able to kick-start the GitHub PR process.

they would have the option to specify in a comment a list of commits to be cherry-picked and a target repo/branch. The comment would look like this (exact syntax to be determined later):

It's a neat idea. I'd definitely ask @CodeGat for advice, as he's already done some comment bot workflows so would likely have some wisdom in this area.

I'm not sure you're saving a lot doing this through CI though.

I think scripting it and using gh to create the PRs would be a lot faster, or using CI to generate a bunch of PRs automatically, and just close the ones you don't want.

Or get CI to create a first tranche of PRs and require approval to create more.

It's something I'm very interesting in as we'd like to implement something like this to trigger PRs when new model versions are deployed.

@aekiss
Copy link
Contributor Author

aekiss commented May 16, 2024

using CI to generate a bunch of PRs automatically, and just close the ones you don't want

I like that idea, if the CI would auto-create a PR for all relevant branches automatically to ensure nothing gets overlooked (which may become important as branches become more numerous).

But the auto-generated PRs should be tagged as such (and ideally be made drafts) so it is clear that they need a human to confirm that they should actually go ahead.

@dougiesquire
Copy link
Collaborator

I think having the ability to only cherry-pick to specified branches will be useful. E.g. if we make a change that's only relevant to one resolution, it will be annoying to have to go and close the PRs that were automatically opened for other branches. A middle ground might be to allow pattern-matching repo/branches:

# Open PR for all configs
cherry-pick <commit_hash_1> *

# Open PR for all 1 deg configs
cherry-pick <commit_hash_1> */1deg*

# Open PR for all MOM6-CICE6 configs
cherry-pick <commit_hash_1> MOM6-CICE6/*

Ofc, we'd then have to keep a list of our configuration branches somewhere.

@aekiss
Copy link
Contributor Author

aekiss commented Aug 1, 2024

@micaeljtoliveira the requirement to cherry-pick across repos would disappear if we have one repo for all configs - see #197

@aekiss
Copy link
Contributor Author

aekiss commented Aug 18, 2024

It would be good to have cherry-pick instructions here. I'd added a quick mention but it could do with more detail. https://github.com/COSIMA/access-om3/wiki/Git-practices#branch-synchronisation-for-releases

@aekiss
Copy link
Contributor Author

aekiss commented Aug 18, 2024

Also (a trivial thing) we're missing ! before cherry-pick in this comment: https://github.com/COSIMA/MOM6-CICE6/blob/main/.github/workflows/automatic-cherry-pick.yaml#L33

@aekiss
Copy link
Contributor Author

aekiss commented Sep 3, 2024

We're retaining separate repos for configs with different executables (#197) so it would be good to be able to cherry-pick between configs. It would also be good to be able to cherry-pick to @kieranricardo's CM3 configs to help keep them in sync.

@aekiss
Copy link
Contributor Author

aekiss commented Sep 3, 2024

Just to note: the comparison table can be useful for spotting any forgotten cherry-picks. #211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants