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

mksurfdata: Rework mapping of landunits to handle coastal areas more rigorously #1920

Merged

Conversation

billsacks
Copy link
Member

@billsacks billsacks commented Dec 2, 2022

Description of changes

This PR reworks the mapping of landunit areas and the determination of wetland areas according to the plan laid out in #1716. (Note that the final implementation differs from the initial comment in #1716, as discussed in follow-up comments in that issue.) See the discussion there as well as the design document in this PR for details.

Specific notes

Contributors other than yourself, if any: Various discussions with others, especially @ekluzek and @slevisconsulting

CTSM Issues Fixed (include github issue #):

Are answers expected to change (and if so in what way)? Areas will change on surface datasets

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

Testing performed, if any:

(1) Compared 1850 f09 surface dataset before and after these changes (and after each individual change from the individual commits on this branch)

(2) By hard-coding landunit areas before the call to normalize_and_check_landuse, I performed various "manual unit tests" of the normalization code, using diffs like this:

diff --git a/tools/mksurfdata_esmf/src/mksurfdata.F90 b/tools/mksurfdata_esmf/src/mksurfdata.F90
index eb91be30c..0f347f8c0 100644
--- a/tools/mksurfdata_esmf/src/mksurfdata.F90
+++ b/tools/mksurfdata_esmf/src/mksurfdata.F90
@@ -593,6 +593,17 @@ program mksurfdata
   ! Perform other normalizations
   ! -----------------------------------
 
+  do n = 1, lsize_o
+     pctlnd_pft(n) = 0._r8
+
+     call pctnatpft(n)%set_pct_l2g(0._r8)
+     call pctcft(n)%set_pct_l2g(0._r8)
+     pctlak(n) = 0._r8
+     pcturb(n) = 0._r8
+     pctgla(n) = 0._r8
+     pctwet(n) = 0._r8
+  end do
+
   ! Normalize land use and make sure things add up to 100% as well as
   ! checking that things are as they should be.
   call normalize_and_check_landuse(lsize_o)

In the following, any landunit not explicitly mentioned had 0% area:

(a) With pctlnd_pft = 0% and all initial landunit areas 0%: wetland area = 100%

(b) With pctlnd_pft = 0% and initial glacier area 1%: glacier area = 100%

(c) With pctlnd_pft > 0% and all initial landunit areas 0%: natural vegetated area = 100%

(d) With pctlnd_pft = 40%, initial crop area 20%, natural vegetated area 10%: crop area = 50%, natural vegetated area = 50%

(e) With pctlnd_pft = 40%, initial crop area 20%, natural vegetated area 10%, glacier area 10%: crop area = 50%, natural vegetated area = 25%, glacier area = 25%

(f) With pctlnd_pft = 40%, initial crop area 20%, natural vegetated area 10%, glacier area 15%: crop area = 50%, natural vegetated area = 12.5%, glacier area = 37.5%

(g) With pctlnd_pft = 40%, initial crop area 20%, natural vegetated area 10%, glacier area 20%: crop area = 50%, glacier area = 50%

(h) With pctlnd_pft = 40%, initial crop area 20%, natural vegetated area 10%, glacier area 30%: crop area = 40%, glacier area = 60%

(i) With pctlnd_pft = 40%, initial crop area 0%, natural vegetated area 40%, glacier area 40%: glacier area = 100%

(j) With pctlnd_pft = 2%, initial natural vegetated area 1%, glacier area 1%: natural vegetated area = 50%, glacier area = 50%

(k) With pctlnd_pft = 2%, initial natural vegetated area 0%, glacier area 1%: natural vegetated area = 50%, glacier area = 50%

(l) With pctlnd_pft = 2%, initial natural vegetated area 2%, glacier area 1%: natural vegetated area = 50%, glacier area = 50%

This will allow switching between FRACAREA and DSTAREA normalization;
we'll use the latter for PCT areas.
Do some rework of adjustments now that we are mapping pctnatveg and
pctcrop with DSTAREA rather than FRACAREA normalization - so they end up
as percent of the grid cell rather than percent of the land area. The
end result won't be identical in all cases, but in practice it should do
the same thing as before.

- Zero out pctnatveg and pctcrop if their areas are less than 1e-6,
rather than the previous logic of zeroing these if pctlnd is less than
1e-6. (This zeroing is now done in mkpft rather than in the main
mksurfdata program.)

- In deciding whether to set pct_nat_pft or pct_cft to a fixed value,
only check for tiny values of pctnatveg / pctcrop, rather than checking
for a tiny value of pctlnd (it should now be the case that a tiny value
of pctlnd implies tiny values of pctnatveg and pctcrop as well).

- Remove the check for no vegetation outside the pft mask. This check no
longer seems important and risks doing more harm than good if the
threshold for this check differs slightly from the threshold used to
zero pctnatveg / pctcrop.
This rework makes things work correctly given that the landunit areas
coming into normalize_and_check_landuse are in % of grid cell, but the
outputs are in % of land area.

Addresses much of the remainder of ESCOMP#1716. Especially see
ESCOMP#1716 (comment)
@billsacks billsacks added this to the ctsm5.2.0 milestone Dec 2, 2022
@billsacks
Copy link
Member Author

@ekluzek @slevisconsulting - I just pushed a somewhat-related change to this branch, where I have removed the code that hard-codes the pole points to glacier. I never liked that we had this code - e.g., this could trip someone up if they were trying to create a surface dataset for a very different climate / continental configuration - and it was leading to weirdness in the landfrac at the north pole (setting that landfrac to 1, which wasn't a problem per se, but was weird). I looked at diffs in an f09 surface dataset from removing it, and the only diff is as expected: the north pole is now classified as wetland rather than glacier; there is no change at the south pole (which is still classified as glacier).

Please let me know if you can think of any reason why this hard-coding at the poles should be retained.

@billsacks
Copy link
Member Author

I have looked at the difference in LANDFRAC_MKSURFDATA for an f09 transient dataset, between 1850 and 2005. There are only 8 grid cells that have any difference in LANDFRAC_MKSURFDATA between years 1850 and 2005. Of these, only 1 has a difference exceeding 1e-4: one grid cell has a difference of 1.45e-3; this is a grid cell at the mouth of the Mediterranean Sea. This is good: there are NOT big differences in our estimated land fraction in a transient case.

@ekluzek and @slevisconsulting - at this point I feel like this is ready to be merged if you're satisfied with it.

@slevis-lmwg
Copy link
Contributor

@ekluzek and @slevisconsulting - at this point I feel like this is ready to be merged if you're satisfied with it.

@billsacks I'm looking through the code and will respond soon.

Copy link
Contributor

@slevis-lmwg slevis-lmwg left a comment

Choose a reason for hiding this comment

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

@billsacks thank you for your careful work in this PR! Looks good to me. I'm clicking "approve" and will let you and @ekluzek decide whether you also need his approval.

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 5, 2022

@billsacks the change you made on the poles in general makes sense to me. Although it does seem a bit weird if the north pole is now wetland rather than glacier. I'm just wondering if you found some information in git logs (or elsewhere) about why this was done way back in the day? That context can be really helpful in figuring out if we should retain a feature we now question.

@billsacks
Copy link
Member Author

@ekluzek I just checked the history and this adjustment of the pole points dates all the way back to the initial version in the CLM svn repository. If I had to guess, my guess would be that it was needed at one time due to problems with mapping to the poles.

Why do you think it's weird if the north pole is now wetland? That is consistent with the raw datasets, where the north pole looks like ocean and isn't claimed by any landunit. This makes the north pole end up with the same land cover as the latitudes slightly south of the north pole, which seems more correct and consistent to me.

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Dec 5, 2022

[...] That is consistent with the raw datasets, where the north pole looks like ocean and isn't claimed by any landunit. This makes the north pole end up with the same land cover as the latitudes slightly south of the north pole, which seems more correct and consistent to me.

Am I right to also conclude that, if a user put a continent over the North Pole with evergreen trees, then the NP would be land with evergreen trees? (I.e. neither glacier nor wetland.)

@billsacks
Copy link
Member Author

Am I right to also conclude that, if a user put a continent over the North Pole with evergreen trees, then the NP would be land with evergreen trees? (I.e. neither glacier nor wetland.)

With the new code, yes: the north pole should end up with whatever is specified on the raw datasets. With the old code, the north pole was forced to glacier regardless of what was specified on the raw datasets.

@ekluzek
Copy link
Collaborator

ekluzek commented Dec 5, 2022

Ahh, OK, thanks for that clarification. I wasn't clear what this really looked like. It sounds like you used to have a completely isolated glacier point around the North pole that was disconnected to anything, the new method does make more sense. We did used to have more issues with remapping, so it makes sense it had something to do with that.

@billsacks
Copy link
Member Author

billsacks commented Dec 5, 2022

It sounds like you used to have a completely isolated glacier point around the North pole that was disconnected to anything

Yes, exactly. Here's what PCT_GLACIER looked like before my recent change: notice the red band (100% glacier) in the far northern latitude band:

Screen Shot 2022-12-05 at 11 43 09 AM

(In practice this probably didn't matter, because 90° North would typically be ocean, but if it were ever land for some reason, it would be classified differently from the nearby grid cells.)

@slevis-lmwg
Copy link
Contributor

I talked with @ekluzek this morning and we agreed:

  • He will look over this PR before it gets merged.
  • Once ready, do you @billsacks have a preference for who does the merging/tagging? I think any of the three of us could do it.

@billsacks
Copy link
Member Author

  • Once ready, do you have a preference for who does the merging/tagging? I think any of the three of us could do it.

Thanks. No preference.

@billsacks billsacks self-assigned this Dec 12, 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.

OK, I have a few questions and comments. And mostly I wanted to make sure I understood this a little better by looking through it.

The general idea is that the routehandles now normally are normalized by fractional area, and if they are NOT they have "_nonorm" at the end to say they aren't.

There's also a bug that you caught that made the North pole glacier, which is really good to catch. I'm thinking that would be good to make into an issue for visibility across the project.

tools/mksurfdata_esmf/src/mklanwatMod.F90 Show resolved Hide resolved
tools/mksurfdata_esmf/src/mksurfdata.F90 Show resolved Hide resolved
tools/mksurfdata_esmf/src/mksurfdata.F90 Show resolved Hide resolved
tools/mksurfdata_esmf/src/mksurfdata.F90 Show resolved Hide resolved
tools/mksurfdata_esmf/src/mksurfdata.F90 Show resolved Hide resolved
@billsacks
Copy link
Member Author

The general idea is that the routehandles now normally are normalized by fractional area, and if they are NOT they have "_nonorm" at the end to say they aren't.

There's also a bug that you caught that made the North pole glacier, which is really good to catch. I'm thinking that would be good to make into an issue for visibility across the project.

Yes, thanks for the summary. I just opened #1930 and marked it as resolved in the initial comment in this PR. In general, raw datasets on fractional area are mapped without normalization, whereas most parameters are mapped with normalization.

@ekluzek
Copy link
Collaborator

ekluzek commented Jan 12, 2023

I'm going to go ahead and merge this in.

@ekluzek ekluzek merged commit 877efb2 into ESCOMP:ctsm5.2.mksurfdata Jan 12, 2023
slevis-lmwg added a commit to slevis-lmwg/ctsm that referenced this pull request Jan 25, 2023
mksurfdata: Rework mapping of landunits to handle coastal areas more rigorously

Description of changes
This PR reworks the mapping of landunit areas and the determination of wetland areas according to the plan laid out in ESCOMP#1716. (Note that the final implementation differs from the initial comment in ESCOMP#1716, as discussed in follow-up comments in that issue.) See the discussion there as well as the design document in this PR for details.

ESCOMP#1920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants