Use of field_lfrac_g in med_phases_prep_glc_mod.F90 seems suspicious

[billsacks]

I should preface this by acknowledging that I may just be mis-understanding things here, and it may be that this code is correct. That said, this looks possibly wrong to me:

Reading through med_phases_prep_glc_mod.F90, the uses of field_lfrac_g look suspicious for two reasons:

(1) It is set here

https://github.com/ESCOMP/CMEPS/blob/ca3a0adebb72fe7a6cc11ea7ee951bab4bfe634b/mediator/med_phases_prep_glc_mod.F90#L1107-L1108

in subroutine med_phases_prep_glc_renormalize_smb, which is only called if smb_renormalize is true; but it is used here

https://github.com/ESCOMP/CMEPS/blob/ca3a0adebb72fe7a6cc11ea7ee951bab4bfe634b/mediator/med_phases_prep_glc_mod.F90#L808-L814

which I believe is done regardless of the value of smb_renormalize

(2) It looks like its setting here:

https://github.com/ESCOMP/CMEPS/blob/ca3a0adebb72fe7a6cc11ea7ee951bab4bfe634b/mediator/med_phases_prep_glc_mod.F90#L1104-L1108

is setting it to the ice fraction from glc, yet its name (lfrac_g) implies that it should be the land fraction on the glc grid.

[mvertens]

@billsacks - Thanks for pointing this out. The issue is the following. In the call to map_field_normalized, field_normdst is calculated as the normalization factor by mapping field_lfrac_l to toglc_frlnd(ns)%field_lfrac_g - but then not used again. toglc_frlnd(ns)%field_lfrac_g is simply memory that is held in place to calculate the normalization factor in the mapping. Later on at line 1107 - the memory intoglc_frlnd(ns)%field_lfrac_g is set to frac_gand overwritten by dataptr1d. The frac_g is used. I totally agree that this is confusing and that 1107 should read call field_getdata1d(toglc_frlnd(ns)%field_frac_g, frac_g, rc) But it would not make any difference in the answer. But it should be fixed to clarify things. Does that make sense?

[billsacks]

@mvertens thanks a lot for looking into this. It's good to know that this doesn't actually impact anything. My mistake was assuming that field_normdst in the call to med_map_field_normalized was an input rather than an output.

In terms of a solution: I don't understand this very well, but my first thought is that it would be more straightforward if you changed this:

https://github.com/ESCOMP/CMEPS/blob/ca3a0adebb72fe7a6cc11ea7ee951bab4bfe634b/mediator/med_phases_prep_glc_mod.F90#L1104-L1108

to get rid of the intermediate dataptr1d, and simply put the data directly into the local frac_g variable without filling anything in a module-level variable. i.e.,

    ! get frac_g(:), the total ice fraction in each glc gridcell
    call fldbun_getdata1d(is_local%wrap%FBImp(compglc(ns),compglc(ns)), Sg_frac_fieldname, frac_g, rc)
    if (chkerr(rc,__LINE__,u_FILE_u)) return

-- deleting the following lines (i.e., deleting the field_getdata1d call). This would be equivalent to what's done to get topo_g a few lines above.

Would that work? Part of why I prefer this - if it works just as well - is that I find the logic confusing if the med_phases_prep_glc_renormalize_smb subroutine sets some module-level variables: that routine is not always called, so any module-level data that it sets would be available to other subroutines in some situations but not in others.

In that case, it looks like you could remove field_frac_g, because it's not currently referenced anywhere.

[mvertens]

@billsacks - can you please suggest a compset to use to test that the proposed fixes are in fact bfb.

[billsacks]

Probably a T compset (e.g., ERS_Ly7.f09_g17_gris4.T1850Gg.cheyenne_intel from the prealpha list) and an I compset (SMS_Lm13.f10_f10_mg37.I1850Clm50SpG.cheyenne_intel from the prealpha list).