-
Notifications
You must be signed in to change notification settings - Fork 301
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
FATES land use v2 API update (CTSM-side) #2507
base: master
Are you sure you want to change the base?
Conversation
Manually cherry-picked from Greg Lemieux's b33a722.
this also temporarily removes the use_fates_lupft check for the use_fates_luh namelist defaults
Update build namelist checks for valid landuse v2 mode combinations
This pull request is now updated with the fates-side tag for land use v2. Regression testing against
The The |
Checking the landuse x pft data is handled on the fates-side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really great to see come in. Seeing this helps me to understand how all of this works. I think the instructions are getting more clear. And I like the array of possible options for fates_harvest_mode. I like the error checking being done and new tests. The code is using parameters to save integer settings needed which is great.
I'm marking as approved even though I ask for a few changes just to facilitate moving forward. There are a couple longer term things that I'll make into issues. I also setup a meeting to go over this with @glemieux to talk about some of this.
event_code: fates logging via fates logging event codes (see fates parameter file) only | ||
surfdata_file: fates harvest driven by CLM landuse timeseries data (dynHarvestMod) | ||
luhdata_area: fates harvest driven by LUH2 raw harvest data, area-based (dynFATESLandUseChangeMod) | ||
luhdata_mass: fates harvest driven by LUH2 raw harvest data, mass-based (dynFATESLandUseChangeMod) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the line. Deprecated is misspelled here, so that would be good to fix. I also wonder about adding asterisks after landuse_timeseries and before the note so it's obvious that both lines reference the landuse_timeseries option. Is event_code likely to be deprecated in the future as well?
event_code: fates logging via fates logging event codes (see fates parameter file) only | ||
surfdata_file: fates harvest driven by CLM landuse timeseries data (dynHarvestMod) | ||
luhdata_area: fates harvest driven by LUH2 raw harvest data, area-based (dynFATESLandUseChangeMod) | ||
luhdata_mass: fates harvest driven by LUH2 raw harvest data, mass-based (dynFATESLandUseChangeMod) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple further suggestions. Deprecated is misspelled in the update. Good to fix that.
It also seems to only apply to landuse_timeseries, and maybe event_code needs a similar comment?
I also wondered about adding astericks to the end of landuse_timeseries and then at the begining of the note regarding it -- so it's more obvious they are connected.
Pre-generating files is a more likely next step relative to auto-gen
@samsrabin @glemieux have been going over this extensively. Greg is going to do the testing for this. And then he'll pass it off for someone else to complete the tag. Since, @samrabin you are the author (and we wanted to cycle through FATES tags) we figure you should be the one to make the actual tag. This is just the final steps of the tagging process. So start at step 16 from: When I've done this myself I've typically do some double checking of the work. So I have done up to all of the following steps
The double checking has the intent of doing quick checking to make sure everything is good and we won't have problems later. I trust everyone on the project, but I also appreciate having my own work double checked to help prevent problems that become more involved to track down. FATES tags are also more involved than regular tags and having one person do the FATES tag/testing and another finalize the CTSM side has been a good workflow for us. Pinging @adrifoster as she'll be doing these final steps as well. I'm adding the above steps to a CTSM SE discussion so we can settle on what we all think should be required and what can be optional (I'm thinking require up to step 3, with the later ones optional). |
This needs to have use_fates_luh to avoid the false negative of failing due to not having this mode with use_fates_potentialveg
@samsrabin @ekluzek aux_clm testing against I'm going to see if I can get things going on izumi. |
@ekluzek @samsrabin Regression testing on derecho is complete and shows B4B results against
That said, I didn't see an issue for this. Is it a known issue? |
Description of changes
Update CTSM to work with FATES land use v2 (NGEET/fates#1116) .
Based on @glemieux's analogous update for E3SM (E3SM-Project/E3SM#6353).
Specific notes
This PR enables the host land model to read in a new landuse x pft static mapping dataset from the fates landuse data tool. A default output at a
4x5
resolution is provided.This update also includes a new ctsm-fates specific system test using the
PVT
prefix which provides for a 5 year spin-up in the new fates "potentival vegetation" mode the output of which is then used to start a fates landuse transient run using the landuse timeseries data (which was added back with ctsm5.1.dev160).The fates harvest and logging options have been refactored and simplified into a new option,
fates_harvest_mode
, to aid the user in selecting harvest modes compatible with other fates run modes. This includes two new modes that use the area or mass harvesting data from the fates LUH2 landuse timeseries data. A new convenience namelist option,use_fates_lupft
has also been provided for turning on fates landuse with no competition and fixed biogeography.Contributors other than yourself, if any: @glemieux, @ckoven
CTSM Issues Fixed (include github issue #): None
Are answers expected to change (and if so in what way)? Yes, but for fates testmods only
Any User Interface Changes (namelist or namelist defaults changes)? Yes.
use_fates_lupft
convenience option (use_fates_luh
+use_fates_nocomp
+use_fates_fixedbiogeog
)use_fates_logging
refactored intofates_harvest_mode
with the addition of two new harvest modesTesting performed, if any: In progress with development.