-
Notifications
You must be signed in to change notification settings - Fork 317
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
Convert wetlands to bare ground #1878
Comments
@ekluzek @wwieder @slevisconsulting - I have a few questions:
|
|
Awesome, thanks @ekluzek . The code changes should be pretty small and isolated to a subroutine that will only be invoked for the new option, so I think it won't be necessary to run full testing on these changes... I think just a subset of (working) tests should be fine. But if desired, I could temporarily rebase the changes onto master in order to run full testing; we can decide once I submit a PR with the changes. |
Don't worry about rebasing to master for a small change. We will get all the testing working on the branch, and can solve any small problems when that happens. |
I am on board with the above decisions. |
My only naive question is if this would just be a compset distinction, as I'm imagining it really only matters for coupled runs? |
I'm not sure I totally understand your question. Are you asking if we would want the behavior to differ for coupled vs uncoupled runs? We usually try to keep the behavior the same between coupled and uncoupled runs both to keep the CTSM operation more consistent in these two cases and to avoid surprises. So I was imagining that any run using CTSM52 or later – coupled or uncoupled – would use this new behavior, but you can let me know if you feel it's important for this new behavior to only be in place for coupled runs. |
I guess this is true, this issue is really just arising because of a mismatch in surface data used by the land and ocean models (and points where the ocean is assigning us land that we don't know anything about, and applying wetlands to)? If we're using our own internally consistent surface data in an uncoupled run this shouldn't really be an issue? More broadly, however, the 2nd and 3rd parts of our negative runoff fix in #1835 we only going to applied in coupled cases, but maybe this isn't accurate either? |
Even in an uncoupled run, we typically use the standard ocean mask so we end up with the same fractional cover. We wouldn't need to do this – and I think the standard for F compsets (AMIP-style runs) is to use a land mask that is unrelated to any ocean grid – but this is what's been done for I compsets for as long as I've been around. I think the rationale is that we want consistency between the operation of I compsets and that of fully-coupled runs. Maybe @ekluzek could provide more historical perspective. One situation to also consider is an I compset for the sake of spinning up CTSM with cplhist data (i.e., forcings from a previous B compset). It seems like we would want the land to work as similarly as possible in this case as it does in the equivalent B compset.
I haven't given this much thought, but for similar reasons, I imagined that we were going to do parts (2) and (3) for all runs, not just fully-coupled runs. But this definitely warrants discussion. Let's talk more at the ctsm-software meeting tomorrow. |
I am tentatively applying the conversion of wetlands to bare ground before the collapsing to N dominant landunits or the collapsing of too-small wetland areas. This intuitively feels more correct to me and also matches the behavior we would have gotten if we did the combination on the surface dataset itself, but others can let me know if you feel we should do the collapsing of wetland areas before converting wetlands to bare ground. |
Add option to convert wetlands to land; apply it by default for CLM51 physics From #1890 This PR adds a new namelist option, "convert_ocean_to_land", which converts all wetland area to natural veg (typically bare ground) at runtime. This is set to true by default when using CLM51 physics. In addition, this PR makes a minor change to the setting of soil properties in mksurfdata: Previously, soil properties were set to some default value outside of the pctlnd_pft-determined land mask. But I can't see any reason why that needs to be done, and doing so could remove some valid soil properties in the situation where there is a grid cell that has valid soil properties even though the pctpft dataset claims the area is ocean. Note that this PR is into the ctsm5.2.mksurfdata branch. Most of the changes could be applied directly to master, but there are some related changes to mksurfdata that motivated me to make this PR into the ctsm5.2.mksurfdata branch. However, standard f09 surface datasets don't show any changes due to the code changes in mksurfdata here, and the mksurfdata changes aren't critical anyway, so it would be reasonable to rebase the non-mksurfdata changes onto master if we want them available sooner. Resolves #1878 Are answers expected to change (and if so in what way)? Yes: changes answers for CLM51 cases in a few grid cells that used to be wetland but now are classified as bare ground. Any User Interface Changes (namelist or namelist defaults changes)? Adds new namelist variable, "convert_ocean_to_land" Testing performed, if any: - Ran the following tests with comparison against the ctsm5.2mksurfdata branch: - `ERP_D_P36x2_Ld3.f10_f10_mg37.I1850Clm50BgcCrop.cheyenne_intel.clm-default`: passes and bit-for-bit - `ERP_Ld9.f45_g37.I2000Clm51Bgc.cheyenne_intel.clm-default`: passes but changes answers, as expected - `ERI_D_Ld9.f10_f10_mg37.I1850Clm51Bgc.cheyenne_gnu.clm-default`: passes (didn't do baseline comparisons, but expect baseline comparisons to fail) - Also made new surface datasets with these changes in soil property mapping, 1850 f09: - Standard surface datasets show no differences - Also did a test where I temporarily set pctlnd_pft to 0 everywhere after the call to mkpft. I verified that this only leads to differences in various PCT fields: there are no longer any differences in soil properties arising from this change in pctlnd_pft. In contrast, setting pctlnd_pft to 0 before the changes in this PR leads to differences in many soil properties, because in this baseline version, soil properties were set to a constant value over grid cells outside the pctlnd_pft-based land mask.
To remove one source of negative runoff, we are planning to use bare ground in place of wetlands. (Note that wetlands are currently just used to represent ocean points: we currently don't run with inland wetlands.)
The original proposal was to change the surface datasets to specify bare ground rather than wetlands. However, I realized that that plan would make it impossible to do the rigorous treatment of coastal areas described in #1716 . So my plan is to add a flag that allows making this change at runtime. The other advantage of this approach is that this conversion can be turned on or off with the same surface datasets.
One other small change I'll make is to remove the code in mksurfdata that forces soil properties to hard-coded default values in wetland grid cells. This forcing seems unnecessary since soil properties are already given the same default values if no data are found for a given output cell; by removing this forcing for wetland grid cells, we will be able to use any existing soil data for a grid cell where soil properties are available despite the grid cell being considered ocean by the PFT data.
@ekluzek @wwieder @slevisconsulting
The text was updated successfully, but these errors were encountered: