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

Intake conversion Figs14-20-23-24-PlotWOCETransects #346

Merged
merged 8 commits into from
Aug 30, 2024

Conversation

rbeucher
Copy link
Contributor

@rbeucher rbeucher commented Jun 6, 2024

Following the discussion in issue #313, we propose converting the recipes to use Intake, given that the Cookbook is no longer supported and the ACCESS-NRI Intake catalog is now available.

A few months ago, @max-anu began working on this transition. This pull request contains the changes @max-anu made to the notebook specified in the title.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@julia-neme
Copy link
Collaborator

I'm almost done converting this notebook to intake, but I'm struggling loading the 0.1deg cross sections. I had a chat with @anton-seaice , tried a couple of things, but my ARE keeps crashing when it's 0.1deg's turn. Can anyone take a look and try?

@anton-seaice
Copy link
Collaborator

Are these the best experiments to use for this notebook?

'1deg_jra55_iaf_omip2_cycle1'
'025deg_jra55_iaf_omip2_cycle1'
'01deg_jra55v140_iaf'

Or do we want later cycles (for example?)

The original notebook used these:

1deg_jra55v13_iaf_spinup1_B1
025deg_jra55v13_iaf_gmredi6
01deg_jra55v13_iaf

Ping @aekiss ?

Copy link

review-notebook-app bot commented Aug 26, 2024

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-26T05:18:40Z
----------------------------------------------------------------

Line #13.        cat_subset = catalog.search(name = expt)

You can get rid of this line and just use

catalog[expt] in the line below it


Copy link

review-notebook-app bot commented Aug 26, 2024

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-26T05:18:41Z
----------------------------------------------------------------

Line #20.        ds = xr.merge(ds.values())

i changed this to

xr.merge(ds.values()).chunk({'time':'auto'])

which seemed to help, but I am sure there is a better solution still


Copy link

review-notebook-app bot commented Aug 26, 2024

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-26T05:18:41Z
----------------------------------------------------------------

Line #35.            model_cross_sections[cs] = ds.sel(xt_ocean = lon, method = 'nearest').sel(yt_ocean = slice(lat[0], lat[1])).load()

@julia-neme : This line is an issue, its trying to load everything into memory at the start, rather than using dask to distribute the work.


julia-neme commented on 2024-08-26T06:36:58Z
----------------------------------------------------------------

So is it better to not load at anytime? I thought it might make it faster for plotting, sp with contourf plots!

Copy link

review-notebook-app bot commented Aug 26, 2024

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-26T05:18:42Z
----------------------------------------------------------------

Line #12.        fig = plt.figure(n+1,figsize=(12,12))

You don't need this n+1, it makes a new figure by default when you call plt.figure, so you can get rid of n totally


Copy link

review-notebook-app bot commented Aug 26, 2024

View / edit / reply to this conversation on ReviewNB

anton-seaice commented on 2024-08-26T05:22:04Z
----------------------------------------------------------------

Line #18.        ).to_dataset_dict()

This line returns too many results:

ie:

set(catalog['01deg_jra55v140_iaf'].search(
        variable = ['temp', 'salt'],
        frequency = '1mon',
    ).df.file_id)

returns:

{'ocean_3d_salt_1_monthly_mean_ym_XXXX_XX',
 'ocean_3d_salt_1_monthly_snap_ym_XXXX_XX',
 'ocean_3d_temp_1_monthly_mean_ym_XXXX_XX',
 'ocean_3d_temp_1_monthly_snap_ym_XXXX_XX'}

You can add a

variable_cell_methods = "time: mean"

to the search


@anton-seaice
Copy link
Collaborator

With the changes in the above comments, the figures generate in about ~15 minutes. I am sure there is a better solution than that, but this notebook is using dated approaches now and could do with a refactor! contourf is always slow though!

Copy link
Collaborator

So is it better to not load at anytime? I thought it might make it faster for plotting, sp with contourf plots!


View entire conversation on ReviewNB

@anton-seaice
Copy link
Collaborator

So is it better to not load at anytime? I thought it might make it faster for plotting, sp with contourf plots!

View entire conversation on ReviewNB

I just saw it and thought it might fill up memory. However with further thought maybe they would be ok. Even though there will be lots of these cross sections they are maybe small enough to fit in memory? I am actually not sure if the contourf will do a distributed calculation anyway (without this load() call) ? Each transect is only used for one plot, so I don't think it would make a speed difference?

Load is probably / possibly useful if the same dataset (which is small enough to fit in memory) is access multiple times in a notebook.

@julia-neme
Copy link
Collaborator

Oki, I'll just wait for confirmation of whether we want the first cycle of these experiments or not (@aekiss , @AndyHoggANU , @adele-morrison ?)

@AndyHoggANU
Copy link
Contributor

My suggestion would be to follow OMIP protocol and use cycle5 from each of the 1° and 0.25°, and cycle4 from the 0.1° (as we've only done 4 cycles of that one). Sound sensible?

@julia-neme
Copy link
Collaborator

Updated and working! I've also added a conversion of the obs potential temperature to the model's conservative one. OG script was comparing without converting.

@navidcy
Copy link
Collaborator

navidcy commented Aug 28, 2024

@julia-neme you are a hero

Copy link
Collaborator

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

lgtm!

@navidcy navidcy merged commit 4c039e9 into main Aug 30, 2024
3 checks passed
@navidcy navidcy deleted the INTAKE_Figs14-20-23-24-PlotWOCETransects branch August 30, 2024 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants