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

FATES land use v2 API update (CTSM-side) #2507

Merged
merged 131 commits into from
Jul 18, 2024

Conversation

samsrabin
Copy link
Collaborator

@samsrabin samsrabin commented May 1, 2024

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.

  • New use_fates_lupft convenience option (use_fates_luh + use_fates_nocomp + use_fates_fixedbiogeog)
  • use_fates_logging refactored into fates_harvest_mode with the addition of two new harvest modes

Testing performed, if any: In progress with development.

@samsrabin samsrabin added FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM PR status: work in progress labels May 1, 2024
@samsrabin samsrabin self-assigned this May 1, 2024
@wwieder wwieder added this to the 2024 CESM June workshop milestone May 2, 2024
@ekluzek
Copy link
Collaborator

ekluzek commented Jul 12, 2024

@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:

https://github.com/ESCOMP/CTSM/wiki/Protocols-on-updating-FATES-within-CTSM#fates-updates-that-include-api-changes

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

  1. Review the final CTSM changes again myself to note for any glaring problems that should kick it back for more testing
  2. Make sure .gitmodules is pointing to a NGEET FATES tag and not a personal branch/hash
  3. Make sure the testing baselines were run and have standard names and are on: izumi, and Derecho for both aux_clm and fates tests
  4. Double check test results are as expected (looking at the their test cases, found either by them giving the directory names in the PR or by looking at the tail of CaseDocs/lnd_in for the directory name in the a test for the baseline)
  5. Double check that the fates version used for the baselines is the same as in .gitmodules for the PR
  6. Review the ChangeLog (maybe do simple updates for clarity or have author update if really needed)
  7. Update the date in the ChangeLog if a day or more has passed (commit and push it to the PR branch)

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).

@glemieux
Copy link
Collaborator

@samsrabin @ekluzek aux_clm testing against ctsm5.2.011 on derecho is underway.

I'm going to see if I can get things going on izumi.

@glemieux
Copy link
Collaborator

glemieux commented Jul 13, 2024

@ekluzek @samsrabin Regression testing on derecho is complete and shows B4B results against ctsm5.2.011 for all non-fates tests with one exception. The RXCROPMATURITYSKIPGEN_Ld1097.f10_f10_mg37.IHistClm50BgcCrop.derecho_intel.clm-cropMonthOutput test failed RUN again with the error that I saw last time:

161 2024-07-12 18:47:39: ERROR: Command /glade/u/home/glemieux/ctsm/bld/build-namelist failed rc=255
162 out=
163 err=ERROR : CLM build-namelist::CLMBuildNamelist::process_namelist_commandline_infile() : Invalid namelist variable in '-infile' /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr2507-aux_clm-final/RXCROPMATURITYSKIPGEN_Ld1097.f10_f10_mg37.IHistClm50BgcCrop.derecho_intel.clm-cropMonthOutput.GC.pr2507-aux_clm-final_int.gddgen/Buildconf/clmconf/namelist.
164  ERROR: in validate_variable_value (package Build::Namelist): Variable name fsurdat has a string element that is too long: '/glade/u/home/glemieux/scratch/ctsm-tests/tests_pr2507-aux_clm-final/RXCROPMATURITYSKIPGEN_Ld1097.f10_f10_mg37.IHistClm50BgcCrop.derecho_intel.clm-cropMonthOutput.GC.pr2507-aux_clm-final_int.gddgen/surfdata_10x15_hist_1850_78pfts_c240216.all_crops_everywhere.nc'

That said, I didn't see an issue for this. Is it a known issue?

@samsrabin
Copy link
Collaborator Author

@glemieux Looks like it's the same error as in #2322. Don't worry about it; I have this working in my current dev branch, and I strongly doubt this PR does anything to break it.

@ekluzek
Copy link
Collaborator

ekluzek commented Jul 15, 2024

@samsrabin and @glemieux maybe the only thing to do about that test is to mark it as an expected fail for this tag? Especially since it will likely be a bit before the fix can get in (based on the queue of tags).

@samsrabin
Copy link
Collaborator Author

@glemieux No, it's not always an expected fail. I think Greg's use of a manual name for his test suite run is the culprit here, because it's usually okay.

@glemieux
Copy link
Collaborator

Testing on izumi is is largely B4B with DIFFS only showing up for fates testmods as expected. That said there was a floating invalid exception caught due to a fates-side issue that I've recorded in NGEET/fates#1221. I've got a fix that I'll turn into a fates PR this morning. I'll rerun all tests with the update post fates merge.

@glemieux
Copy link
Collaborator

Status update: running aux_clm regression testing on izumi and derecho

@glemieux
Copy link
Collaborator

glemieux commented Jul 17, 2024

Regression testing aux_clm on izumi against ctsm5.0.12 is nearly complete. I'm just waiting on the 20 year crop testmod. All other expected tests are B4B aside from the expected DIFFs from the fates testmods.

Location: /scratch/cluster/glemieux/ctsm-tests/tests_0717-144328iz

UPDATE: the last testmod, the long Ly20 cropmonthly test, came back b4b.

@glemieux
Copy link
Collaborator

Regression testing of aux_clm on derecho finished up over night. All non-fates tests are B4B. Fates test DIFFs are expected as the update incorporates a number of other fates-side answer changes. There were a few TPUTCOMP "fails" although these are all on the clm-side of things.

Location: /glade/derecho/scratch/glemieux/ctsm-tests/tests_0717-151608de

@slevis-lmwg slevis-lmwg added PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete and removed PR status: work in progress labels Jul 18, 2024
@adrifoster adrifoster merged commit a784f41 into ESCOMP:master Jul 18, 2024
2 checks passed
@ckoven
Copy link
Contributor

ckoven commented Jul 18, 2024

Woot! Thanks @glemieux @samsrabin @ekluzek @adrifoster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete
Projects
Status: Ready to eat (Done!)
Status: Done (non release/external)
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants