Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to improve vegetation health at high latitudes #2348

Merged
merged 15 commits into from
Mar 14, 2024

Conversation

olyson
Copy link
Contributor

@olyson olyson commented Feb 6, 2024

Description of changes

Goal is to improve vegetation health/survivability at high latitudes (address LMWG_dev discussion #3)

Specific notes

These changes include:

Remove snicar_snobc_intmix from EXPERIMENTAL endrun (allow it to be true)
Remove flg_snoage_scl in SNICAR such that xdrdt can have an effect (fixes #2298 )
New parameter file and namelist values for clm5_1:

  1. snow_thermal_cond_method = Sturm1997 (default for clm5_1)
  2. snicar_snobc_intmix = .true. (default for clm5_1)
  3. ctsm51_params.c240208.nc is the new CTSM parameter file (changes: froot_leaf(11:12)=1.2, FUN_fracfixers(11:12)=1, xdrdt=5, scvng_fct_mlt_sf=0.5, snw_rds_refrz=1500, fresh_snw_rds_max=400)
  4. New history fields for coupler history verification (default off)
  5. Add snow5d_thresh_for_onset to parameter file, set to 0.2 for clm51 and 0.1 (unchanged) for clm50 and clm45.

Contributors other than yourself, if any: @wwieder , @dlawrenncar , @slevis-lmwg , LMWG members

CTSM Issues Fixed (include github issue #): LMWG_dev discussion #3 and #2298

Are answers expected to change (and if so in what way)? Yes, new climate

Any User Interface Changes (namelist or namelist defaults changes)? Yes, new namelist defaults including new parameter files:
ctsm51_params.c240208.nc
clm50_params.c240208.nc
clm45_params.c240208.nc
ctsm51_ciso_cwd_hr_params.c240208.nc

Testing performed, if any:
Replicate simulation in LMWG_dev issue #51 as bfb when snow5d_thresh_for_onset on ctsm51_ciso_cwd_hr_params.c240208.nc was set to original value of 0.1.

aux_clm on Derecho:
No unexpected failures.
NLCOMP failures as expected.
clm51 compsets fail baseline comparison as expected.
clm50 and clm45 pass baseline comparisons as expected.
These EXPECTED FAILURE tests passed:
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Bgc.derecho_gnu.clm-default--clm-NEON-NIWO RUN time=62 (UNEXPECTED: expected FAIL)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Bgc.derecho_gnu.clm-NEON-MOAB--clm-PRISM RUN time=79 (UNEXPECTED: expected FAIL)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Fates.derecho_gnu.clm-FatesFireLightningPopDens--clm-NEON-FATES-NIWO RUN time=52 (UNEXPECTED: expected FAIL)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL RUN time=35 (UNEXPECTED: expected FAIL)
PASS SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm51SpRs.derecho_gnu.clm-default--clm-NEON-TOOL RUN time=33 (UNEXPECTED: expected FAIL)

@olyson olyson added this to the ctsm5.2.0 milestone Feb 6, 2024
@olyson olyson self-assigned this Feb 6, 2024
@ekluzek ekluzek added enhancement new capability or improved behavior of existing capability next this should get some attention in the next week or two. Normally each Thursday SE meeting. labels Feb 6, 2024
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olyson this is really nice to see become a PR. It'll be a nice update.

I have a couple things that we need to do here. You won't necessarily be the one that does them, but I marked them as something that needs to happen.

It also looks to me like this will also change answers for clm45 and clm50 physics, and I wasn't sure if that's acceptable or not? So there's some questions about that...

bld/namelist_files/namelist_defaults_ctsm.xml Outdated Show resolved Hide resolved
src/biogeophys/SnowSnicarMod.F90 Show resolved Hide resolved
src/main/controlMod.F90 Show resolved Hide resolved
@ekluzek ekluzek removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Feb 8, 2024
Copy link
Contributor

@wwieder wwieder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, @olyson just a few comments / questions.

src/main/atm2lndType.F90 Outdated Show resolved Hide resolved
bld/namelist_files/namelist_defaults_ctsm.xml Show resolved Hide resolved
bld/namelist_files/namelist_defaults_ctsm.xml Show resolved Hide resolved
src/biogeophys/SnowSnicarMod.F90 Show resolved Hide resolved
@ekluzek
Copy link
Collaborator

ekluzek commented Feb 22, 2024

Just FYI on the latest I heard about the cesm2_3_beta17 tag. The next alpha tag is close to being done. And there are two tags after it. The first one has several CAM changes that need to come in that are likely a few weeks away. It would be really great to get this into cesm2_3_beta17, so hopefully that will give time for this to be figured out before then.

So the beta tag is roughly a few weeks to a month away...

@wwieder
Copy link
Contributor

wwieder commented Mar 5, 2024

@olyson the scientific tests you ran look good. Is it worth discussing this?
Are we ready to bring this PR into main?

@olyson
Copy link
Contributor Author

olyson commented Mar 5, 2024

Maybe we can discuss at our meeting tomorrow. I think it is probably as good as we can get it for now. A question that came up in the LMWG meeting was whether to do some kind of elevation threshold for the Sturm approach, but that would require more research, etc. And maybe that would be more important for high-resolution simulations. We'll also have to think about what initial file we'll want to use in conjunction with these changes in the next coupled model set of simulations.

@wwieder
Copy link
Contributor

wwieder commented Mar 5, 2024

I agree, the alpine vs. tundra distinction is something that needs more scientific evaluation as we as conversations about SE support. I suggest we table this option for now. Moreover, for regional or point runs users can still switch between schemes using the namelist options.

We should discuss the initial condition question, however. Is a longer spinup needed to provide better initial conditions? Maybe this would be a good idea anyway if the 5.2 PR is close to coming to main? We can discuss tomorrow.

@slevis-lmwg
Copy link
Contributor

I need to ./rimport the new params files.

@ekluzek I have the go-ahead to merge this to main (from @wwieder ).

@ekluzek
Copy link
Collaborator

ekluzek commented Mar 6, 2024

@slevis-lmwg cool! I moved it up in upcoming tags. @rgknox has the next tag for FATES history dimension control. And this should then be next.

@ekluzek ekluzek marked this pull request as ready for review March 6, 2024 17:58
@slevis-lmwg slevis-lmwg self-assigned this Mar 6, 2024
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through this again since it will be coming in right away. @slevis-lmwg already knows this, but I just pointed out about moving files to CESM inputdata.

There's some discussion about namelist settings and specifying the default for all clm physics versions. Technically if you do...

<namelist_thing>blah</namelist_thing>
<namelist_thing phys="clm5_1">bigger blah</namelist_thing>

That's equivalent to

<namelist_thing phys="clm4_5">blah</namelist_thing>
<namelist_thing phys="clm5_0">blah</namelist_thing>
<namelist_thing phys="clm5_1">bigger blah</namelist_thing>

BUT, the advantage of the later is that if you don't set ALL the clm physics versions -- it'll complain if that namelist setting is required. So it'll flag an error when we bring in a new physics version for the first time. I think it's also clearer.

bld/namelist_files/namelist_defaults_ctsm.xml Outdated Show resolved Hide resolved
Set initial t_soisno=272K for soils and 274K for urban road

slevis resolved conflicts:
bld/namelist_files/namelist_defaults_ctsm.xml
cime_config/testdefs/testmods_dirs/clm/ciso_cwd_hr/user_nl_clm
src/main/atm2lndType.F90
@slevis-lmwg
Copy link
Contributor

I will hold off running the test-suites until I update to dev172.

@slevis-lmwg
Copy link
Contributor

@olyson could you look over my ChangeLog entry and let me know what you think?

Copy link
Contributor Author

@olyson olyson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment regarding the parameter date, otherwise, it looks good to me. I suppose that instead of saying "new climate" we could say "new climate at high latitudes".

doc/ChangeLog Outdated Show resolved Hide resolved
@slevis-lmwg
Copy link
Contributor

@ekluzek helped me resolve the ./manage_externals error with the following (I will post this in the CTSM_SE chat, too):

cd /src/fates
git fetch --all
cd ../..
./manage_externals/checkout_externals

Now I have submitted the test-suites.

@slevis-lmwg slevis-lmwg merged commit f448885 into ESCOMP:master Mar 14, 2024
2 checks passed
@slevis-lmwg slevis-lmwg deleted the deadveg branch March 14, 2024 23:10
@olyson
Copy link
Contributor Author

olyson commented Mar 14, 2024

I was just looking at the test results on derecho for this which I assume is:

/glade/derecho/scratch/slevis/tests_0314-145540de

I noticed that there are differences with baseline for some Clm50 tests (all fates tests). Based on my original testing, I wouldn't expect this. My results showed no differences for all Clm50 tests. Do we know why these baseline comparisons fail?

@slevis-lmwg
Copy link
Contributor

Sorry for overlooking that. I do not know why...

@olyson
Copy link
Contributor Author

olyson commented Mar 14, 2024

Given the discussion I saw regarding fates externals, I wonder if it has something to do with that?

@slevis-lmwg
Copy link
Contributor

I was about to say the same.

@slevis-lmwg
Copy link
Contributor

I am open to suggestions on how to proceed @ekluzek

@olyson
Copy link
Contributor Author

olyson commented Mar 14, 2024

Just noting that there are a couple of Clm45Fates DIFF failures as well.

@ekluzek
Copy link
Collaborator

ekluzek commented Mar 15, 2024

Here's the list of changed tests that I'm seeing (for non Clm51):

ERP_Ld9.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesColdAllVars
ERS_D_Ld3_PS.f09_g17.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
ERS_D_Ld5.f10_f10_mg37.I2000Clm50Fates.derecho_intel.clm-FatesCold
ERS_D_Mmpi-serial_Ld5.1x1_brazil.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold
ERS_D_Mmpi-serial_Ld5.5x5_amazon.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold
ERS_Ld30.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesColdFixedBiogeo
ERS_Ld30.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesColdSizeAgeMort
ERS_Ld9.f10_f10_mg37.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesColdCH4Off
SMS_D_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold
SMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold
SMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
SMS_D_Lm6_P256x1.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold
SMS_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
SMS_Ld5_PS.f19_g17.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold

@rgknox
Copy link
Collaborator

rgknox commented Mar 15, 2024

I'm looking through this more closely, but I think that the change to "dr" should propogate into FATES results as well, which should explain the diffs.

https://github.com/ESCOMP/CTSM/pull/2348/files#diff-a313a6093d335d65fdb56abaedbbc3416f8b8f5f095f828ddd703f907d57526bR1659

@slevis-lmwg
Copy link
Contributor

I'm looking through this more closely, but I think that the change to "dr" should propogate into FATES results as well, which should explain the diffs.

https://github.com/ESCOMP/CTSM/pull/2348/files#diff-a313a6093d335d65fdb56abaedbbc3416f8b8f5f095f828ddd703f907d57526bR1659

@olyson didn't think so according to this conversation, but - just in case - I'm trying SMS_Ld5_PS.f19_g17.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold (the last test in Erik's list above) as follows:

git checkout ctsm5.1.dev173
git switch -c ctsm5.1.dev173
./manage_externals/checkout_externals
vi SnowSnicarMod.F90

I changed line 1659 to what's in dev174 (same as Ryan's link in the last post).

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Mar 15, 2024

The last test returned:
PASS SMS_Ld5_PS.f19_g17.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev174:
FAIL SMS_Ld5_PS.f19_g17.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev173: DIFF

Going up the above list of tests:
PASS SMS_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev174:
FAIL SMS_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev173: DIFF

PASS SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev174:
FAIL SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev173: DIFF

BUT... I restored the code to dev173 and reran the very last test and got exactly the same result:
PASS SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev174:
FAIL SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold BASELINE /glade/campaign/cgd/tss/ctsm_baselines/ctsm5.1.dev173: DIFF

Which tells me that the dev173 baseline is wrong? We will meet to discuss in 4 minutes.

@slevis-lmwg
Copy link
Contributor

@rgknox will rerun the dev173 baselines.
Then I will rerun a clm45 + clm50 test on each machine (izumi + derecho) to confirm that things work.

ekluzek added a commit to ekluzek/CTSM that referenced this pull request Mar 15, 2024
Changes to improve vegetation health at high latitudes

Details in PR ESCOMP#2348

 Conflicts:
	.git-blame-ignore-revs
	bld/unit_testers/build-namelist_test.pl
	cime_config/testdefs/testlist_clm.xml
	python/ctsm/test/test_sys_lilac_build_ctsm.py
ekluzek added a commit to dmleung/CTSM that referenced this pull request Mar 15, 2024
Changes to improve vegetation health at high latitudes

Details in PR ESCOMP#2348

 Conflicts:
	bld/unit_testers/build-namelist_test.pl
@slevis-lmwg
Copy link
Contributor

I reran the following tests and confirmed that Clm45 and Clm50 are bit-for-bit against dev173 now:

PASS SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold BASELINE ctsm5.1.dev173:
PASS SMS_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold BASELINE ctsm5.1.dev173:
PASS SMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.izumi_nag.clm-FatesCold BASELINE ctsm5.1.dev173:
PASS SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.izumi_intel.clm-FatesCold BASELINE ctsm5.1.dev173:

slevis-lmwg added a commit to chrislxj/ctsm that referenced this pull request Mar 19, 2024
Changes to improve vegetation health at high latitudes

Details in PR ESCOMP#2348

slevis resolved conflicts:
bld/unit_testers/build-namelist_test.pl
slevis-lmwg pushed a commit that referenced this pull request Jul 11, 2024
and add Clm45 default for urban_explicit_ac

Changes to improve vegetation health at high latitudes

Details in PR #2348
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new capability or improved behavior of existing capability
Projects
Status: Done
5 participants