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

Fix defaults in clm6_0 physics #2492

Closed
wwieder opened this issue Apr 26, 2024 · 13 comments · Fixed by #2501
Closed

Fix defaults in clm6_0 physics #2492

wwieder opened this issue Apr 26, 2024 · 13 comments · Fixed by #2501
Assignees
Labels
bug something is working incorrectly priority: Immediate Highest priority, something that was unexpected

Comments

@wwieder
Copy link
Contributor

wwieder commented Apr 26, 2024

Brief summary of bug

I'm creating cases with CLM5.2 and noticed that snow_thermal_cond_method defaults to Jordan1991 in both SP and BGC cases. Is this intended? I thought we switched this to Sturm1997 in #2348?

@wwieder wwieder added type: -discussion bug something is working incorrectly labels Apr 26, 2024
@wwieder wwieder added this to the ctsm5.3.0 milestone Apr 26, 2024
@ekluzek ekluzek modified the milestones: ctsm5.3.0, cesm2_3_beta17 Apr 26, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Apr 26, 2024

@wwieder good job on pointing this out!

This looks to be my bad, and was an oversight in bringing in clm6_0 physics. I think I see why this happened, I'm going to review to figure out what process could have prevented it. I also think this applies to a few other things and I'll point them out.

We need to fix this for cesm2_3_beta17, so I'm changing the milestone.

@ekluzek ekluzek changed the title Is Sturm1997 the default in CLM6 physics Fix defaults in clm6_0 physics Apr 26, 2024
@ekluzek ekluzek added the priority: Immediate Highest priority, something that was unexpected label Apr 26, 2024
@ekluzek ekluzek self-assigned this Apr 26, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Apr 26, 2024

@wwieder the workaround would be to use clm5_1 physics. You can do that by either using a long compset name with CLM51 rather than CLM60, or to change CLM_PHYSICS_VERSION in your case to clm6_0.

@ekluzek ekluzek moved this from Todo to In Progress in LMWG: Near Term Priorities Apr 26, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Apr 26, 2024

@wwieder I'm thinking the version with the right defaults is ctsm5.1.dev174, with the updated defaults for arctic vegetation health. Does that sound right to you?

@olyson
Copy link
Contributor

olyson commented Apr 26, 2024

Wondering if we should quickly conduct a simulation or two (spinup, historical?) once this comes to main to see if we get the results we expect, particularly with regard to the deadveg changes?

@ekluzek
Copy link
Collaborator

ekluzek commented Apr 26, 2024

@olyson that does make sense. I was thinking that minor version updates should have that done for them. It's possible once this is corrected that you'll get identical results to when you did this in ctsm5.1.dev174 though. So we might not have to do full long simulations.

@ekluzek
Copy link
Collaborator

ekluzek commented Apr 26, 2024

In comparing clm5_1 and clm6_0 namelists I see this difference...

diff lnd_in.clm5_1-bgc.-bgc+bgc+-crop+-res+0.9x1.25+-envxml_dir+.  lnd_in.clm6_0-bgc.-bgc+bgc+-crop+-res+0.9x1.25+-envxml_dir+. 
32c32
<  paramfile = '/glade/campaign/cesm/cesmdata/cseg/inputdata/lnd/clm2/paramdata/ctsm51_params.c240208.nc'
---
>  paramfile = '/glade/campaign/cesm/cesmdata/cseg/inputdata/lnd/clm2/paramdata/ctsm60_params.c240208.nc'
36c36
<  snicar_snobc_intmix = .true.
---
>  snicar_snobc_intmix = .false.
42c42
<  snow_thermal_cond_method = 'Sturm1997'
---
>  snow_thermal_cond_method = 'Jordan1991'

the parameter file is a renamed copy for clm6_0, and I reverified that they are identical with cprnc...

$CPRDIR/cprnc -m /glade/campaign/cesm/cesmdata/cseg/inputdata/lnd/clm2/paramdata/ctsm60_params.c240208.nc /glade/campaign/cesm/cesmdata/cseg/inputdata/lnd/clm2/paramdata/ctsm60_params.c240208.nc
.
.
.
SUMMARY of cprnc:
 A total number of    423 fields were compared
          of which      0 had non-zero differences
               and      0 had differences in fill patterns
               and      0 had different dimension sizes
               and      0 had different data types
 A total number of      3 fields could not be analyzed
 A total number of      0 fields on file 1 were not found on file 2.
 A total number of      0 fields on file 2 were not found on file 1.
  diff_test: the two files seem to be IDENTICAL 

I do remember comparing clm5_1 and clm6_0 namelists, so I'm trying to figure out what I did there and why it failed...

ekluzek referenced this issue Apr 26, 2024
Update to ctsm5.1.dev174

Update to newer tag. Which also includes an externals
update which broke the build/run of mksurfdata_esmf.
As a result I added some additional unit-testing in
order to detect this sort of thing sooner.

 Conflicts:
	bld/namelist_files/namelist_defaults_ctsm.xml
	bld/unit_testers/build-namelist_test.pl
@ekluzek
Copy link
Collaborator

ekluzek commented Apr 27, 2024

I ran ctsm_sci for ctsm5.1.dev174 so I could compare to ctsm_sci from ctsm5.2.0 and for example with one test I see the above things we talked about along with the ctsm5.2.0 expected changes. But, also irrigate is set to .false.

CIME/Tools> ./compare_namelists /glade/campaign/cgd/tss/ctsm_baselines/ctsm_sci-ctsm5.1.dev174/ERI_Ld9.f09_g17.I2000Clm51BgcCrop.derecho_intel.clm-default/CaseDocs/lnd_in $BASELINE/ctsm_sci-ctsm5.2.0/ERI_Ld9.f09_g17.I2000Clm60BgcCrop.derecho_intel.clm-default/CaseDocs/lnd_in
No case id data available, will not be able to normalize values as effectively
ERROR: Namelist diff between files /glade/campaign/cgd/tss/ctsm_baselines/ctsm_sci-ctsm5.1.dev174/ERI_Ld9.f09_g17.I2000Clm51BgcCrop.derecho_intel.clm-default/CaseDocs/lnd_in and /glade/campaign/cgd/tss/ctsm_baselines/ctsm_sci-ctsm5.2.0/ERI_Ld9.f09_g17.I2000Clm60BgcCrop.derecho_intel.clm-default/CaseDocs/lnd_in
Differences in namelist 'clm_inparm':
  BASE: finidat = clmi.I2000Clm50BgcCrop.2011-01-01.1.9x2.5_gx1v7_gl4_simyr2000_c190312.nc'
  COMP: finidat = clmi.I2000Clm50BgcCrop.2011-01-01.1.9x2.5_gx1v7_gl4_simyr2000_c240223.nc'
  BASE: fsurdat = surfdata_0.9x1.25_hist_78pfts_CMIP6_simyr2000_c190214.nc'
  COMP: fsurdat = surfdata_0.9x1.25_hist_2000_78pfts_c240216.nc'
  BASE: irrigate = .true.
  COMP: irrigate = .false.
  BASE: paramfile = ctsm51_params.c240208.nc'
  COMP: paramfile = ctsm60_params.c240208.nc'
  BASE: snicar_snobc_intmix = .true.
  COMP: snicar_snobc_intmix = .false.
  BASE: snow_thermal_cond_method = 'Sturm1997'
  COMP: snow_thermal_cond_method = 'Jordan1991'
  found extra variable: 'convert_ocean_to_land'
  found extra variable: 'dust_emis_method'
  BASE: stream_fldfilename_urbantv = CLM50_tbuildmax_Oleson_2016_0.9x1.25_simyr1849-2106_c160923.nc'
  COMP: stream_fldfilename_urbantv = CTSM52_tbuildmax_OlesonFeddema_2020_0.9x1.25_simyr1849-2106_c200605.nc'
Found extra namelist: zendersoilerod

@wwieder
Copy link
Contributor Author

wwieder commented Apr 27, 2024

@wwieder I'm thinking the version with the right defaults is ctsm5.1.dev174, with the updated defaults for arctic vegetation health. Does that sound right to you?

Yes this seems accurate

ekluzek added a commit to ekluzek/CTSM that referenced this issue Apr 29, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Apr 30, 2024

In investigating this, I setup a case to reproduce the Arctic health simulations done in preparation for ctsm5.1.dev174 from @olyson, in there I noticed another file that is different the ndep stream file:

[fndep_clm_hist_b.e21.BWHIST.f09_g17.CMIP6-historical-WACCM.ensmean_1849-2015_monthly_0.9x1.25_c180926.nc](http://fndep_clm_hist_b.e21.bwhist.f09_g17.cmip6-historical-waccm.ensmean_1849-2015_monthly_0.9x1.25_c180926.nc/)
for ctsm5.2.0.002
and for ctsm5.1.dev166 through ctsm5.1.dev176
[fndep_clm_WACCM6_CMIP6piControl001_y21-50avg_1850monthly_0.95x1.25_c180802.nc](http://fndep_clm_waccm6_cmip6picontrol001_y21-50avg_1850monthly_0.95x1.25_c180802.nc/)

It's not clear to me right now where this is coming from, but it's also something to figure out more carefully.

@wwieder
Copy link
Contributor Author

wwieder commented Apr 30, 2024

This is funny. Presumably the first file is for a HIST compset, while the second would be for an 1850 compset, but I'm assuming both cases are for 1850 compsets?

@ekluzek
Copy link
Collaborator

ekluzek commented Apr 30, 2024

Yes, both cases are 1850 compsets. And I think I just figured out why this happened and what I need to do to fix it. It's again the common problem of having more than one place where changes need to go. So a traditional software problem. And it's a specific problem we've identified with the namelist defaults where there are several ways of doing it that are mostly duplicative. We discussed this sort of thing in this document: https://docs.google.com/presentation/d/1FIaQmgN35v3tQxd9hpxcUQZ2CJXFpfx991rZgXwMKzA/

I think it's also possible that the HIST version actually has the same date for 1850, but that is another thing to check. If so we likely should just use the HIST version rather than the 1850 one.

For issues that cover making this problem less likely:

#605 #1084 #1763

and of course #585 which is bigger...

Note also #1166 that had similar namelist problems for clm5.0.001. So there is also likely an issue when a new physics option comes in. There are some changes to process that I think will help for that situation.

@ekluzek
Copy link
Collaborator

ekluzek commented Apr 30, 2024

Also I'm not sure it's "funny", at least from my perspective. :-) But, maybe you mean funny in a different way?

@ekluzek
Copy link
Collaborator

ekluzek commented May 1, 2024

OK, so the problem with the remaining settings are the clm5_1 settings in the bld/namelist_defaults/use_cases directory. #605 would help prevent that. The other problem is that the namelist testing wasn't going over each physics versions for each use-case, that would've also prevented this. This also means that we don't have coverage in aux_clm testing for each physics version with each use-case. The full set would be 16x3=48 and we probably only have a quarter of those. So the namelist testing might suffice, and maybe just add a few for the important cases...

@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label May 1, 2024
ekluzek added a commit to ekluzek/CTSM that referenced this issue May 2, 2024
…clm5_1 and clm6_0 physics options is only the params_file which is what's expected this finishes out issues in ESCOMP#2492
@ekluzek ekluzek modified the milestones: cesm2_3_beta18, cesm2_3_beta17 May 7, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in LMWG: Near Term Priorities May 13, 2024
@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly priority: Immediate Highest priority, something that was unexpected
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants