Dev healpix

[sortega87]

This PR adds the ability to compute the wave spectra on healpix grids.

The same interfaces are kept and 2 additional arguments can now be used to specify the grid type.

grid_type = "healpix"
grid_dict = {'nside': 32, 'nest': True, 'minmax_lat': 15}
window_length = "92D"
overlap_length = "60D"
data_frequency = "12h"
symmetric_spectrum = get_power_spectrum(
    dataset,
    "symmetric",
    data_frequency,
    window_length,
    overlap_length,
    grid_type=grid_type,
    grid_dict=grid_dict,
)

[sortega87]

Hi @HF7weatherman! Would be great to know your view on this PR.

[HF7weatherman]

Hi @sortega87! I'll try to have a look on this by the end of this week ;)

[sortega87]

Hi @sortega87! I'll try to have a look on this by the end of this week ;)

Sounds good! Thanks!

[HF7weatherman]

Hey @sortega87! Thanks for implementing the HEALPix support of pywk, I think this is a great addition. Overall, the code looks fine, and I only have minor additions which you can find in my comments.
However, I have a question about the longitude interpolation, which is probably related to my lack of detailed knowledge of the HEALPix grid: Is the longitude of each pixel in every second ring the same? Shouldn't it decrease away from the equator as every HEALPix pixel has the same area? I assume that you studied this in detail and thus are probably right, but I still struggle to understand this. However, we should discuss this in person.

[HF7weatherman]

In my opinion, there's no need to define use **grid_dict here. If you follow my advice in pywk99/grid/grid.py, then grid_dict would still be passed as a dictionary, so using ** to gather keyword arguments into a dictionary here is not necessary.

[sortega87]

Thanks for the detailed review @HF7weatherman! Regarding the longitude interpolation, as you mentioned, I think it is indeed the case that the longitude of each pixel in every second ring the same. The following figure, taken from here, gives an idea why:

Other two important properties of the healpix grid are the following:

• An equatorial zone is defined as the region where $\cos{\theta} \le 2/3$
• On the equatorial zone the number of grids is 4*nside

Note: I tested this in code using healpy.

[HF7weatherman]

Hi @sortega87! Thanks for clarifying this. So this means that the number of longitudes is the same for each latitude ring and, strictly speaking, there wouldn't be the need to interpolate to the same longitudes everywhere before doing the spectral transformation in the zonal direction. In principle, you could do the spectral transformation on each latitude ring irrespective of longitudes as long as the total amount of longitudes is the same on each latitude ring since the exact longitudes no longer matter in wavenumber space. Therefore, I wondered why you did the interpolation to the same longitudes? Is it for consistency and efficiency reasons?

[sortega87]

Hi @HF7weatherman! Thanks! Yes, that is correct, strictly speaking the interpolation is not needed yet, as you mention, it is made for consistency and efficiency reasons. The starting point of the "get_spectrum" and "get_spectrum" functions is currently a "lat-lon" dataarray, thus, by adding a single grid transformation step, we are able to support Healpix without modification of the downstream functions. I believe this makes for a simple implementation that is easy to reason about.

[HF7weatherman]

Hi @sortega87! Alright, that makes sense and I agree that this is the simplest possible implementation so I would also keep it as is. I also think that the interpolation error is potentially small/negligible, so the results should be more or less independent of it. Nevertheless, we should keep this in mind and might implement this later.

Therefore, I would approve merging and closing this PR now.

[sortega87]

Hi @sortega87! Alright, that makes sense and I agree that this is the simplest possible implementation so I would also keep it as is. I also think that the interpolation error is potentially small/negligible, so the results should be more or less independent of it. Nevertheless, we should keep this in mind and might implement this later.

Therefore, I would approve merging and closing this PR now.

Thanks Henning! I agree. Merging now to the main branch.