Conversation
|
@chrisb13 - we can add the MOM version to this branch too. It doesn't build yet. There is some more to chase down in CESM_share still (It gives this mysterious error: ) |
|
If ESCOMP/CESM_share#58 gets merged we can remove the #ifdef TIMING patch again. |
|
If ESCOMP/WW3#33 gets merged we can remove the ifdef W3_IC4 patch too |
|
I also asked about the wrong variable name in CDEPS here I moved a newer CDEPS because it's sets a default value for a namelist parameter we don't set, (ESCOMP/CDEPS#315) |
660e4d6 to
038cc27
Compare
25ec99c to
e47ee7c
Compare
d809721 to
6c082da
Compare
|
This is basically ready for review. This change implements new component versions for the model components as follows:
See individual commits for minor implementation details. See pre-release deployment: and Draft config update: |
|
This pull request has been mentioned on ACCESS Hive Community Forum. There might be relevant details there: https://forum.access-hive.org.au/t/cosima-twg-announce/401/54 |
|
This pull request 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-2025/4067/2 |
aekiss
left a comment
There was a problem hiding this comment.
LGTM on a quick look, but I'm too far removed from the nitty-gritty to pick much up. Maybe @dougiesquire could have a look if he's back?
chrisb13
left a comment
There was a problem hiding this comment.
LGTM too, although I'm most familiar with the MOM changes. Small things...
To be clear, I've understood this PR to involve these 3 sets of changes (22 commits), plus model component updates, which would all be accepted/merged at once (correct?):
- main...209-new-build
- ACCESS-NRI/ACCESS-OM3@main...209-new-cosima-build-2
- ACCESS-NRI/access-om3-configs@dev-1deg_jra55do_ryf...209-dev-1deg_jra55do_ryf
On a practical point, once everything is merged (assuming you'll do the default GH 'create merge commit'), it's not clear to me how this reference to 209-new-build will work? @micaeljtoliveira tells me the branch name will remain where it is now but the CI may choose to work off the merge commit, which in some cases could be problematic. I see how it's convenient to reference a branch when testing an evolving build but I think now that it's at a release it is safest for this to be changed to a tagged commit. I also think a tagged commit should be used for CICE (perhaps a preliminary one whilst you're awaiting resolution on this).
What is the reason for this failing? I note that the repro check actually uses pr33-12 (not the failing pr33-14). (Although, the symmetric option has been tested elsewhere.) In general, can we please write a brief status when running CI commands such that there's a record of what's changed/why it's being run.
Since @dougiesquire is back on Monday, perhaps we should ask him to have a quick look too?
Sort of. This PR merges the first one, which i've tried to work into 6 neat commits. The other two are related, but need further updates which are dependent on this update. And then get their own PRs. And another 4 ish PRs for the other config branches in om3-configs.
Yes
Ah yep. Ill merge the CICE PR first and update the commit in this PR.
I think the PR is marked as failing because the most recent commit triggered build failed.
Okie |
|
@anton-seaice. Thanks for clarifying, sounds good.
Ok, let's have a chat about this next week. Would be good to make sure we're on the same page -- to my mind, the safest option is to make the tag before the merge (is that what you mean?). |
6c082da to
785df9f
Compare
|
At the team meeting today - we decided to use patches for MOM6 in this release instead of a commit from a branch. I've changed MOM6 to pull the latest commit from MOM-ocean, and reverted the patches. The build fails now and I assume that @dougiesquire and or @chrisb13 will fix the patch files in the MOM6/patches folder |
|
It offers me a delete option for the |
Probably yes, but maybe worth holding off until we have a working alternative |
|
Okay, okay, I'll get this working locally first 😑... |
d6c7576 to
403b96a
Compare
|
I've redone the patch files for the recent MOM update (now up to date with current xref ACCESS-NRI/MOM6#5 |
dougiesquire
left a comment
There was a problem hiding this comment.
Thanks @anton-seaice et al - looks great! One question, one suggestion.
|
Build with todays review changes (redeploy 16): ACCESS-NRI/ACCESS-OM3#33 (comment) and the test with MOM6-CICE6: ACCESS-NRI/access-om3-configs#154 (comment) I also ran an adhoc test with MOM6-CICE6-WW3 which worked with removal of |
…ifdef timing in CESM_share Co-authored-by: Dougie Squire <42455466+dougiesquire@users.noreply.github.com>
…d and rpointer fix CICE 2025.01.0 from access-nri fork
2266467 to
6f4670c
Compare
- update patch files for updated MOM6 version - Add new modules to MOM CMakeLists Co-authored-by: Dougie Squire <42455466+dougiesquire@users.noreply.github.com>
6f4670c to
f05ae2c
Compare
|
Ill double check on Tuesday and then merge :-) |
Draft of new versions for ACCESS-NRI/access-om3-configs#390