Update regional recipe to work with regional-mom6 v1.0.1

[mmr0]

1. Updated to use regional-mom6 v1.0.1.
2. Changed the ACCESS-OM2 model experiment used to produce the boundary forcing as well as the time period, to be consistent with the regional model setup instructions here

Closes #513

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[mmr0]

@navidcy

[navidcy]

Great! Thanks!

It's a good opportunity to force myself install the environment on xp65 to review this. @mmr0 could you point me to the instructions to do so?

[mmr0]

Thanks @navidcy. Some info here:

https://forum.access-hive.org.au/t/access-nri-analysis3-conda-environments-new-release-announcement/4377

When using Australian Research Environment (ARE), add gdata/xp65 to “Storage”, /g/data/xp65/public/modules/ to “Module directories” (under “Advanced options”), and add conda/analysis3 to “Modules”. Additionally, users can switch kernels inside their ARE JupyterLab instance to select the appropriate environment.

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:38Z
----------------------------------------------------------------

Line #1.    client = Client(threads_per_worker = 7)

why change this to 7?

mmr0 commented on 2025-06-10T03:54:40Z
----------------------------------------------------------------

This is a hangover from my larger domain where I needed more memory (per worker) to run `setup_initial_condition'. So, 7 is not a special number. It might be more useful to revert to 1 but add a comment. Here's the relevant thread:

COSIMA/regional-mom6#215 (comment)

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:39Z
----------------------------------------------------------------

Line #1.    #expt_name = "tassie-access-om2-forced"

why commented out code? doesn't add anything

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:39Z
----------------------------------------------------------------

Line #7.    # longitude_extent = [131, 158]

we don't need commented out code; just have one domain here

there is no need to keep history of what was "the original domain" and then "the post-original domain" and what not; this is just an example it's not the reference case that all researchers are using

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:40Z
----------------------------------------------------------------

What are all those xarray warnings? They seem scary? Should we deal with this? @anton-seaice, @dougiesquire do you have an opinion?

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:41Z
----------------------------------------------------------------

Line #5.    #experiment = catalog["01deg_jra55v13_ryf9091"]

remove commented out code

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:42Z
----------------------------------------------------------------

where's the plot?

ashjbarnes commented on 2025-06-13T03:44:52Z
----------------------------------------------------------------

Yeah when we're happy we should re-execute the whole notebook, then clear the output from the top cell so we have the plots, but not 1241521 messages from dask

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:42Z
----------------------------------------------------------------

is this step required?

[review-notebook-app]

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2025-06-10T03:24:43Z
----------------------------------------------------------------

Line #6.        #minimum_layers=1

why is this commented out?

if it's not needed then let's delete it

commented out code doesn't mean anything...

mmr0 commented on 2025-06-10T03:56:28Z
----------------------------------------------------------------

It throws an error with the new version. @ashjbarnes should it be replaced with something or can I just delete?

[navidcy]

I haven't run it yet, but I posted few comments just from glancing the PR -- thanks!

[mmr0]

This is a hangover from my larger domain where I needed more memory (per worker) to run `setup_initial_condition'. So, 7 is not a special number. It might be more useful to revert to 1 but add a comment. Here's the relevant thread:

COSIMA/regional-mom6#215 (comment)

---

View entire conversation on ReviewNB

[mmr0]

It throws an error with the new version. @ashjbarnes should it be replaced with something or can I just delete?

---

View entire conversation on ReviewNB

[ashjbarnes]

It throws an error with the new version. @ashjbarnes should it be replaced with something or can I just delete?

View entire conversation on ReviewNB

Yeah the functions have been changed so that 'minimum layers' is replaced with 'minimum depth', and this is implemented at the initialisation of the experiment rather than at the bathymetry step now.

The ACCESS-OM2 forced notebook should be identical to the demo in the package where possible, as this demo notebook is part of the github testing so will always work with latest version

https://github.com/COSIMA/regional-mom6/blob/main/demos/reanalysis-forced.ipynb

[ashjbarnes]

Yeah when we're happy we should re-execute the whole notebook, then clear the output from the top cell so we have the plots, but not 1241521 messages from dask

---

View entire conversation on ReviewNB

[ashjbarnes]

This looks good, thanks @mmr0 ! I'm happy to approve the PR when

1. Commented out code deleted as pointed out by Navid
2. The notebook has been executed to populate the plot Navid noted are missing
3. Outputs that we don't need has been cleared, e.g. the dask worker stuff after the first cell and some of the walls of text that rmom6 prints out after regridding and writing the config files

I wouldn't worry about handling the errors that @navidcy mentioned for the purposes of this PR, but good to keep note of them in case we need to action them later

[mmr0]

Thanks heaps for looking at this @ashjbarnes @navidcy - sorry it was a mess to begin with. Have done your 1-3. I think it looks tidy now.

[ashjbarnes]

This is great, thanks for the bug finding and updating the notebook @mmr0 !!

[ashjbarnes]

I'm happy - I assume @navidcy is too since his comments are now addressed from what I can see

[navidcy]

Let me run the notebook and then I will approve (and merge)!
Thanks both!

[navidcy]

3. Outputs that we don't need has been cleared, e.g. the dask worker stuff after the first cell and some of the walls of text that rmom6 prints out after regridding and writing the config files

I put the output back in. If they are output when people run the notebook then they should be in the recipe as well. Otherwise people thing that something went wrong when they run themselves. (By people I mean eg myself)

[navidcy]

thanks both!

[ashjbarnes]

@navidcy I disagree. the notebook is now 25% printouts of the dask client. How is this useful to anyone? Perhaps we can find a way to suppress it instead if you do want to have all the outputs included - although this would be the same as just suppressing that particular cell

[navidcy]

As I said, if the recipe code gives different output from what a user gets when they try it then it creates questions in their heads. It definitely creates question in my head.

The recipe showcases what happens when you run a piece of code. So we shouldn't be manually tweaking the output of the cells... If the output is not useful then why is it being printed? (I don't have strong opinion on whether it is useful or not.)
Perhaps we need to disable it from within the regional-mom6 package instead of modifying the output cells by hand?

[ashjbarnes]

As I said, if the recipe code gives different output from what a user gets when they try it then it creates questions in their heads. It definitely creates question in my head.

The recipe showcases what happens when you run a piece of code. So we shouldn't be manually tweaking the output of the cells... If the output is not useful then why is it being printed? (I don't have strong opinion on whether it is useful or not.) Perhaps we need to disable it from within the regional-mom6 package instead of modifying the output cells by hand?

I'm referring to the dask initialisation as this is the biggest offender, making a reader on github scroll for ages to get to the actual code. This happens before rmom6 is even called, so isn't related to the package. Look at the notebook and you'll see what I mean

https://github.com/COSIMA/cosima-recipes/blob/main/Recipes/regional-mom6-forced-by-access-om2.ipynb

[navidcy]

Damn... True, you are absolutely right! I missed that!
Let me rerun the notebook using 2 or 4 CPUs (which will reduce the workers)

[adele-morrison]

I agree @ashjbarnes! This drives me nuts on all the Recipes. I think we should add a comment in like "# You can uncomment this line to print out the client info:". And then comment the "client" line on all the COSIMA Recipes.

[navidcy]

See #530

[navidcy]

When I run the notebooks I see this:

I never see the expanded version with 48 panels of information!
That's why I didn't initially understood what you were referring to @ashjbarnes ..

[navidcy]

Btw, even before #530 when I look at the recipe at

https://cosima-recipes.readthedocs.io/en/latest/Recipes/regional-mom6-forced-by-access-om2.html

the dask cluster output is not expanded.

How come it is expanded for you?

[navidcy]

Are you using GitHub's notebook viewer to browse through the recipes?

[adele-morrison]

I don't use readthedocs, I just look at them on github, e.g. like the link posted above by Ashley.

[navidcy]

I suggest using the docs; it's much better experience.
Do you have a reason to avoid that? It's as easy as 3 clicks (one to click the docs link, one click for "Recipes" and then another click for the recipe you like). Especially if you are driven nuts with all that clutter from Github non-ideal jupyter notebook viewer.

But regardless, I tried to remove bit of the clutter you were both talking about in #530!

[navidcy]

I will take 5min next COSIMA meeting to show people how to look at the recipes because it's a pitty to just use the clunky Github notebook viewer. It's a bit like going to a fancy decorated restaurant with excellent food and you get take away and go eat in your car in the parking lot --- there is so much more the Cookbook has to offer.

@chrisb13 comment #529 (comment) also implies that they don't use the docs or didn't even know they existed! It's probably a lot of people....

[ashjbarnes]

Awesome thanks Navid, this is really helpful I didn't know that either

[navidcy]

https://cosima-recipes.readthedocs.io/en/latest/

this will change everything :)

[chrisb13]

I will take 5min next COSIMA meeting to show people how to look at the recipes because it's a pitty to just use the clunky Github notebook viewer. It's a bit like going to a fancy decorated restaurant with excellent food and you get take away and go eat in your car in the parking lot

Looking forward to trying out this new fancy restaurant at the next COSIMA meeting! Thanks @navidcy.

[chrisb13]

Just looking at the recent release note -- thanks @navidcy!

@mmr0 and @chrisb13 are listed as making "their first contribution" in 496 and 525, respectively but I don't think @mmr0 knew/remembered to add herself to the .zenodo.json file? As both Madi/myself don't appear on the zenodo version of the release?

(Given that my PR was trivial, I don't mind but wanted to highlight the case from this PR from @mmr0. I wonder if there's a way to catch this before the release is made?)

[navidcy]

Thanks for picking it up. We can always update the Zenodo entry for v0.1.3; just added these two as contributors!

There is no trivial and non-trivial contributions! Your contribution also counts! Feel free to open a PR adding yourself (and Madi?) in the .zenodo so that things are automated next time!

[chrisb13]

Thanks @navidcy for fixing this up. I wonder if there's a more "fail safe" way to do this? Perhaps an item on a release checklist?

There is no trivial and non-trivial contributions! Your contribution also counts! Feel free to open a PR adding yourself (and Madi?) in the .zenodo so that things are automated next time!

Oh that's kind! On the adding to the .zenodo, you've gone ahead and done this for both @mmr0 and myself, right?

As a sidenote, I don't think I would have brought it up if it weren't for @mmr0's contribution so I wonder if ECRs and the like might need specific encouragement.

[navidcy]

It's so easy to forget to do that...

If I am reviewing a PR and notice a new contributor I ask them to edit the file. But I also forget (eg didn't mention it in this PR!)

We can't always fix this later (like now).

But it's probably worth adding a mention on editing the .zenodo.json file in the Contributing section in the docs/wiki?

[chrisb13]

Yeah, I bet!

(FWIW, at ACCESS-NRI we have like actual checklists that we go through for various processes but we still forget stuff!)

But it's probably worth adding a mention on editing the .zenodo.json file in the Contributing section in the docs/wiki?

So the README has this:

Contributors to the COSIMA Cookbook are added to the citable DOI entry associated with the repository. Hence, users who put together a pull request for a new contribution, should ensure that the pull request also modifies the .zenodo.json file to include their affiliation details.

I had a quick look and I couldn't see something similar here or here, perhaps these are more important given it's the more detailed instructions. The latter in particular as it hopefully prompts reviewers to suggest that the author adds.