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

Explicitly handle the input argument ranges from Aerobulk #29

Closed paigem closed 2 years ago

paigem commented 2 years ago

Per @TomNicholas's comment on the xarray wrapper PR, we should think a bit more about the allowed value ranges for input arguments in aerobulk-python.

We could write a function to check this in our xarray wrapper. The ranges are defined in this Fortran file in Aerobulk (line 136).

We will also address this comment about RST blocks.

This issue is to note that these comment will be addressed, but after an initial merge of the xarray wrapper PR to help keep progress going on the project.

paigem commented 2 years ago

@TomNicholas also pointed out in this comment that we should address the function's expectation of units separate from the min and max values.

I suggest we:

jbusecke commented 2 years ago

From a practical standpoint a 'range_checker' function would be awesome! I think we need to gain some clarity on #31 and then we could have a stand-alone checking function or an optional check within the xarray wrapper, that users could activate during development stage and deactivate when they run large production calculations?

jbusecke commented 2 years ago

I propose that we implement a range_check=True kwarg in the xarray wrappers. This would cost some performance (we should quantify how much using snakeviz), but would basically eliminate the need for #11 I think .

The proposed workflow would be: A user starts experimenting with the data, the range check is on by default and masks out invalid values (while displaying a warning), and then the user can investigate where these are coming from and mask/replace them on their side. For production runs they can then set range_check=False to get the full performance.