Version 1.0: North Atlantic Update - Tides, Curved Boundaries, Config, and More!😀🤯🥳 #193
Conversation
…CESM when parsing
Override min depth
Changes to make compatible with CESM
having mask table and layout no longer required
Fix layout requirements
fix typo in find replace
Replace minimum layers with min depth
…nt num After talking with Ashley, the find_MOM6_orientation will likely be removed
The function names for rectangular and sinmple boundaries were changed because the tides are a kind of boundary function, the old names now give a warning and call the correct function. GFDL had rough horizontal subsetting for the tpxo dataset (probably for efficiency?), implemented in setup_tides_rectangular_boundaries.
Collapsed the *_tidal_dims functions into the encode_tides function in segment, and add citing documentation to the docstrings for now.
…l-mom6 into croc_with_req_changes
| rotational_method: RotationMethod, hgrid: xr.Dataset, orientation=None | ||
| ): | ||
| """ | ||
| Returns the rotation angle - THIS IS ALWAYS BASED ON THE ASSUMPTION OF DEGREES - based on the rotational method and provided hgrid, if orientation & coords are provided, it will assume the boundary is requested. |
There was a problem hiding this comment.
are the capitals here note to self? should we remove it? Or at least make it lowercase?
There was a problem hiding this comment.
Making it lower case is probably best!
| ```bash | ||
| docker pull ghcr.io/cosima/regional-test-env:updated | ||
| ``` |
There was a problem hiding this comment.
@manishvenu I install docker and run this and got an error!
(base) navid:croco-regional-mom6/ (main) $ docker pull ghcr.io/cosima/regional-test-env:updated [17:26:51]
Cannot connect to the Docker daemon at unix:///Users/navid/.docker/run/docker.sock. Is the docker daemon running?
The instructions need to be expanded to be self-sufficient... I'm sorry I don't have any idea how to work with docker images to help here.
There was a problem hiding this comment.
It looks like you don't have docker engine running. You may need to start the service first before running docker commands if you want to test this out.
I think taking your recommendation of a link to the Docker documentation would be best (http://docs.docker.com/get-started/get-docker/). This error is a great example, it's a pretty common error, and any future devs who are trying to learn Docker and add to testing can figure this out with docs & online sources. Docker is a pretty big software with a lot of documentation . It's hard enough learning Docker without providing our own attempt at running Docker. I can add the link to the docs when I get back to this.
Lmk how you feel about that, happy to discuss further.
There was a problem hiding this comment.
Right! We don't need to include all the instructions. Just point the devs to somewhere they can get info? Perhaps we can say something along the lines of "As soon as you have a docker set up and running and X, Y, Z is OK then do this..." and we continue with how the docs are atm?
What I'm suggesting is 1-2 sentences before of what is currently in the docs atm, not much more.
There was a problem hiding this comment.
Yeah, sounds like we're in agreement. I'm not sure what X, Y, & Z would be but I can come up with some checks probably!
There was a problem hiding this comment.
I think our docker setup is fairly standard. There are plenty of resources out there for people to learn docker basics like the one Manish mentioned which is all you need to get going with this so probably no need to go overboard on what we include in our docs!
There was a problem hiding this comment.
Perhaps just mention the website and tell people have a docker setup or something. Don’t worry for the X, Y, Z. It was just an example. If nothing comes to mind straight away then it’s not the way forward.
Like what would you advise me to do before starting to follow the instructions in the docs already. I still don’t know what to do.
There was a problem hiding this comment.
@navidcy I would read the documentation to get docker installed and make sure you can run docker commands on the command line (hah! There's a check!)
I'll add this to the docs!
|
seems like we skip a lot of cells from the tests via |
Tidal data isn't directly public: https://www.tpxo.net/. Putting it on our docker image is probably not a good idea. (Check the comment in that cell) We do have test functions for tides in the actual test files. I don't think the point of the demos is to test our functions, and it's probably not worth the overhead to either add fake data to a docker image or make the image not public - we're losing pretty much no coverage. |
Fair! Thanks for elaborating on this! |
|
@ashjbarnes Thanks!
|
* Change Copernicusmarine version & remove xarray limiter * Put back Xarray just in case even though it's probably no longer relevant * Remove Xarray Dependency
I think either is fine! |
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:
Minor Features:
Breaking Changes (for backwards compatibility):
Documentation Changes:
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:
Outstanding Questions:
regridding.pyandrotation.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.