Problems with namelist defaults starting in ctsm5.0.dev001 #1166
Comments
|
For one, organic_frac_squared should not have changed from CLM5 to CLM5.1.
…On Tue, Sep 29, 2020 at 8:41 PM Bill Sacks ***@***.***> wrote:
Brief summary of bug
I am reviewing the changes in ctsm5.0.dev001. I noticed a few possible
problems with the namelist defaults in that tag. I'm not sure if all of
these are actually bugs, but I wanted to bring this to the attention of
@ekluzek <https://github.com/ekluzek> and others in case some should be
fixed. *One of these issues, organic_frac_squared, appears to lead to
incorrect results in out-of-the-box clm51 configurations (unless this
change was intentional).* The other issues would just affect runs in
non-standard configurations, or are cleanup issues.
General bug information
*CTSM version you are using:* ctsm5.1.dev001
*Does this bug cause significantly incorrect results in the model's
science?* Yes, I think so, for clm51 cases, because of incorrect value
for organic_frac_squared.
*Configurations affected:* clm51 cases, and non-standard configurations
Details of bug
These are really a few somewhat unrelated issues. I'm grouping them
together here for ease of review:
- organic_frac_squared for clm5_1 uses the same setting as clm4_5; I'm
guessing this is wrong:
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/namelist_defaults_ctsm.xml#L163-L165
- Some fire-related namelist variables depend on the physics version
directly, whereas my intuition is that they should depend on the fire
method. As a thought experiment: if someone sets up a clm51 case but
reverts to the li2016crufrc fire method, or conversely sets up a clm50 case
but changes to li2021gswpfrc, what defaults should be used for these?
Specifically, this is the case for the lightning and popdens streams: the
light_res variable and the three popdens variables that depend on phys
version – though it looks like the popdens variables don't actually
differ with different phys versions, so the duplication in xml is
irrelevant and could probably be removed:
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/namelist_defaults_ctsm.xml#L1569-L1573
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/namelist_defaults_ctsm.xml#L1592-L1602
- It looks like some fire-related namelist variables would be
undefined for certain values of lnd_tuning_mode, when using li2016crufrc:
rh_low and pot_hmn_ign_counts_alpha. I think these should be given
default values for li2016crufrc without a lnd_tuning_mode attribute,
so that they are given some reasonable value for other lnd_tuning_modes (I
*think* that's the right way to fix this problem). This could arise in
practice if someone sets up a clm51 case but reverts to the li2016crufrc
fire method, or conversely sets up a clm50 case but changes to
li2021gswpfrc (and probably other ways, too):
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/namelist_defaults_ctsm.xml#L265-L270
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/namelist_defaults_ctsm.xml#L276-L281
- Cleanup that shouldn't have any impact: spinup_state has some
duplicated lines for fates: there are 2 unique lines here, each doubled:
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/namelist_defaults_ctsm.xml#L53-L56
-
Cleanup that shouldn't have any impact: the 2010 control use case has
some unnecessary clm4_0 settings
-
Cleanup that shouldn't have any impact: the 2010 control use case has
a duplicated line:
https://github.com/ESCOMP/CTSM/blob/08bc0ade130b23a67a9366120f8033897505c74b/bld/namelist_files/use_cases/2010_control.xml#L11-L12
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1166>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFABYVCZ6K76S7F62WKPU23SIKLHDANCNFSM4R6Q6SJA>
.
|
|
I was thinking a bit about how you could test to ensure that there are no other unintentional changes between clm50 and clm51. @ekluzek maybe you've already done something like this, but if not, I'll share my thoughts: The simplest thing to do, which would pick up problems with the default configuration but not with modified configurations, would be to do one multi-year run with a clm51 case and its clm50 equivalent; in the clm51 case, revert the few things that have changed intentionally (tuning mode, fire method, param file, lightning stream info; maybe others?) back to their clm50 values. I think these should be bit-for-bit. I think that more rigorous testing could be done as follows:
|
Partially addresses ESCOMP#1166
|
OK, I looked into the history of the organic_frac_squared setting to figure out where this bug happened. It can be instructive to look into it in order to see how to prevent such things in the future. I thought this likely started with the original branch based off of release-clm5.0, but it was correct there. The error came in when I used patch to merge in to the latest version. I don't think it was a problem with patch itself, but in my wrong interpretation of it. But, possibly this puts another tick in favor of re-base over patch. In most cases the build-namelist unit testing would also catch this problem, but because the baseline didn't have clm5_1 to compare to -- it couldn't. A test I didn't do that might have been useful was to compare it to the release-clm5.0 version. The problem is all the other namelist differences would swamp out the results. I did that now, and I agree with that assessment. Technically the problem shows up, but I likely wouldn't have seen it. But, a simple test I could have done was to compare the clm5_0 namelist with clm5_1, when I do that the problem pops out. It also shows other fire namelist settings as correct, and two fire namelist settings that depend on the lnd_tuning_mode settings. But, I could then validate those as correct. So that would've been a better way to do this. The other thing I thought of doing but didn't was to run a simulation in the release version, and then one in the updated version. But, because answers are expected to be different and the namelists are expected to be different I still don't think that would've been useful. |
|
@ekluzek If you haven't already, I still feel it could be good to do some testing like what is discussed in #1166 (comment) to ensure that there aren't other issues like this. Although, as you point out, it's probably sufficient to compare the namelists... but ideally this namelist comparison would be done across a range of configurations, not just a single test. |
Answer changing bug fixes for clm51: fire and organic_frac_squared A few bug fixes for CLM51: - Initialize btran2 to 0 in CNFireLi2021Mod - Remove multiplications by col%wtgcell in CNFireLi2021Mod - Initialize nfire to 0 in CNFireLi2021Mod - Fix organic_frac_squared default for clm5_1 Resolves #1171 (btran2 not initialized to zero in CNFireLi2021Mod.F90) Partially addresses #1169 (CNFireLi2021Mod reintroduces a number of bugs that were fixed in earlier versions) Partially addresses #1166 (Possible problems with namelist defaults starting in ctsm5.0.dev001)
|
So far I've compared namelists, and I'll show the list below. The one additional problem I've identified is the finidat setting for 2000 for clm5_1 physics is wrong. the list of tests includes various resolutions, bgc settings, crop on and off, as well as spinup. The bad finidat for 2000 won't matter for simulations where you startup from a REFCASE you define. I'll add this to the list above. Here's the list of namelist comparisons... ./lnd_in.clm5_1-bgc.-res+1.9x2.5+-bgc+bgc+-crop+-use_case+1850_control+-envxml_dir+.+-namelist+++a+start_ymd=18500101++ |
ekluzek
changed the title
|
Thanks @ekluzek . I think the extensive namelist comparisons are sufficient: no need to do actual test runs as I originally suggested. |
Fire edge-case bugfix
samsrabin
added
good first issue
Brief summary of bug
I am reviewing the changes in ctsm5.0.dev001. I noticed a few possible problems with the namelist defaults in that tag. I'm not sure if all of these are actually bugs, but I wanted to bring this to the attention of @ekluzek and others in case some should be fixed. One of these issues, organic_frac_squared, appears to lead to incorrect results in out-of-the-box clm51 configurations (unless this change was intentional). The other issues would just affect runs in non-standard configurations, or are cleanup issues.
General bug information
CTSM version you are using: ctsm5.1.dev001
Does this bug cause significantly incorrect results in the model's science? Yes, I think so, for clm51 cases, because of incorrect value for organic_frac_squared.
Configurations affected: clm51 cases, and non-standard configurations
Details of bug
These are really a few somewhat unrelated issues. I'm grouping them together here for ease of review:
organic_frac_squaredforclm5_1uses the same setting asclm4_5; I'm guessing this is wrong:CTSM/bld/namelist_files/namelist_defaults_ctsm.xml
Lines 163 to 165 in 08bc0ad
Default finidat files for sim-year 2000 are incorrect (fixed in ctsm5.1.dev008)
Some fire-related namelist variables depend on the physics version directly, whereas my intuition is that they should depend on the fire method. As a thought experiment: if someone sets up a clm51 case but reverts to the li2016crufrc fire method, or conversely sets up a clm50 case but changes to li2021gswpfrc, what defaults should be used for these? Specifically, this is the case for the lightning and popdens streams: the
light_resvariable and the threepopdensvariables that depend on phys version – though it looks like thepopdensvariables don't actually differ with different phys versions, so the duplication in xml is irrelevant and could probably be removed:CTSM/bld/namelist_files/namelist_defaults_ctsm.xml
Lines 1569 to 1573 in 08bc0ad
CTSM/bld/namelist_files/namelist_defaults_ctsm.xml
Lines 1592 to 1602 in 08bc0ad
rh_lowandpot_hmn_ign_counts_alpha. I think these should be given default values for li2016crufrc without alnd_tuning_modeattribute, so that they are given some reasonable value for other lnd_tuning_modes (I think that's the right way to fix this problem). This could arise in practice if someone sets up a clm51 case but reverts to the li2016crufrc fire method, or conversely sets up a clm50 case but changes to li2021gswpfrc (and probably other ways, too):CTSM/bld/namelist_files/namelist_defaults_ctsm.xml
Lines 265 to 270 in 08bc0ad
CTSM/bld/namelist_files/namelist_defaults_ctsm.xml
Lines 276 to 281 in 08bc0ad
CTSM/bld/namelist_files/namelist_defaults_ctsm.xml
Lines 53 to 56 in 08bc0ad
Cleanup that shouldn't have any impact: the 2010 control use case has some unnecessary
clm4_0settingsCleanup that shouldn't have any impact: the 2010 control use case has a duplicated line:
CTSM/bld/namelist_files/use_cases/2010_control.xml
Lines 11 to 12 in 08bc0ad
The text was updated successfully, but these errors were encountered: