NGEET / fates

repository for the Functionally Assembled Terrestrial Ecosystem Simulator (FATES)
Other
102 stars 92 forks source link

parameters seed_germination_timescale and seed_decay turnover mixup? #591

Closed evalieungh closed 4 years ago

evalieungh commented 5 years ago

I'm going through a list of parameters from a google sheet called FATES parameters, and comparing that list to the actual parameters in fates_params_default.cdl.

In the (older?) google sheet, two of the parameters look like this:

param long name
fates_seed_germination_timescale turnover time for seeds with respect to germination
fates_seed_decay_turnover turnover time for seeds with respect to decay

In the code, the long names are switched, so the germination_timescale has long name with "decay" and the decay_turnover har long name with "germination". Like this:

double fates_seed_decay_turnover(fates_pft) ;
    fates_seed_decay_turnover:units = "1/yr" ;
    fates_seed_decay_turnover:long_name = "turnover time for seeds with respect to germination" ;
double fates_seed_germination_timescale(fates_pft) ;
    fates_seed_germination_timescale:units = "1/yr" ;
    fates_seed_germination_timescale:long_name = "turnover time for seeds with respect to decay" ;

Which is correct? In the sheet they seem to have very similar values (0.5 and 0.51) but if the long name is wrong in the code maybe it's worth checking that the parameter values are not switched as well.

rgknox commented 5 years ago

Hi @evaleriksen,

I don't have a sense of what are reasonable values here, but maybe @lmkueppers might push us in the right direction?

We should indeed update the meta data in the file (and in the code where they are defined). Any suggestions on new definitions?

    double fates_seed_decay_turnover(fates_pft) ;
        fates_seed_decay_turnover:units = "1/yr" ;
        fates_seed_decay_turnover:long_name = "turnover time for seeds with respect to decay" ;
    double fates_seed_germination_timescale(fates_pft) ;
        fates_seed_germination_timescale:units = "1/yr" ;
        fates_seed_germination_timescale:long_name = "germination timescale for seeds (fraction germinated per year)" ;
evalieungh commented 5 years ago

Hi @rgknox, we are going through the list of parameters for a couple of small projects here and aim to contribute something to the documentation in the other end.

Right now I haven't really understood exactly how these parameters are used in the model or what they represent, so I don't have suggestions for new definitions but thought it might be useful to raise this issue in case there is a mix-up in the model with these two parameters.

evalieungh commented 4 years ago

To clarify, the google sheet I mentioned was Hui Tang's working document and not an 'official' one, but he had copied the parameters from an older version of the parameter file.

To me it looks more intuitive that the parameters should be:

   fates_seed_decay_turnover:long_name = "turnover time for seeds with respect to decay"
   fates_seed_germination_timescale:long_name = "germination timescale for seeds (fraction germinated per year)"

so the issue still stands; has there been a mixup with the long names?

rgknox commented 4 years ago

@evaleriksen , I'm going to submit an update to the parameter file, I will include these fixes to the long names.

pollybuotte commented 4 years ago

@rgknox, #581 notes these and one other parameter file definition edit.

rgknox commented 4 years ago

Actually, I'm looking at how these two parameters, seed_decay_turnover and seed_germination_timescale are used in EDPhysiologyMod. They are indeed rates, with units yr-1, and not timescales. I will also flag @pollybuotte and @evaleriksen on the PR to view the changes.

rgknox commented 4 years ago

closing via #595