hainegroup / oceanspy

A Python package to facilitate ocean model data analysis and visualization.
https://oceanspy.readthedocs.io
MIT License
98 stars 32 forks source link

remove unused, untested functions in utils? #299

Closed Mikejmnez closed 1 year ago

Mikejmnez commented 1 year ago

There are, to my knowledge, two unused, untested functions in oceanspy.utils:

  1. find_cs_sn (line 903)
  2. local_to_latlon (line 844)

@MaceKuailv I checked and you wrote them. I think find_cs_sn was written to calculate CS and SN but that was made redundant since cs and sn are provided as output by the model. Is there a reason to keep find_cs_sn? I also remember there were some issues with it but don't remember very well.

The function local_to_latlon, on the other hand, is very relevant. However, aside from having no tests (in test_utils.py), it is actually inconsistent with the staggered C grid. The function is quite short, and basically is:

def local_to_latlon(u, v, cs, sn):
    uu = u * cs - v * sn
    vv = u * sn + v * cs
    return uu, vv

The transformation assumes all u, v, cs and sn are defined at the same point, but they are not. Both CS and SN are defined at center points (Y, X) and thus each need to be interpolated to U and V points.

@MaceKuailv are you using local_to_latlon within oceanspy?

My suggestion:

Have local_to_latlon take the od as argument, so that it can interpolate all of CS and SN along with the respective vector pairs and transform all vector fields at once.

ThomasHaine commented 1 year ago

Sounds good to me. @MaceKuailv what do you think?

MaceKuailv commented 1 year ago

I think this is very reasonable. I think most models can provide CS and SN.

Mikejmnez commented 1 year ago

closed by #300