Added a recipe for along-isobath averaging

[taimoorsohail]

Hi, I am posting a new Contributed Example which averages properties along an isobath - useful for Antarctic margins analysis. Let me know if you have thoughts!

closes #397

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[navidcy]

Seems that you added two files?

[taimoorsohail]

Yeah sorry - I accidentally included a random change I made to the ContributedExamples/Cross-slope_section_potrho_cc.ipynb file - please delete if possible before pushing the new file :)

[navidcy]

@taimoorsohail is there a reason you wanted to make this a "Contributed Example"?

We decided to just have "Examples" and they should all be documented and in good shape; see #407.

[taimoorsohail]

Ah sorry, I didn't know that we had moved on! Yes happy to add it to "Examples" if everyone is happy with the level of documentation and commenting.

[navidcy]

if everyone is happy with the level of documentation and commenting.

We'll let the review process determine that. :)

[adele-morrison]

Hey @taimoorsohail, out of interest, is it necessary to use st_edges_ocean for the bins? Would it be possible in theory to use a more even binning (say spaced every 50m in isobath depth) and then use ht/hu for computing the histogram?

[adele-morrison]

Currently the time average is done just before plotting, but the time information is not used. Perhaps time averaging and loading earlier could make it run faster?

[adele-morrison]

I'm wondering if we can get away with only one set of plots? Since they are identical except for the x-axis labels. Would having different example axis labels on different sub-panels be too confusing?

Reply via ReviewNB

[taimoorsohail]

Yes that is definitely possible. I tend to maximise all possible combinations when writing out an example, just to account for more possible use-cases. But yes, In theory we could just do a 1x3 plot, with each subplot showing the three different x-axes (i.e., ht_bins, normed_area and pseudo-lat) for a single variable (e.g. temperature). I think the different x-axis labels might be confusing but yes, also a possibility.

[taimoorsohail]

I have added an additional x-label (pseudo-lat) to reduce the number of figures. Hope this works!

[adele-morrison]

Also, if you edit your top post to add:
closes https://github.com/COSIMA/cosima-recipes/issues/397,
it will automatically close the issue when this is merged.

[taimoorsohail]

Hey @taimoorsohail, out of interest, is it necessary to use st_edges_ocean for the bins? Would it be possible in theory to use a more even binning (say spaced every 50m in isobath depth) and then use ht/hu for computing the histogram?

Hey @adele-morrison! In theory, it should be OK. I had some issues masking ht/hu with st_edges_ocean for the bins, as ht and hu cover a larger range of depths than the bottom depth of the variables (see https://forum.access-hive.org.au/t/difference-between-bottom-bathymetry-variable-ht-and-actual-data-in-access-om2-01/2221). This meant I was getting more "jagged" averages which only really affected the "prettiness" of the visualisation. I didn't try reducing the bin spacing though.

Currently the time average is done just before plotting, but the time information is not used. Perhaps time averaging and loading earlier could make it run faster?

Won't time-averaging after binning account for the temporal variability within each bin? Whereas time-averaging before binning would get rid of that? I'm not sure...

[navidcy]

this cell looks like the same as the cell above?

then why not making it a function and just call it twice with different inputs?

Reply via ReviewNB

[adele-morrison]

Hey @taimoorsohail how is this one going? This would be super useful code for a bunch of people, so would be great to get it included!

[taimoorsohail]

Hi @adele-morrison thanks for the prod! I think I was confused how to handle varying vertical grids in this code. At the moment, I bin using a new bottom depth variable I create based on the last depth where T/S/rho data is available - this aligns with st_edges_ocean. In reality, the ht/hu variable contains the 'actual' bottom depth. It would be more accurate to use this value, but would involve stretching/shrinking the T/S/rho profiles, right? Anyway, your thoughts are welcome...

[navidcy]

I don't have strong opinions on the vertical grid question. But even leaving it as is and opening an issue for further discussion after the PR is merge is totally OK.

I do see a lot of plots though and a comment to reduce them. Could we simplify the notebook @taimoorsohail? I'm happy to have a look after -- let me know.

Just mentioning that this PR should also include an entry for you @taimoorsohail in the .zenodo.json at https://github.com/taimoorsohail/cosima-recipes/blob/main/.zenodo.json so that your name appears in all future releases in the Zenodo entry after this PR is merged.

[taimoorsohail]

Thanks again both for prodding me on this. I have cleaned up the file and reduced the number of figures as per Adele's suggestion! Please feel free to review @navidcy. I will add an issue about the adaptive vertical grid for the next hackathon :)

[navidcy]

let's just change the "This code" in the beginning of the notebook (first 2 sentences) to "This recipe"?

[navidcy]

I actually can't run the notebook :(
How did you run it??
I get the error you see (I run it and pushed it with the error)

[taimoorsohail]

OK I have pushed a fix - does this work?

[navidcy]

OK I have pushed a fix - does this work?

Still I hit a wall opening the files... I pushed the file with the errors I get.

But for you it works??? Can you ensure that before you push you restart the kernel and run all cells sequentially (that's what I'm trying to do). Also could you tell me what resources you have on the notebook when you run it?
[I'm using gadi_jupyter with ncpus=48, i.e. a whole node at the normal queue.]

[taimoorsohail]

I've pushed the error I am getting when running the code. I think it has to do with accessing the variables using COSIMA cookbook - there may be a bug I don't know about? Any ideas @anton-seaice @dougiesquire or others at NRI?

[navidcy]

Yeah... I've been confused by this myself... any help is welcomed!

[navidcy]

Are we sure it's the Cookbook?

Have you tried with intake @taimoorsohail?

[anton-seaice]

That error normally means you need to set threads_per_worker = 1 as an argument in client = Client()

There is an long term bug in the netcdf libraries that causes this.

See #409

[navidcy]

Thanks! I'll try that!

[navidcy]

great!

[navidcy]

Thanks @taimoorsohail for the recipe! Welcome to the cookbook!

I will add an issue about the adaptive vertical grid for the next hackathon :)

Please remember to raise that issue mentioned above. Not necessarily "for the next hackathon" though ;) The issue can be taken up from anybody at anytime, even today!

[navidcy]

Just a small note: in 7defbf0 also changed a bunch of code comments to markdown text. I find that markdown text with formatting etc is much more user friendly to the eye than Python # comments within the code.