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

Updates to 1deg_jra55do_ryf #91

Merged
merged 5 commits into from
Aug 9, 2024

Conversation

dougiesquire
Copy link
Collaborator

@dougiesquire dougiesquire commented Jul 29, 2024

This PR:

I'll try cherry-pick to other configs once this is merged

@anton-seaice
Copy link
Contributor

anton-seaice commented Jul 29, 2024

We should update the wiki, and OCN_modelio:: in nuopc.runconfig (Can we set all the lines starting with pio to unused?) (i.e. COSIMA/access-om3#185)

@anton-seaice
Copy link
Contributor

anton-seaice commented Jul 29, 2024

I ran this as written:

ll -h archive/output000/access-om3.mom6.*
-rw-r----- 1 as2285 tm70 185M Jul 29 15:15 archive/output000/access-om3.mom6.h.native_1900_01.nc
-rw-r----- 1 as2285 tm70  80M Jul 29 15:15 archive/output000/access-om3.mom6.h.sfc_1900_01.nc
-rw-r----- 1 as2285 tm70 847K Jul 29 15:15 archive/output000/access-om3.mom6.h.static.nc
-rw-r----- 1 as2285 tm70  97M Jul 29 15:15 archive/output000/access-om3.mom6.h.z_1900_01.nc

with payu walltime of 1055s

and then ran with deflate=1:

-rw-r----- 1 as2285 tm70 211M Jul 29 15:35 archive/output001/access-om3.mom6.h.native_1900_02.nc
-rw-r----- 1 as2285 tm70  79M Jul 29 15:35 archive/output001/access-om3.mom6.h.sfc_1900_02.nc
-rw-r----- 1 as2285 tm70 993K Jul 29 15:35 archive/output001/access-om3.mom6.h.static.nc
-rw-r----- 1 as2285 tm70 111M Jul 29 15:35 archive/output001/access-om3.mom6.h.z_1900_02.nc

with payu walltime of 1022s

So it looks like the extra time for compression is maybe worth it. Noting tuning the IO layout might improve this.

@anton-seaice
Copy link
Contributor

I ran this as written:

ll -h archive/output000/access-om3.mom6.*
-rw-r----- 1 as2285 tm70 185M Jul 29 15:15 archive/output000/access-om3.mom6.h.native_1900_01.nc
-rw-r----- 1 as2285 tm70  80M Jul 29 15:15 archive/output000/access-om3.mom6.h.sfc_1900_01.nc
-rw-r----- 1 as2285 tm70 847K Jul 29 15:15 archive/output000/access-om3.mom6.h.static.nc
-rw-r----- 1 as2285 tm70  97M Jul 29 15:15 archive/output000/access-om3.mom6.h.z_1900_01.nc

with payu walltime of 1055s

and then ran with deflate=1:

-rw-r----- 1 as2285 tm70 211M Jul 29 15:35 archive/output001/access-om3.mom6.h.native_1900_02.nc
-rw-r----- 1 as2285 tm70  79M Jul 29 15:35 archive/output001/access-om3.mom6.h.sfc_1900_02.nc
-rw-r----- 1 as2285 tm70 993K Jul 29 15:35 archive/output001/access-om3.mom6.h.static.nc
-rw-r----- 1 as2285 tm70 111M Jul 29 15:35 archive/output001/access-om3.mom6.h.z_1900_02.nc

with payu walltime of 1022s

So it looks like the extra time for compression is maybe worth it. Noting tuning the IO layout might improve this.

Oh february has 28 days and january has 31 ... the time difference might be unrelated to compression

Copy link
Collaborator Author

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this will run will it? Scratch that, I misread your change. I assume you tried to run with these changes?

@anton-seaice
Copy link
Contributor

Yes it works (or im having a 5 oclock moment!). It only removes lines for components which don't exist

@dougiesquire
Copy link
Collaborator Author

Let me know when you're happy and I'll do another test run with your added changes

@anton-seaice
Copy link
Contributor

Nothing further @dougiesquire

@dougiesquire
Copy link
Collaborator Author

Thanks @anton-seaice. Test run ran successfully. But I'm going to bundle in a couple more changes to close COSIMA/access-om3#168 and COSIMA/access-om3#170 to reduce the amount of cherry-picking I have to do. I won't include changes for COSIMA/access-om3#194 in this PR since this requires more testing.

Copy link

Thanks @anton-seaice. Test run ran successfully. But I'm going to bundle in a couple more changes to close COSIMA/access-om3#168 and COSIMA/access-om3#170 to reduce the amount of cherry-picking I have to do. I won't include changes for COSIMA/access-om3#194 in this PR since this requires more testing.

Automatic cherry-pick failed. Incorrect cherry-pick command:

  • the command is missing the into separator.

@dougiesquire
Copy link
Collaborator Author

^ 👀 @micaeljtoliveira

@micaeljtoliveira
Copy link
Contributor

Oh no... We need to change the parsing of the comment to make sure we only trigger the workflow if the expression "cherry-pick" is at the beginning of the line.

Copy link

Oh no... We need to change the parsing of the comment to make sure we only trigger the workflow if the expression cherry-pick is at the beginning of the line.

Automatic cherry-pick failed. Incorrect cherry-pick command:

  • the command is missing the into separator.

@dougiesquire
Copy link
Collaborator Author

Maybe worth having a character at the start to distinguish the command? E.g. Tommy's versioning commands start with a "!".

@aekiss
Copy link
Contributor

aekiss commented Jul 31, 2024

also trigger only for !cherry-pick (with trailing space) or !cherry-pick into ?

@micaeljtoliveira
Copy link
Contributor

I think just adding the exclamation mark is enough to avoid 99.99% of unwanted triggers of the command.

Triggering it when not specifying into is a good thing, as it gives some feedback to the user that might otherwise be left wondering why the comment didn't trigger the workflow.

@micaeljtoliveira
Copy link
Contributor

See #92 for the fix.

@micaeljtoliveira
Copy link
Contributor

It should now be safe to discuss cherry-picks in PRs without being interrupted by the github bot.

@aekiss
Copy link
Contributor

aekiss commented Aug 1, 2024

cherry-pick cherry-pick cherry-pick cherry-pick cherry-pick cherry-pick cherry-pick cherry-pick

@aekiss
Copy link
Contributor

aekiss commented Aug 1, 2024

seems to work :-)

@dougiesquire dougiesquire changed the title Use netcdf4 with MOM6: 1deg_jra55do_ryf Updates to 1deg_jra55do_ryf Aug 2, 2024
@dougiesquire
Copy link
Collaborator Author

@anton-seaice, I've bundled a couple more things into this PR with the hope it might reduce the amount of cherry-picking and PRs. Let me know if you're happy and I'll do some history rewriting to simplify the cherry-picking.

Copy link
Contributor

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the wiki please for where to set MOM IO & netcdf configs ?
I suggest we put this in four commits, and link to the related issue for each PR in the commit / description.

Is there somewhere obvious to document it needs payu/dev or payu 1.1.4 to run ? (in config.yaml, in the README, somewhere else?)

I haven't tested, but expect its fine.

Flag OCN_modelio::pio_* params as unused in nuopc.runconfig

remove extra lines in nuopc.runconfig

update comment for ocean nc type
- work-around symlink restart bug in CICE
- concatenate CICE daily output
- auto-generate an intake-esm datastore
@anton-seaice
Copy link
Contributor

This looks good:

  • ncdump of MOM history and restart files shows format = netcdf-4 , shuffle = true, chunksize exist and are full size of x/y/time etc, deflate is 4.
  • payu finds the exe using the module, and the module name contains the access-om3 git commit used for the build. full binary path is in the manifests
  • folder is named INPUT and no errors are given loading forcings / grids etc
  • setup user scripts copies cice restart into work folder but there no extra copies in the output archive, intake esm datastore exists. We have however overwritten the monthly cice output with the daily cice output !

@anton-seaice
Copy link
Contributor

Ill update the cice config and add to this PR

@anton-seaice
Copy link
Contributor

For reasons that are unclear, the hist_suffix namelist config in cice appears to only support one character per history frequency. I can set this to 'd' and 'm' to progress the PR, or we can park it. Ill raise an issue tomorrow.

@anton-seaice
Copy link
Contributor

This is blocked on COSIMA/access-om3#201 for now

@anton-seaice
Copy link
Contributor

I suggest we drop the userscripts from this PR and merge the rest :)

@dougiesquire
Copy link
Collaborator Author

We decided in a meeting today to set hist_suffix to:

  • '.' for daily files
  • 'm' for monthly files

The CICE filenames will look a bit yucky until COSIMA/access-om3#201 is resolved, but we can then at least use the concatenation userscript without modification.

…er at this point, and the userscript to concat ice daily output expects the default name to be used. See COSIMA/access-om3#201
@anton-seaice
Copy link
Contributor

* '.' for daily files

sadly i was wrong and it needs to be blank until we change the userscripts see COSIMA/om3-scripts#21

Copy link
Contributor

@anton-seaice anton-seaice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done the old change and approve trick :). This works now.

@dougiesquire
Copy link
Collaborator Author

sadly i was wrong and it needs to be blank until we change the userscripts see COSIMA/om3-scripts#21

I vote we change the userscript now to something that can work with both:

  • the *1day.mean* format
  • some workaround format for now

Is that possible? Then we won't end up with more dependent changes down the track

@anton-seaice
Copy link
Contributor

sadly i was wrong and it needs to be blank until we change the userscripts see COSIMA/om3-scripts#21

I vote we change the userscript now to something that can work with both:

* the `*1day.mean*` format

* some workaround format for now

Is that possible? Then we won't end up with more dependent changes down the track

Yep "should" be fine. We can do that change at any point going forward - it doesn't impact this PR. It just needs to be done before changing the filenames to the desired filenames.

@dougiesquire
Copy link
Collaborator Author

it doesn't impact this PR

True - I wasn't thinking clearly sorry

@dougiesquire dougiesquire merged commit 7beab25 into 1deg_jra55do_ryf Aug 9, 2024
@dougiesquire
Copy link
Collaborator Author

!cherry-pick 34e6e16 a8bd3a1 b191c68 81262fa 615c049 into 1deg_jra55do_iaf

Copy link

github-actions bot commented Aug 9, 2024

!cherry-pick 34e6e16 a8bd3a1 b191c68 81262fa 615c049 into 1deg_jra55do_iaf

Automatic cherry-pick failed. Could not find commit(s) 34e6e16 a8bd3a1 b191c68 81262fa 615c049 in 1deg_jra55do_ryf.

@dougiesquire
Copy link
Collaborator Author

!cherry-pick d24a5d2 2ee5b1d e152ad4 9f551ec 7beab25 into 1deg_jra55do_iaf

Copy link

github-actions bot commented Aug 9, 2024

Automatic Git cherry-picking of commit(s) d24a5d2 2ee5b1d e152ad4 9f551ec 7beab25 into 1deg_jra55do_iaf failed. This usually happens when cherry-picking results in a conflic or an empty commit. To manually cherry-pick the commits and open a pull request, please follow these instructions:

  1. Create new branch from target branch:
git checkout 1deg_jra55do_iaf
git pull
git checkout -b cherry_pick_from_pr91_into_1deg_jra55do_iaf
  1. Cherry-pick commits:
git cherry-pick d24a5d2c8f057ef3e7571fcde3abd6f2db13f753 2ee5b1d9e2852dcb8ab7a220e15d95eea09eceb6 e152ad48f0c9b41d9709b691cec1641275b6333a 9f551ec66734f5cd30c88f9e7479af4fdd000530 7beab257bfecb6d27227875dced02016f271a9ac
  1. Fix any conflicts and/or empty commits by following the instructions provided by Git.
  2. Push the new branch:
git push --set-upstream origin cherry_pick_from_pr91_into_1deg_jra55do_iaf
  1. Open a new pull request on github making sure the target branch is set to 1deg_jra55do_iaf.

@aekiss
Copy link
Contributor

aekiss commented Aug 9, 2024

@dougiesquire are you hitting merge conflicts due to the change in MOM_input format in 1deg_jra55do_ryf?

Should I expedite COSIMA/access-om3#200?

@dougiesquire
Copy link
Collaborator Author

There are conflicts in the MOM_input. But there are conflicts in every other modified file also, so expediting COSIMA/access-om3#200 won't help much here.

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

Successfully merging this pull request may close these issues.

4 participants