knaughten / mitgcm_python

Python scripts designed for my Weddell Sea and Amundsen Sea configurations of MITgcm.
25 stars 17 forks source link

Confusion about argument order #13

Closed DaniJonesOcean closed 3 years ago

DaniJonesOcean commented 3 years ago

Hi @knaughten. This is more of a question than an issue. I'm confused about the order of a couple of the arguments to interp_reg. My question may very well end up being a basic python one, but I'd still like to understand. I'll step through what I noticed below:

First, we call the sose_ics function:

from mitgcm_python.ics_obcs import *
sose_ics(grid_path='../grid/', sose_dir='/data/oceans_input/raw_input_data/SOSE_monthly_climatology/', nc_out='initial_conditions/sose_ics.nc', output_dir='initial_conditions/', constant_t=-1.9, constant_s=34.4, split=180)

This function calls get_fill_mask:

fill, model_cavity = get_fill_mask(sose_grid, model_grid)

This function calls interp_reg, using this call if missing_cavities==True:

model_cavity = np.ceil(interp_reg(model_grid, source_grid, xy_to_xyz(model_grid.ice_mask, model_grid), fill_value=0)).astype(bool)

Check out the definition of interp_reg, which has source_grid as a first argument.

def interp_reg (source_grid, target_grid, source_data, dim=3, gtype='t', fill_value=-9999):

I'm confused as to why the arguments in the definition are source_grid, target_grid, while the arguments in the function call are model_grid, source_grid.

I know that python has keyword arguments as well, but don't you have to call them with a keyword and an equals sign? As far as I understand, source_grid and target_grid are positional arguments, but they seem to be reversed between the function call and the function definition. Thanks in advance for any clarification you can provide!

knaughten commented 3 years ago

The reversal is correct (although the names are confusing - maybe we should rename source_grid in get_fill_mask!) This call to interp_reg is interpolating the cavity mask from the MITgcm grid to the SOSE grid, using np.ceil to include any points which are "partial" cavities. In other words, it's a rare case where we're interpolating from the model back to the SOSE grid (usually it's the other direction), but interp_reg still works fine for this.

DaniJonesOcean commented 3 years ago

Okay, that makes sense, thanks. : )

I'll close this for now, but we could make a different issue for renaming at some point.