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

Dynamical lakes and new lake cover map #1049

Closed
wants to merge 40 commits into from

Conversation

Ivanderkelen
Copy link
Contributor

Description of changes

Dynamic lakes representing reservoir construction and new lake cover input map using HydroLAKES and GRanD data.

Specific notes

Contributors other than yourself, if any: @billsacks

CTSM Issues Fixed (include github issue #): #200

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

Any User Interface Changes (namelist or namelist defaults changes)?
New namelist handle use_dynlakes is added, off by default

Testing performed, if any:
(List what testing you did to show your changes worked as expected)
(This can be manual testing or running of the different test suites)
(Documentation on system testing is here: https://github.com/ESCOMP/ctsm/wiki/System-Testing-Guide)
(aux_clm on cheyenne for intel/gnu and izumi for intel/gnu/nag/pgi is the standard for tags on master)

Succesfully ran test simulations with IHistCLM50Sp at f09_g17 and f19_g17 (1900-2017)

NOTE: Be sure to check your coding style against the standard
(https://github.com/ESCOMP/ctsm/wiki/CTSM-coding-guidelines) and review
the list of common problems to watch out for
(https://github.com/ESCOMP/CTSM/wiki/List-of-common-problems).

Ivanderkelen and others added 30 commits February 20, 2019 12:31
…dynConsBiogeophysMod.f90

cv is kept constant at water cv for simplicity (altough there is a cv for each lake layer)
TEMPORARY change
… as an history field.

The history field only contains the heat content of the lake water itself, and is a column output.
Up until now, when computing the dynbal correction fluxes for transient
glacier columns, we have been (1) counting the mass and energy in the
ice column as if that is a real state, but on the other hand (2) NOT
accounting for the fact that glacier columns don't represent the soil
under glacier. These two issues work in opposite directions, but (1)
dominates, because there is much more ice (and energy, I think) in the
roughly 50 m of glacial ice than there is in a typical 50 m soil
column.

In discussing this issue with Bill Lipscomb, we came up with the idea of
subtracting some baseline value from each glacier column. I think that,
as long as we subtract the same baseline value throughout an entire
simulation for a given column, we will still conserve mass and energy
through dynamic landunit transitions.

So, here we subtract baseline values from glacier columns, accounting
for the two issues mentioned above: (1) we subtract the water and energy
in the glacier ice, because this is a virtual state in CTSM, and (2) we
add the water and energy from the vegetated column(s), to account for
the fact that we don't have an explicit representation of
soil-under-glacier (this carries the assumption that the
soil-under-glacier has the same state as the initial vegetated state in
that grid cell). We currently set these baselines in initialization, so
they are set equal to the cold start state. Water and ice in the glacial
ice stay fixed over the course of a simulation, so the cold start values
should be the same as the current values at any point in time. The heat
content of the glacial ice does change over time, but by subtracting
this baseline value, we should be able to reduce the dynbal sensible
heat fluxes.

In a following commit, I plan to allow the user to reset these baselines
at some desired point in the simulation. I think that, in general, this
resetting would break conservation. But as long as it is done before the
onset of transient glaciers, I think this should be okay. In this way,
the user can minimize dynbal fluxes even further, by resetting the
baselines after the system has spun up.

Partially addresses ESCOMP#274
This will be needed on the restart file once we introduce a flag that
allows the user to reset them at any point in the run.
Erik suggests making this a fatal error (even though Mariana at one
point said that we should allow branch runs to succeed if they have the
same namelist settings as the valid namelist settings for a startup
run).
…namic lake land unit. Use similar to do_transient_crops
We want at least one restart test that covers the new
reset_dynbal_baselines flag, to make sure restarts happen properly with
this flag: the baselines set in the initial run should be preserved
across restart.

Also, change this test from ERS to ERP to catch errors related to
changing processor count.
Ideally, we would keep begwb and endwb consistent with liq1, liq2, ice1
and ice2 in this respect - i.e., subtracting the dynbal baselines in all
cases. However, this changes answers for methane in a way we want to
avoid right now.

See ESCOMP#659 for details.
PGI didn't like these long comment lines
…) and accounted for cases where pct urban + pct lake > 100, adjusting pct lake so that total is 100%
There are two files in NetCDF-4 format that the model uses. Copy these files to NetCDF-3 classic format and point
to the new version in the CLM XML database (use nccopy -k classic). There are some machines that have trouble with
reading NetCDF-4 files in pnetcdf.

There are still some NetCDF-4 files for mksurfdata_map, but some of these are required to be in NetCDF-4 format. And we only
support mksurfdata_map and mkmapdata on cheyenne.
The purpose of this merge is to get the dynbal baseline stuff that is in
this branch tag but not on the release branch.

Resolved Conflicts:
	src/biogeophys/TotalWaterAndHeatMod.F90
Merge dynbal baseline changes into dynlake branch
@Ivanderkelen
Copy link
Contributor Author

The dynlakes branch includes everything in branch_tags/ismip6.n01_release-clm5.0.25 and is up to date with the release-clm5.0.30 tag.

I found issue #43, which might interfere with the dynlakes branch (not investigated in detail).

When starting my simulations @billsacks had an first review and made the following comments:

(1) You accidentally added some files. Not a big deal, but something we'll want to clean up before this is merged in eventually:

$ git diff --name-status HEAD..Ivanderkelen/dynlakes | grep '^A' | grep -v dynlakeFileMod
A tools/mksurfdata_map/landuse_timeseries_hist_16pfts_Irrig_CMIP6_simyr1850-2015.txt
A tools/mksurfdata_map/landuse_timeseries_hist_dynlake_simyr1900-2009.txt
A tools/mksurfdata_map/surfdata_0.9x1.25_hist_16pfts_Irrig_CMIP6_simyr1850_c190722.namelist
A tools/mksurfdata_map/surfdata_1.9x2.5_hist_16pfts_Irrig_CMIP6_simyr1850_c190715.namelist

This was done intentionally, as requested by Erik to be able to update the mksurfdat tool to also take the new lake fraction map I created.

(2) It looks like the code you added in surfrdMod – to read the haslake variable – will only work if there is actually a landuse_timeseries file present. Some runs don't have this, which would cause this code to crash. Not important to fix now, but we'll want to fix it eventually.

(3) In dynSubgridControl, you prevent running with dynlakes and fates. I think these would be compatible. (We can't currently run with both transient veg and fates, but I think it would work to run with transient lakes & fates.)

This originally came from following the harvest example. I deleted the lines preventing it but didn't test it with fates.

@billsacks billsacks changed the base branch from master to release-clm5.0 June 19, 2020 15:59
@billsacks
Copy link
Member

@Ivanderkelen thanks a lot for sharing these changes with us! I won't get a chance to do a final review for a couple of weeks, but I'll look forward to doing so once I get a chance.

However, there is some initial work needed here: Your branch was off of the release-clm5.0 branch, but we want to get this onto master, not release-clm5.0. I have gone ahead and moved your commits onto a new branch off of master. I thought this would be relatively quick for me to do, although it ended up being tricky due to a lot of merge conflicts. I have pushed this branch here: https://github.com/billsacks/ctsm/tree/dynlakes_master . I'd suggest the following next steps:

  1. Add my fork as a remote, if you haven't already: git remote add billsacks https://github.com/billsacks/ctsm.git then git fetch billsacks

  2. Checkout a local copy of my branch: git checkout -b dynlakes_master billsacks/dynlakes_master

  3. Check whether you can at least compile this code. As I mentioned, there were a lot of merge conflicts. I did my best to resolve them, but some of them were tricky and I may have gotten some things wrong. Luckily, most of the merge conflicts were in things where, if I got it wrong, the most likely outcome is that the code won't compile (use statements, associate statements, and subroutine argument lists; the last of these tended to be the trickiest).

  4. Push this branch to your fork

  5. Open a new Pull Request from this branch

  6. Carefully review the differences in this branch relative to master. This is largely to make sure that I did all of the migration of these commits to master correctly. You can compare the diffs in (a) new branch vs. master (your new PR that you'll open) vs. (b) old branch vs. release-clm5.0 (this PR).

  7. In addition, do one or more simulations (at least short simulations) with the code off of master to make sure it is behaving as expected - again, to confirm that everything was migrated to master correctly.

If you'd like, you can do steps (6) & (7) before step (5); I am partly recommending the above order because opening a PR can be a convenient way to examine the diffs. Either way is fine with me.

In reviewing the diffs, pay particular attention to these files, for which there were git conflicts, but you should really look at everything because there may have been more subtle conflicts that git didn't pick up:

  • src/biogeophys/TotalWaterAndHeatMod.F90 (conflict in da90b2f, which is now c7566b2; conflict in 99b5558, which is now 60c0aaf; conflict in 2b7b6a5, which is now 2ea9eee; conflict in ceaa825, which is now c55c2a5)
  • tools/mksurfdata_map/src/mksurfdat.F90 (conflict in 3738c46, which is now 8e45295)
  • src/main/clm_initializeMod.F90 (conflict in a6fd2b5, which is now d7296c8; conflict in f9318cc, which is now bada0c9)
  • src/main/surfrdMod.F90 (conflict in a6fd2b5, which is now d7296c8)
  • src/biogeophys/BalanceCheckMod.F90 (conflict in 2b7b6a5, which is now 2ea9eee)
  • src/biogeophys/LakeHydrologyMod.F90 (conflict in 2b7b6a5, which is now 2ea9eee)
  • src/dyn_subgrid/dynConsBiogeophysMod.F90 (conflict in 2b7b6a5, which is now 2ea9eee; conflict in ceaa825, which is now c55c2a5) (MAJOR CONFLICTS IN THIS FILE - though mainly in argument lists (so hopefully will be caught by compiler)!)
  • src/dyn_subgrid/dynSubgridDriverMod.F90 (conflict in 2b7b6a5, which is now 2ea9eee)
  • src/main/clm_driver.F90 (conflict in 2b7b6a5, which is now 2ea9eee)

@billsacks
Copy link
Member

Closing this because, as noted above, a new PR is needed into master rather than the release branch.

@billsacks billsacks closed this Jun 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants