Version 1.0: North Atlantic Update - Tides, Curved Boundaries, Config, and More!😀🤯🥳

[manishvenu]

Authors: @ashjbarnes, @manishvenu, @helenmacdonald

Description:

Ashley and I worked together to add a few features to RM6. While we get the changes ready to merge with the main COSIMA branch, please check out this draft with a list of the changes below.

Major Features:

1. Accepting curved boundary supergrids and bathymetry (Add angled grids to RMOM6 CROCODILE-CESM/regional-mom6#13)
2. Implement Boundary Tides from OSU's TPXO using GFDL NWA25 Code (Merge various quality-of-life changes & tides boundary functions (From GFDL NWA25) into RMOM6 CROCODILE-CESM/regional-mom6#12)
3. Import experiment configuration from a light config JSON (Merge various quality-of-life changes & tides boundary functions (From GFDL NWA25) into RMOM6 CROCODILE-CESM/regional-mom6#12)
4. Major bathymetry fixes for land-sea mask (Various bathymetry improvements / bugfixes CROCODILE-CESM/regional-mom6#17)
5. Moving experiment specific changes to MOM_override (All OBC, Tides, and other minor changes) and out of MOM_input
6. New GLORYS downloader (that can account for curved boundaries) (Merge various quality-of-life changes & tides boundary functions (From GFDL NWA25) into RMOM6 CROCODILE-CESM/regional-mom6#12)
7. Tests: Additional tests (tides. rotation, and regridding), Conftest.py, Add to Docker Image (MOM6 Angle Calculation CROCODILE-CESM/regional-mom6#34)
8. Changing function names to verb beginnings (initial_condition -> setup_initial_condition) (Merge various quality-of-life changes & tides boundary functions (From GFDL NWA25) into RMOM6 CROCODILE-CESM/regional-mom6#12)
9. Add properties (i.e. more functional object variables) and dataset plotting (Merge various quality-of-life changes & tides boundary functions (From GFDL NWA25) into RMOM6 CROCODILE-CESM/regional-mom6#12)
10. Add Rotation: Add MOM6 angle calculation at t points (MOM6 doesn't use an input hgrid's angle_dx field. It calculate the angle internally at t-points) & two angle approximation methods for the boundary q/u/v points (MOM6 Angle Calculation CROCODILE-CESM/regional-mom6#34)
11. Add Land Mask (Fix tides on angled boundaries, consolidate regridded of velocity and tracers, add proper body tide parameters, add MOM6's angle calculation. CROCODILE-CESM/regional-mom6#33)

Minor Features:

1. Ability to code without greek letters
2. Removed unintentional machine-specific code
3. Increased print statements

Breaking Changes (for backwards compatibility):

1. Functions changed names, so it's not compatible with older code. All that needs to be updated is the function name itself in previous code (like the demos). (Merge various quality-of-life changes & tides boundary functions (From GFDL NWA25) into RMOM6 CROCODILE-CESM/regional-mom6#12)

Documentation Changes:

1. Update README and Docs (MOM6 Angle Calculation CROCODILE-CESM/regional-mom6#34, Fix tides on angled boundaries, consolidate regridded of velocity and tracers, add proper body tide parameters, add MOM6's angle calculation. CROCODILE-CESM/regional-mom6#33)

Co-authored-by: @ashjbarnes, @helenmacdonald

this PR closes #184 , #168 , and also #55 in a roundabout way since user now specifies which boundaries they want, and the MOM inputs are adjusted accordingly.

Before merging, need to resolve the following:

• Include a curved boundary / tide example alongside the existing Tassie example (Adding example notebook to demonstrate new features CROCODILE-CESM/regional-mom6#41)
• Resolve the tidal regridding issue here (Tidal regridding doesn't appear to take into account curved lat / lon CROCODILE-CESM/regional-mom6#22 and (Fix tides on angled boundaries, consolidate regridded of velocity and tracers, add proper body tide parameters, add MOM6's angle calculation. CROCODILE-CESM/regional-mom6#33)

Outstanding Questions:

1. (Solved!) Logging was implemented in the two new modules of RM6 (regridding.py and rotation.py). Do we want to switch over RM6 to use python logging instead of print statements? Currently, we would prefer that but open it up to opinions.
2. (Solved!) A while back, we all briefly discussed that this should be version 0.8 instead of 1. Any opinions or insight?
3. (Solved!) The docker image is currently CROCODILE's image, if we want to update RM6's image, it's a three step process by someone with write permissions & docker downloaded

[ashjbarnes]

Tomorrow I'll check one more time that with these new changes, the example notebook produces a runnable model on our system. If that works, I'm happy for the merge! Then we can make other quality of life changes and further updates to docs or whatever in further PRs.

[navidcy]

I'd really like to merge this with the documentation. If we don't, it will never happen...
Doesn't have to be perfect, but it can't be incomplete.

For example, I'd push for some clarity in the section we are describing the rotation algorithm; I don't understand what's explained there.

I would also like to see the test run and pass before we merge. At the moment they don't do anything? I see this for every commit:

and the test keep hanging, never run?

[navidcy]

Also before we merge it's nice if we run the notebooks so that the figures appear. I can't do that because I never managed to set up the docker. Could somebody else do that?

[ashjbarnes]

Also before we merge it's nice if we run the notebooks so that the figures appear. I can't do that because I never managed to set up the docker. Could somebody else do that?

I've already done that for the reanalysis forced example. I cleared all the other outputs too so there's not the other mess cluttering up the notebooks, only the plots. Could you please do this for the BYO grid one @helenmacdonald since you've already got it running somewhere?

[helenmacdonald]

Could you please do this for the BYO grid one @helenmacdonald since you've already got it running somewhere?

I also cleared the example but I am happy to redo and leave to output in. Is it Ok if I leave it to Friday to do this?

[navidcy]

Is it Ok if I leave it to Friday to do this?

Of course! Thanks!

[ashjbarnes]

I'd really like to merge this with the documentation. If we don't, it will never happen... Doesn't have to be perfect, but it can't be incomplete.

For example, I'd push for some clarity in the section we are describing the rotation algorithm; I don't understand what's explained there.

Which bits still need clarity with the updated documentation? I don't think it's that relevant to the regular user as long as it works. We don't spell out most of the algorithms we're using in the main docs, so why is the rotation algorithm different? The interested user can look at the code and docstrings (which are great btw! ), and the relevant bit of the MOM6 documentation, which @manishvenu has already linked in the docstrings.

I would also like to see the test run and pass before we merge. At the moment they don't do anything? I see this for every commit:

Yeah not sure what's going on! They all passed yesterday with my changes but are hanging today. Nothing structural has changed since they passed yesterday though

[navidcy]

I'd really like to merge this with the documentation. If we don't, it will never happen... Doesn't have to be perfect, but it can't be incomplete.
For example, I'd push for some clarity in the section we are describing the rotation algorithm; I don't understand what's explained there.

Which bits still need clarity with the updated documentation? I don't think it's that relevant to the regular user as long as it works. We don't spell out most of the algorithms we're using in the main docs, so why is the rotation algorithm different? The interested user can look at the code and docstrings (which are great btw! ), and the relevant bit of the MOM6 documentation, which @manishvenu has already linked in the docstrings.

I don't understand the steps described here:

https://regional-mom6--193.org.readthedocs.build/en/193/angle_calculation.html#boundary-rotation-algorithm
I made a comment on step 1.

If we include it, then let's make it so that people understand.

I would also like to see the test run and pass before we merge. At the moment they don't do anything? I see this for every commit:

Yeah not sure what's going on! They all passed yesterday with my changes but are hanging today. Nothing structural has changed since they passed yesterday though

Which commit of yours yesterday had all the tests working? We may able to figure out why if that was the case. But as far as I understand, the last time I saw the test jobs starting and passing is 0523f1d. Perhaps I'm wrong.

[manishvenu]

I would also like to see the test run and pass before we merge. At the moment they don't do anything? I see this for every commit:

Yeah not sure what's going on! They all passed yesterday with my changes but are hanging today. Nothing structural has changed since they passed yesterday though

Which commit of yours yesterday had all the tests working? We may able to figure out why if that was the case. But as far as I understand, the last time I saw the test jobs starting and passing is 0523f1d. Perhaps I'm wrong.

These pending checks on this PR have been around the whole time for me. The tests have been running on CROCODILE's fork so not sure what's up here.

Thanks,
Manish V

[navidcy]

They were running up to some point; last time I've seen them run was for commit 0523f1d.

[manishvenu]

Fixed!

[navidcy]

G R E A T!

[navidcy]

@manishvenu we can perhaps zoom on Fri and sort out the explanation of the steps at https://regional-mom6--193.org.readthedocs.build/en/193/angle_calculation.html#boundary-rotation-algorithm ?

I'm in California timezone until Fri but busy up to Thu...

We can also always continue chat via GitHub.

[manishvenu]

@manishvenu we can perhaps zoom on Fri and sort out the explanation of the steps at https://regional-mom6--193.org.readthedocs.build/en/193/angle_calculation.html#boundary-rotation-algorithm ?

I'm in California timezone until Fri but busy up to Thu...

We can also always continue chat via GitHub.

Yes, we can definitely zoom to wrap up this PR. Feel free to email me to coordinate! I'm happy to do Friday, though it may also be easier to do sometime next week if anyone in the Australian timezone wants to join.

[navidcy]

@manishvenu we can perhaps zoom on Fri and sort out the explanation of the steps at https://regional-mom6--193.org.readthedocs.build/en/193/angle_calculation.html#boundary-rotation-algorithm ?
I'm in California timezone until Fri but busy up to Thu...
We can also always continue chat via GitHub.

Yes, we can definitely zoom to wrap up this PR. Feel free to email me to coordinate! I'm happy to do Friday, though it may also be easier to do sometime next week if anyone in the Australian timezone wants to join.

Let's play it by ear on Fri...
Next week is super busy for me :(

[manishvenu]

@manishvenu we can perhaps zoom on Fri and sort out the explanation of the steps at https://regional-mom6--193.org.readthedocs.build/en/193/angle_calculation.html#boundary-rotation-algorithm ?
I'm in California timezone until Fri but busy up to Thu...
We can also always continue chat via GitHub.

Yes, we can definitely zoom to wrap up this PR. Feel free to email me to coordinate! I'm happy to do Friday, though it may also be easier to do sometime next week if anyone in the Australian timezone wants to join.

Let's play it by ear on Fri... Next week is super busy for me :(

Sounds good.

[ashjbarnes]

I've fixed the MOM_layout not writing issue, and tried testing the example notebook again. However, it crashes with the 'eta has dropped below bathy' issue even with a really short timestep. It's suggesting a huge instability too, with the sea surface passing through the centre of the planet and making it all the way to Neptune. The instabilities are located at the boundaries too.

Could you confirm whether MOM6 runs the demo successfully out of the box on Derecho @manishvenu ? If not, then some of the changes since October might have caused this to break.

Last I'd checked in October, the rectangular Tasmania demo worked fine on the NCAR computers, but the curved boundary with tides on did not because of a bug I introduced but which you've subsequently fixed. If this is only an issue on our machine then that would make the diagnosis a lot easier!

[manishvenu]

I've fixed the MOM_layout not writing issue, and tried testing the example notebook again. However, it crashes with the 'eta has dropped below bathy' issue even with a really short timestep. It's suggesting a huge instability too, with the sea surface passing through the centre of the planet and making it all the way to Neptune. The instabilities are located at the boundaries too.

Could you confirm whether MOM6 runs the demo successfully out of the box on Derecho @manishvenu ? If not, then some of the changes since October might have caused this to break.

Last I'd checked in October, the rectangular Tasmania demo worked fine on the NCAR computers, but the curved boundary with tides on did not because of a bug I introduced but which you've subsequently fixed. If this is only an issue on our machine then that would make the diagnosis a lot easier!

Hey Ashley,

The tidal bug did have pretty much that error (I think). I can try to run the reanalysis_forced demo -> We've shifted away from using a few of the functions however, CrocoDash isn't using setup_run_directory and such, mostly just the forcing functions. That should still apply to this error (it's boundary related iirc??).

For the tidal bug, it's because the corners of the boundaries had NaNs in them, but that should be fixed now.

Did you try turning off the tides?

Thanks,
Manish V.

[ashjbarnes]

I've fixed the MOM_layout not writing issue, and tried testing the example notebook again. However, it crashes with the 'eta has dropped below bathy' issue even with a really short timestep. It's suggesting a huge instability too, with the sea surface passing through the centre of the planet and making it all the way to Neptune. The instabilities are located at the boundaries too.
Could you confirm whether MOM6 runs the demo successfully out of the box on Derecho @manishvenu ? If not, then some of the changes since October might have caused this to break.
Last I'd checked in October, the rectangular Tasmania demo worked fine on the NCAR computers, but the curved boundary with tides on did not because of a bug I introduced but which you've subsequently fixed. If this is only an issue on our machine then that would make the diagnosis a lot easier!

Hey Ashley,

The tidal bug did have pretty much that error (I think). I can try to run the reanalysis_forced demo -> We've shifted away from using a few of the functions however, CrocoDash isn't using setup_run_directory and such, mostly just the forcing functions. That should still apply to this error (it's boundary related iirc??).

For the tidal bug, it's because the corners of the boundaries had NaNs in them, but that should be fixed now.

Did you try turning off the tides?

Thanks, Manish V.

Thanks yeah if you can check whether it works on your setup we’ll know at least that it’s not the forcing and related to the differences to our setups. If there’s a problem for you though then the forcing files might still have an issue!

[ashjbarnes]

But the tidal but didn’t used to be an issue on the non rotated Tasmania example right? I remember first getting it on the curved NWA domain

so something else might have happened in the meantime which messed up the non rotated simple case. I’ll try without tides though

[manishvenu]

But the tidal but didn’t used to be an issue on the non rotated Tasmania example right? I remember first getting it on the curved NWA domain

so something else might have happened in the meantime which messed up the non rotated simple case. I’ll try without tides though

Yeah, it shouldn't be an issue of the non rotated Tasmania because don't get NaNs there. I haven't tested this but it probably would fail if there were NaNs in the corner. Probably isn't that error tho! I'll see if it runs on our computer tmrw.

We got to figure out that way to share cases across computers at some point:)