search

COSIMA / cosima-recipes

A cookbook 📒 of recipes (i.e., examples) for analysing ocean and sea ice model output. 👩🏽‍🍳🌊👨🏻‍🍳
https://cosima-recipes.readthedocs.io
Apache License 2.0
46 stars 66 forks source link

Small contour ordering bug in `Cross-contour_transport.ipynb` #383

Open claireyung opened 5 months ago

claireyung commented 5 months ago

The cross-contour transport currently has a bug (even when run with the right kernel -note this is an issue too https://github.com/COSIMA/cosima-recipes/issues/327) where the ordering of the contour transports at corners can be slightly wrong. My understanding is that the correct transports are taken across the contour, but when put in the array they can be a slightly different order to the order expected by following the contour (monotonically increasing contour_index). More details on the ordering issue are here: https://github.com/COSIMA/cosima-recipes/issues/291#issuecomment-1747990241

This is currently a warning ("Alert") in the notebook. It shouldn't affect cumulative sums of transport or transport calculations with smoothing or averaging, but it might make a difference if you're looking very locally at the transport values.

It would require some changes to the algorithm that determines where cells are positioned relative to each other. I might be missing something but this seems quite tricky as there are there are a lot of possible configurations of the contour when diagonal connections are allowed.

navidcy commented 5 months ago

Thanks for opening this!

If we don't know how to resolve this for now, let's at least add a link to this issue in the notebook's intro so that people who use it don't miss it.

adele-morrison commented 5 months ago

Options for resolving (apart from a straight fix) could be:

  1. Go back to using contours with non-diagonal connections.
  2. Investigate whether we can replace this notebook with xwmb functions if that’s better.
navidcy commented 5 months ago

What's xwmb? Could you elaborate please or point to their docs?

navidcy commented 5 months ago

(I tried to google and got weird results for Vietnamese lottery tickets...)

adele-morrison commented 5 months ago

https://forum.access-hive.org.au/t/new-python-package-for-water-mass-transformation-calculation/

Henri will be giving a talk at the COSIMA meeting in 2 weeks about it.

navidcy commented 5 months ago

OK gotcha! xwmb! Seems sweet!

Some minor issues with using that package at the moment: I noticed that the package is currently not installable via conda and therefore can't be added in the conda analysis environments we use for analysing COSIMA outputs on NCI. Also the package does not have any tests and this makes it amenable to silent breaking changes as package development continues.

We can help the xwmb team to create some meaningful unit tests and also bit more elaborate tests for the package!

hdrake commented 5 months ago

xwmb is overkill for this. I think you just need the more concise sectionate (the relevant dependency of xwmb), which I do have unit tests for but is not yet available for install via conda. My other package regionate essentially allows you to perform the discrete divergence theorem to convert between surface flux integrals (e.g. cross-contour transport) and flux divergences over a volume (e.g. volume-weighted sum of budget terms).

I'll try to register the package on conda ASAP (thanks for the nudge @navidcy) but it might take me a while to get it right.

Any and all contributions from the COSIMA community are welcome!