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

Xgcm operations #220

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Xgcm operations #220

wants to merge 3 commits into from

Conversation

edoddridge
Copy link
Collaborator

@edoddridge edoddridge commented Jan 24, 2023

First pass at a notebook that pulls in all the metrics required to create an xgcm grid object.

It's all pretty rough and ready at the moment. The MOM6 example at the end is especially rough.

closes #164

@edoddridge edoddridge added 💻 hackathon 2.0 like the 1.0 but better draft labels Jan 24, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@navidcy
Copy link
Collaborator

navidcy commented Jan 24, 2023

I'll have a look!

@navidcy navidcy removed the draft label Jan 25, 2023
@navidcy
Copy link
Collaborator

navidcy commented Jan 26, 2023

Just had a quick look at this -- will look carefully soon.

Seems like we can ditch the Relative vorticity example and keep this one? What do you think?

@review-notebook-app
Copy link

review-notebook-app bot commented Jan 30, 2023

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2023-01-30T01:10:44Z
----------------------------------------------------------------

Line #2.    rel_vort_region.plot()

Change to

rel_vort_region.plot(vmin=-2e-5, vmax=2e-5, cmap='RdBu_r');

otherwise it's like a blank plot.


julia-neme commented on 2023-09-04T04:39:49Z
----------------------------------------------------------------

I would probably delete this map and go straight to the circumpolar plot

@review-notebook-app
Copy link

review-notebook-app bot commented Jan 30, 2023

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2023-01-30T01:10:45Z
----------------------------------------------------------------

Line #1.    foo = xr.open_dataset('/g/data/ik11/outputs/mom6-panan/panant-v2/output090/19980701.ocean_daily.nc')

let's not foo but rather use the cookbook to load some variables?


@review-notebook-app
Copy link

review-notebook-app bot commented Jan 30, 2023

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2023-01-30T01:10:46Z
----------------------------------------------------------------

Line #3.    ds_panant = ds_panant.rename({'latq':'yq'}).rename({'lath':'yh'}).rename({'lonq':'xq'}).rename({'lonh':'xh'})

why do you rename? is it needed? perhaps we should explain? (would cf-xarray be useful here?)


@review-notebook-app
Copy link

review-notebook-app bot commented Jan 30, 2023

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2023-01-30T01:10:47Z
----------------------------------------------------------------

Line #22.    grid = xgcm.Grid(ds_panant, coords={'X':{'center':'xh', 'left':'xq'},

I feel that the grid construction belongs to its own cell with a bit of a remark on how this is a bit different than before making the connection with MOM5 being B-grid while MOM6 C-grid?


@navidcy
Copy link
Collaborator

navidcy commented Jan 30, 2023

I had a quick go and put a few comments. It's a bit rough... but I'm keen to work to polish it.

  • I'm wondering whether we should incorporate pint/cf-xarray practices.
  • Also wondering whether this should become a tutorial.

@edoddridge
Copy link
Collaborator Author

Thanks Navid

I had a quick go and put a few comments. It's a bit rough... but I'm keen to work to polish it.

Absolutely. I didn't spend time polishing it, because I wanted to have a conversation about what we wanted the final result to look like before doing that. (And there were other people working on xgcm things who wanted to look at it, so I pushed it before it was ready for merging.)

I'm wondering whether we should incorporate pint/cf-xarray practices.

Oh definitely. Now that I've learnt about cf-xarray, I'm keen to use it.

Also wondering whether this should become a tutorial.

I think there's scope for it to be a tutorial. Performing grid-aware operations is a sufficiently basic task that that most users will need to do it and xgcm elegantly deals with the intricacies. But, it would be good to have a streamlined process for creating the grid objects before promoting it as the go-to method. Is that currently blocked by #212 ?

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 10, 2023

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2023-02-10T10:16:20Z
----------------------------------------------------------------

Line #1.    sim = '0.1RYF'

Would the tutorial here break if we load 0.25-degree output?


julia-neme commented on 2023-09-04T04:29:43Z
----------------------------------------------------------------

Also, why wouldn't this work with IAF? This could be probably generalised to whatever access experiment we want right? An option could be adding a line that defines the experiment, and explaining that it can be changed to whatever experiment is of interest?

@review-notebook-app
Copy link

review-notebook-app bot commented Feb 10, 2023

View / edit / reply to this conversation on ReviewNB

navidcy commented on 2023-02-10T10:16:21Z
----------------------------------------------------------------

Line #4.    ocean_grid = xr.open_dataset('/g/data/ik11/outputs/access-om2-01/01deg_jra55v13_ryf9091/output000/ocean/ocean_grid.nc')

can we use cosima cookbook to load this?

I don't like hardcoding the path of the .nc.file? how would the users know the path the file their experiment is?

@angus-g?


@navidcy
Copy link
Collaborator

navidcy commented Aug 31, 2023

@edoddridge should we aim to finish this during this hackathon? do you wanna push this or you have something else in mind for Monday's hackathon?

@edoddridge
Copy link
Collaborator Author

edoddridge commented Sep 1, 2023

We definitely should. I'll be in transit for much of Monday :( so I'm not sure I should lead it, but am happy to pick it up when I arrive if nobody else has.

Copy link
Collaborator

Also, why wouldn't this work with IAF? This could be probably generalised to whatever access experiment we want right?


View entire conversation on ReviewNB

Copy link
Collaborator

I would probably delete this map and go straight to the circumpolar plot


View entire conversation on ReviewNB

@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:27Z
----------------------------------------------------------------

I would maybe add a description here with what this notebook does, and maybe divide in sections so that it is easier to browse the notebook. For example: a section that loads necessary variables, a section that creates the grid info, and a section with example calculations


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:28Z
----------------------------------------------------------------

Line #19.    %matplotlib inline

I don't think this is necessary


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:28Z
----------------------------------------------------------------

This is me being ignorant, but what is the difference between this and doing Client()?


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:29Z
----------------------------------------------------------------

Wouldn't it be more efficient to make this selection for U and V before calculating relative vorticity?


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:30Z
----------------------------------------------------------------

The equation is not rendering for me.


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:31Z
----------------------------------------------------------------

As per relative vorticity comment above, might be better to load U selection before calculating and computing?


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:32Z
----------------------------------------------------------------

Could we put the units in Sverdrup?


@review-notebook-app
Copy link

review-notebook-app bot commented Sep 4, 2023

View / edit / reply to this conversation on ReviewNB

julia-neme commented on 2023-09-04T04:44:32Z
----------------------------------------------------------------

Could you make this figure into a circumpolar map like the others?


Copy link
Collaborator

@julia-neme julia-neme left a comment

Choose a reason for hiding this comment

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

as per comments in reviewNB

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

Successfully merging this pull request may close these issues.

A Documented Example on how to use xgcm with cosima output is wanted
4 participants