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

Adding excess ground ice to CTSM #1787

Merged
merged 103 commits into from
Aug 5, 2023
Merged

Conversation

mvdebolskiy
Copy link
Contributor

@mvdebolskiy mvdebolskiy commented Jun 24, 2022

Description of changes

Add parameterization to allow excess ice in soil and subsidence

Specific notes

Parameterization for excess ice is described in Lee et al., 2014.

This code is a modified version of code provided by Lei Cai.

Works only for the nuopc driver.

Files changed:
bld/CLMBuildNamelist.pm, bld/namelist_files/namelist_defaults_ctsm.xml, bld/namelist_files/namelist_definitionss_ctsm.xml -- added namelist options;
src/main/clm_varctl.F90, src/main/controlMod.F90 -- added option to switch excess ice physics and read namelist option;
src/biogeophys/WaterStateType.F90 -- added prognostic excess ice variable and a history field;
src/biogeophys/WaterStateBulkType.F90, src/main/clm_instMod.F90
, -- added arguments to soubrutiens for proper initialization;
src/biogeophys/TemperatureType.F90 -- initial soil temperature set to 268.15 K at the cold start (might be redundant if #1460 is added)
src/biogeophys/WaterDiagnosticBulkType.F90 -- added two diagnostic excess ice variables and two history fields;
src/biogeophys/SoilTemperatureMod.F90 -- added main excess ice calculations;
src/biogeophys/TotalWaterAndHeatMod.F90 -- added excess ice to total water for balance checks;
src/biogeophys/SoilHydrologyMod.F90 -- added excess ice for ice fraction calculation;

New files:
src/cpl/share_esmf/ExcessIceStreamType.F90 -- routines to read dataset with initial excess ice concentration

CTSM Issues Fixed (include github issue #):
Fixes #1229
Fixes #1818

Are answers expected to change (and if so in what way)?

Yes, when excess ice parameterization is used.

Any User Interface Changes (namelist or namelist defaults changes)?

New namelist options added:
use_excess_ice (logical, in clm_inparm) default - .false.; turns on excess ice physics
stream_meshfile_exice, stream_fldfilename_exice and stream_mapalgo_exice (char, in exice_streams) defaults - lnd/clm2/paramdata/exice_init_0.125x0.125_ESMFmesh_c20220516.nc, lnd/clm2/paramdata/exice_init_0.125x0.125_c20220516.nc andbilinear; meshfile, stream file and spatial interpolation algorithm for initial values of excess ice. Dataset is interpolated to 0.125x0.125 degrees grid from Brown et al., 1997, can be found here. Dataset is only used during the cold start or for hybrid runs (or start from finidat) that do not have excess ice.

Testing performed, if any:
On sigma2 machines: comparisons with the upstream/master version and switched-off excess ice physics give the same results for soil state variables. More information and figures on changes and testing can be found here

@ka7eh

@ekluzek ekluzek added enhancement new capability or improved behavior of existing capability tag: enh - new science labels Jun 24, 2022
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.

This looks really good and it's exciting to see this advancement come into the model.

I have some detailed requests for changes, most are pretty minor. There's some issues with indentation that would be good to straighten out, as it really helps with readability. I suggest indenting at least two spaces. Here's our style guide on indentation:

https://wiki.ucar.edu/display/ccsm/CLM+Coding+Conventions#CLMCodingConventions-Indentationindent

There's a few places where you have a !MVD comment to show your changes in the code. These need to be removed in final versions, as having these things scattered in the final code make it confusing and hard to read.

There's a hard coded date from my example in ch4FinundatedStreams, we should assess if this is still needed. We'll talk more about that.

I also ask you to put a copy of the stream files on cheyenne so we can look at them. We'll copy the files to the standard location and get it checked into inputdata. But, we should assess if you'll be doing that more often, and if so give you some training on doing it, and then give you the needed permissions.

The last more detailed thing is how this will be used in practice in terms of restarts and such. The stream file gives the level of excess ice at some point in time. If you want to run for a different date (especially a future date) the ice levels should have melted. And as such you should startup from levels from a restart file. Does that mean if you do a startup type run that it'll use values from the stream file, but if you do a hybrid or branch it'll be from the restart file and the stream is ignored? If so I wonder if the stream should only be read on startup to make this more clear? I'd like to discuss this more and figure out the best way to make this clear to CTSM users.

Thanks so much for your work on this. Very exciting to bring it in.

bld/namelist_files/namelist_defaults_ctsm.xml Show resolved Hide resolved
bld/CLMBuildNamelist.pm Outdated Show resolved Hide resolved
src/biogeophys/SoilTemperatureMod.F90 Outdated Show resolved Hide resolved
src/biogeophys/SoilTemperatureMod.F90 Outdated Show resolved Hide resolved
src/biogeophys/SoilTemperatureMod.F90 Outdated Show resolved Hide resolved
src/main/controlMod.F90 Outdated Show resolved Hide resolved
src/main/controlMod.F90 Outdated Show resolved Hide resolved
src/cpl/share_esmf/ExcessIceStreamType.F90 Show resolved Hide resolved
src/biogeophys/SoilTemperatureMod.F90 Outdated Show resolved Hide resolved
src/biogeophys/SoilTemperatureMod.F90 Outdated Show resolved Hide resolved
@mvdebolskiy
Copy link
Contributor Author

@slevis-lmwg Added you to my fork. This branch is linked to this PR.

@slevis-lmwg
Copy link
Contributor

@swensosc @mvdebolskiy @ekluzek
I made a branch tag as follows:

git tag -a branch_tags/exice_wrpmip.n01_ctsm5.1.dev129
git push escomp branch_tags/exice_wrpmip.n01_ctsm5.1.dev129

Test suites OK and generated baselines using the branch tag's name exice_wrpmip.n01_ctsm5.1.dev129

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jun 27, 2023

  • Add a note to the issue handling that this is a permanent feature as opposed to others that might be removed.
  • Add issue as a parameter to restUtils and use in restFileMod so it's defined in one place.
  • Shorten the 12 month test, but add a note that we aren't really testing excess ice unless there's the rare chance that a gridcell will have excess ice melt. Since, this isn't a standard feature that's always on, we think this might be OK.
  • Only have one test for this since this isn't currently a standard feature right now.

To address these pending TODOs, I followed up with @ekluzek :

  • Not shortening the 12-m test because it only runs in the ctsm_sci test-suite. However, Erik requested that I make sure to run this test because it probably didn't run in the aux_clm test_suite SMS_Lm12.f09_f09_mg17.I1850Clm51Sp.cheyenne_intel.clm-ExcessIceStartup_output_sp_exice
  • Of the other excess ice test (that runs in the aux_clm test-suite), we will keep the izumi one and remove the cheyenne one.

@slevis-lmwg
Copy link
Contributor

PASS: SMS_Lm12.f09_f09_mg17.I1850Clm51Sp.cheyenne_intel.clm-ExcessIceStartup_output_sp_exice

I think this PR is ready. When it's next in line to merge, I will need to:

  • Rerun the test-suites
  • Update ChangeLog/ChangeSum

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jul 31, 2023

Test-suites:
Izumi ./run_sys_tests -s aux_clm -c ctsm5.1.dev131 -g ctsm5.1.dev132 OK

  • NLCOMP fails expected
  • ERI_D_Ld9_P48x1_Vmct.f10_f10_mg37.I2000Clm50Sp.izumi_nag.clm-SNICARFRC FAIL SHAREDLIB_BUILD
    lnd.bldlog: WARNING: buildlib is being called as a program rather than a subroutine as it is expected to be in the CESM context ...but I also see no evidence of this test in the dev131 baselines, so I suspect that it should have been removed.

Cheyenne OK

  • NLCOMP fails expected

@ekluzek ekluzek merged commit f47ce5c into ESCOMP:master Aug 5, 2023
@ekluzek ekluzek deleted the exice_nuopc_pr branch August 5, 2023 00:01
@samsrabin samsrabin added the science Enhancement to or bug impacting science label Aug 8, 2024
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 science Enhancement to or bug impacting science
Projects
Status: Ready to eat (Done!)
Status: Done (non release/external)
4 participants