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

Update mksurfdata_map to include glaciers outside of pft landmask #564

Merged
merged 2 commits into from
May 1, 2020

Conversation

lawrencepj1
Copy link

@lawrencepj1 lawrencepj1 commented Nov 8, 2018

… as well as lakes and wetlands outside of the landmask

with new code:

<        ! Assume wetland, glacier and/or lake when dataset landmask implies ocean

<           if (pctgla(n) < 1.e-6_r8) then
<               pctwet(n)    = 100._r8 - pctlak(n)
<               pctgla(n)    = 0._r8
<           else
<               pctwet(n)    = 100._r8 - pctgla(n) - pctlak(n)
<           end if

from

>        ! Assume wetland and/or lake when dataset landmask implies ocean

>           pctwet(n)        = 100._r8 - pctlak(n)
>           pctgla(n)        = 0._r8

Description of changes

Specific notes

Contributors other than yourself, if any:

CTSM Issues Fixed (include github issue #):
Addresses #545 (fixing it will require new surface datasets generated with this code)

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

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

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 gnu/pgi and hobart for gnu/pgi/nag is the standard for tags on master)

NOTE: Be sure to check your Coding style against the standard:
https://github.com/ESCOMP/ctsm/wiki/CTSM-coding-guidelines

…as well as lakes and wetlands outside of the landmask

with new code:

<        ! Assume wetland, glacier and/or lake when dataset landmask implies ocean

<           if (pctgla(n) < 1.e-6_r8) then
<               pctwet(n)    = 100._r8 - pctlak(n)
<               pctgla(n)    = 0._r8
<           else
<               pctwet(n)    = 100._r8 - pctgla(n) - pctlak(n)
<           end if

from

>        ! Assume wetland and/or lake when dataset landmask implies ocean

>           pctwet(n)        = 100._r8 - pctlak(n)
>           pctgla(n)        = 0._r8
@billsacks billsacks changed the title The logic of mksurfdata_map has been updated to include the glaciers… Update mksurfdata_map to include glaciers outside of pft landmask Nov 9, 2018
@billsacks billsacks self-requested a review November 9, 2018 12:02
Copy link
Member

@billsacks billsacks left a comment

Choose a reason for hiding this comment

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

Thanks, @lawrencepj1 !

@ekluzek ekluzek added priority: high High priority task to fix soon, e.g., because it is a problem in important configurations type: enhance - science PR status: ready PR: author feels this is ready to merge in labels Nov 9, 2018
@ekluzek
Copy link
Contributor

ekluzek commented Nov 9, 2018

@billsacks and I discussed this and realized we want this on master, but NOT on the release branch. We'll probably create datasets on master, that we then move to the release branch (for future scenarios), but we want to make sure you can create the old style datasets on the release branch.

I thought this PR might've been to go to the release branch, but it is correctly being sent to master.

@billsacks
Copy link
Member

Update: Based on today's co-chairs discussion, we actually want this on both master and the release branch. We want both the updated mksurfdata_map and all new surface datasets to come out-of-the-box starting in CESM2.1.1. To be clear: Starting with CESM2.1.1, all CTSM surface datasets should have this bug fix, and so should have glaciers rather than wetlands over the Antarctic ice shelves. This will mean that CESM2.1.1 changes answers relative to CESM2.1.0, and that is deemed acceptable. (We should just carefully check the new surface datasets against the old to ensure that the only thing that changes is this one aspect.)

In discussion over lunch, @ekluzek and I agreed that he would make this change in time for the CESM2.1.1 release. The earlier the better, probably, in terms of supporting upcoming cmip6 runs out-of-the-box, but there isn't a critical need for it before that release.

@billsacks billsacks added this to the cesm2.1.1 milestone Dec 4, 2018
@ekluzek ekluzek changed the base branch from master to release-clm5.0 December 10, 2018 21:28
@ekluzek ekluzek changed the base branch from release-clm5.0 to master December 10, 2018 21:28
@ekluzek
Copy link
Contributor

ekluzek commented Dec 10, 2018

I'm going to put this on the release branch first and then move to master.

@ekluzek
Copy link
Contributor

ekluzek commented Jan 15, 2019

I've put this on the release branch.

@billsacks billsacks added type: enhancement new capability or improved behavior of existing capability and removed type: enhance - science labels May 24, 2019
@billsacks billsacks added this to To do in High priority Jul 10, 2019
@billsacks billsacks moved this from To do to In progress in High priority Jul 10, 2019
@billsacks billsacks removed this from In progress in High priority Jul 10, 2019
@billsacks billsacks added this to In progress in High priority via automation Nov 4, 2019
@billsacks billsacks moved this from In progress to Done but not on master in High priority Nov 4, 2019
@ekluzek ekluzek merged commit 92057e3 into ESCOMP:master May 1, 2020
High priority automation moved this from Done but not on master to Done May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR status: ready PR: author feels this is ready to merge in priority: high High priority task to fix soon, e.g., because it is a problem in important configurations type: enhancement new capability or improved behavior of existing capability
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants