xarray-contrib / xwrf

A lightweight interface for working with the Weather Research and Forecasting (WRF) model output in Xarray.
https://xwrf.readthedocs.io/
Apache License 2.0
59 stars 16 forks source link

Add destaggering functionality #93

Closed jthielen closed 2 years ago

jthielen commented 2 years ago

Change Summary

This is an updated version of #37 to handle de-staggering in all three dimensions and in a (hopefully, still needs to be tested) dask-friendly way.

Currently a draft PR to get eyes on my implementation right away; will be working on tests, documentation updates, and cleaning up the type hints later today.

Related issue number

Closes #35

Checklist

lpilz commented 2 years ago

I think it would be nice to support destaggering multiple dimensions at once. Couldn't we just recursively call _destagger_variable for a single dim?

jthielen commented 2 years ago

I think it would be nice to support destaggering multiple dimensions at once. Couldn't we just recursively call _destagger_variable for a single dim?

It would require a little bit of refactoring of the automatic staggered dim handling and a bit more internal complexity on the accessor methods, but if this is a use case we wanted to support, I could certainly make those changes! That being said, I don't believe I've seen this situation occur in practice. Do you have a sample dataset with such a variable with multiple staggered dimensions?

lpilz commented 2 years ago

You're right, I was originally thinking about maybe finding some variables staggered in z and one of the x or y dimensions, but it seems that this doesn't occur (at least in output data). So nevermind! :D

jthielen commented 2 years ago

While this doesn't add direct tests with dask (deferring to https://github.com/xarray-contrib/xwrf/issues/60 and a later release...perhaps v0.1?), with some some cursory exploration it looks like everything works out for https://github.com/xarray-contrib/xwrf/issues/52#issuecomment-1248425926.

jthielen commented 2 years ago

Thank you for the feedback @lpilz and @andersy005! I believe all the issues raised have been addressed. With an approving review from someone on @xarray-contrib/xwrf-devs, this should be good to merge.

lpilz commented 2 years ago

Awesome work :rocket: Thanks a lot!