xgcm / aerobulk-python

A python wrapper for aerobulk (https://github.com/brodeau/aerobulk)
GNU General Public License v3.0
14 stars 4 forks source link

Land masks #31

Closed paigem closed 2 years ago

paigem commented 2 years ago

We may need to better handle land masking. In our hacking session today with @jbusecke and @zoecharlotte47, we found that some variables get through the flux computations with lands correctly masked as 0, while others (e.g. evap) have some weirdness.

We suggest making a MRE (minimal reproducible example) with some sample data to test how aerobulk-python handles NaNs.

rabernat commented 2 years ago

The fortran source code contains a check for land and generation of an internal mask. I would have assumed that would cover these cases. Have you verified that the land mask generation is happening inside aerobulk?

https://github.com/brodeau/aerobulk/blob/7cd85ba8d05fd23eb81a8fdd202f11d9b63fa0b7/src/mod_aerobulk.f90#L104-L124

paigem commented 2 years ago

Thanks @rabernat - we have not yet checked the land mask implementation in aerobulk. Sounds like @jbusecke has looked into this a bit, and it may be a mismatch in the atmosphere variables compared to ocean (since the atmosphere variables don't need land masks!).

paigem commented 2 years ago

@jbusecke and I did a small test today to see what values are allowed in aerobulk. We tested:

We concluded that we need to handle land masking ourselves. From the meeting today, we decided to use @rabernat's idea of flattening the input variables that get passed to aerobulk and removing the land points. This will cover the handling of a mask and will also reduce the computations being carried out by aerobulk since it won't have to compute unphysical quantities over land.

A PR will follow shortly. 🙂