-
Notifications
You must be signed in to change notification settings - Fork 11
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
683 update access esm how to run #726
683 update access esm how to run #726
Conversation
|
Merge remote-tracking branch 'origin/683-update-access-esm-how-to-run' into 683-update-access-esm-how-to-run
Hi everyone, I've finally finished adding in some of the more ESM1.5 specific information now that the changes to the configurations and payu are (mostly) finalised and I think it's ready for a review. The following two points are still waiting on further information/links, but everything else should be ready for a review:
Sorry about the delay on this! |
Thanks for that @blimlim! Looks great to me! I would perhaps suggest to merge this PR into the development branch for now. For the rest of the changes, happy to create a new PR and once we have all the changes, we can deploy it on the main branch. :) |
Thanks @KAUR1984! I just realised there were a couple of spelling errors which I've just fixed up, and then will merge into development |
I'm part way through reviewing. Should I just close off that review and allow you to make the suggested changes? |
That sounds good! I'll wait until I've added any suggested changes before merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @blimlim! Awesome work.
I have made a number of suggestions, not all are necessarily required, happy to leave it to you to decide what to keep and what to not, or further modify.
I have some thoughts about the post-processing, but that may require changes to the script invocation, which will then have to be changed here to be consistent.
Review suggestions Co-authored-by: Aidan Heerdegen <[email protected]>
Thanks for spotting those things – I think they're all good suggestions and I've added them in.
Sounds good, lets see if we can simplify how the script is called. |
Co-authored-by: Aidan Heerdegen <[email protected]>
I've just updated the docs with the change to the NetCDF conversion behaviour and added in the correct payu version. After looking over everything, I think the information is now correct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for all the hard work.
76bdfbf
to
d2e612f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one small change, otherwise looks good and thanks for the changes.
d2e612f
to
69a4107
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Thanks!
Description
This pull request updates the run access esm tutorial to match the structure of the OM2 tutorial, and to include changes related to the latest payu updates and the (to be) released configurations.
The new ESM how to run page was set up by copying the OM2 page, and modifying the information to be correct for ESM1.5.
This is not currently ready to merge, as some changes cannot be made until the final release, specifically:
release-
branches are ready.postscript
example will have to wait for the NetCDF conversion to be finalised.In the mean time though, it would be great to get feedback and suggestions on the other changes!
Fixes #683
Type of change
Checklist: