ESCOMP / CISM

Community Ice Sheet Model
GNU Lesser General Public License v3.0
6 stars 11 forks source link

Glad's is_in_active_grid is slightly inconsistent with logic elsewhere in CISM #41

Open billsacks opened 2 years ago

billsacks commented 2 years ago

This is a spin-off from #39 . I noticed that the conditional that determines the icemask in is_in_active_grid checks usrf > 0. But apparently some code elsewhere in CISM treats points with topg exactly equal to 0 as land. So, for consistency, is_in_active_grid should also consider these points with topg exactly equal to 0 to be land, and thus within the ice mask.

As discussed in #39 , this inconsistency is a problem for conservation, so it should definitely be fixed. However, it only impacts a small number of grid cells. As also discussed in #39 , changing this conditional to usrf >= 0 doesn't work, because ocean grid cells have usrf == 0, so that change led all points in the CISM domain to be considered to be within the ice mask. @whlipscomb suggested a possible change like usrf > 0 .or. topg >= 0, but we want to check to make sure that really is the correct conditional, consistent with what is used elsewhere. Ideally, we would use an existing mask variable that is consistent with what is used elsewhere.

billsacks commented 2 years ago

After changing this, a good sanity check that it is correct is: when running the CISM standard testing (aux_glc tests in the CISM-wrapper test suite), answers should change, but only (initially) in a small number of grid cells: those with topg exactly equal to 0.

One way to confirm that we aren't getting inadvertent changes elsewhere could be: create a temporary input file where grid cells with topg == 0 are replaced with topg = 1. (e.g., ncap2 -s 'where(topg == 0.) topg = 1.;' greenland_4km_epsg3413_c171126.nc greenland_4km_epsg3413_c171126_topgNonZero.nc.) Confirm that this only changes a small number of grid cells in the input file. Then run both the baseline and new code with this modified file (e.g., in test SMS_Ly3.f10_f10_mg37.I1850Clm50SpG.cheyenne_intel), and confirm that they are bit-for-bit. This will demonstrate that any answer changes that arise when using the actual input file come from just this small set of grid cells with topg == 0.

whlipscomb commented 2 years ago

@billsacks – I like your suggestion with the temporary input file.