Update default FATES parameter file with arctic shrubs and understory allometry update #6639
Conversation
|
Is this still WIP ? |
@rljacob yes, but it should be ready for testing by Monday next week. Waiting on NGEET/fates#1255 to be integrated and tagged first. |
This update calls the fates parameter modification tooling to set the grass allometry smode to 1, which is the default for all other fates PFTs. This is a temporary workaround for NGEET/fates#1254.
glemieux
changed the title
|
@bishtgautam @peterdschwartz this is ready for review. Testing forthcoming. |
|
Regression testing with the elm_land_developer suite shows B4B results against Location: |
|
So, the fateshydro testmod isn't going to be used for any tests but is just being used for setting up cases with |
Good question. This is update to |
|
Ok, if it helps the current workflow, sure maybe this can stay for now. But this test and the fates twostream test will have to be changed to use parameter files stored on the input data server before they can be used as part of the test suites. To be honest, unless there's a compelling reason, I'd prefer it be changed now so that we don't have to remember to do it later. |
|
Since it's not strictly necessary at this time, I'm willing to take it out of this PR. That said, I'm wondering if we should start up a github discussion tracking how to plan for allowing on-the-fly parameter file builds. The only place we really discussed this on the E3SM side of things was in the context of https://github.com/E3SM-Project/E3SM/pull/6279/files#r1564414955 when this was first introduced. For background, we've had some discussion of providing this capability a while back via ESMCI/cime#2935. The main thing we're trying to enable by creating parameter files on-the-fly is to provide a way to test a variety of fates settings that are controlled by fates parameter file switches (that don't warrant a namelist setting) while reducing the number of default parameter files that the host land model has to update. |
|
Thank you for the links, and I do remember that comment discussion on that PR -- apologies for not thinking more about it then. I don't fully understand how @rgknox's workflow is being bottlenecked nor how defining the test this way solves the issue. From my current understanding, it seems like it should be sufficient to have a script run when you need to generate new parameter files (and that script can even timestamp everything correctly, add it to the server and change the defaults for you in xml files if you want) rather than every time we run the test suites. I think for our testing every change should be attributable to a specific Pull Request so the parameter file used by a test really should not be changing on a nightly basis. Edit: That's why I feel the script should be run as a part of your workflow rather than built into the test suite. If the concern is clutter (every programmer hates repetition!), I worry that would make it annoying to debug one of the parameter sets out of the 10. If the parameter differences are really significant enough to warrant testing each, then explicitly making them their own test makes it very simple to debug only the configuration of interest as well as tracking the changes in model behavior over time. Regardless, any solution will have to work on all the supported machines for the nightly testing. |
This reverts commit ed346d4.
|
@peterdschwartz , thanks for working through this with us. I like automatically generating changes to the parameter file, or at least having it scripted, because it removes the human error component. Right now we've only shined the light on a couple of cases where modifying parameter files on-the-fly would help us expand our testing coverage, but this could be greatly expanded and we could exercise so much more of the code. Hypothetically, this could be more than 10 different parameter file configurations. But we understand if the testing infrastructure makes this unreasonable. We could maintain a script to create the variants of the parameter file that is manually invoked when we update the base files configuration, and then load these files to the data server along with the corresponding file path name changes. This would maybe get dicey if we wanted to have many variants of the parameter file, but it would work for a handful of them. Sorry, I'm not sure I understand this comment: "If the parameter differences are really significant enough to warrant testing each, then explicitly making them their own test makes it very simple to debug only the configuration of interest as well as tracking the changes in model behavior over time". Our broader objective here is to test as much of the FATES code as reasonably possible, so yes, can only happen if we modify parameter constants. The important parameters are "switches" that select or turn on/off different modules. There are other ways we can approach this if automated python scripting is not possible. We could migrate these switches in the parameter file to the elm namelist (although that will probably confuse all of our users and it will be a stumbling block until they get used to it). In general we've thus far kept "major" (like whether or not to run with just carbon cycling or also include N and P) switches in the namelist, and "minor" switches (like choice of leaf respiration scheme) in the parameter file. To avoid confusion, we would need to migrate all switches to the namelist. And to further complicate this, in theory we don't want to modify only switches for testing purposes. And, if we started moving switches from the parameter file to the namelist, this would be a major FATES community decision... We also were wondering if maybe the auto modification tool could be compiled fortran or c (perhaps next to or as part of cprnc), so that we don't have to worry about conda. |
As far as I'm aware, the majority of the fates testmods that we have defined in the elm configuration are not run nightly. I believe the |
|
I believe I am on the same page as you two now. I do not have any concerns about the generating parameter files from the cdl file (So long as the config is clearly logged). I am assuming the cdl file is a part of the FATES repo and any changes to it will happen through a PR updating the FATES api (correct me if this is wrong). The question, in my mind, is whether it makes more sense to save the output of your scripts for later use or to recreate/delete them with every new case. How large are these parameter files and how long does it take to generate them? The main downside of storing them seems to be about cluttering and taking up too much space in the data server (anything else?). The upside would be that there's no added maintenance when setting up E3SM on a new machine or when updating existing machine configs. Writing in C may be the best approach here as E3SM requires a C compiler and netcdf-c already, and we can copy how cprnc auto-builds if not already present.
The concern here was if you are going to have a many-to-one correspondence between parameter files and testmods. If so, that could complicate debugging a specific configuration denoted by a specific parameter file and perhaps we would want a new kind of test to handle this functionality properly. So, are you planning on having only one parameter file per testmod?
But these tests (or at least a representative sample) should eventually make it into the nightly testing to ensure ELM only changes do not break compatibility with FATES. You all have done a pretty good job of laying the groundwork for the test coverage, and I have been assuming (hoping given how prominent FATES is becoming in ELM) that they will be included in our test suites when the features are ready. Thanks for walking me through this. Having a dedicated discussion page for this would be great to refer to. |
|
An alternate to To load modules that CIME loads: When the above command is run as test mods, one must iterate to get the relative path to |
That's correct, the CDL file is housed in the fates repo: https://github.com/NGEET/fates/tree/main/parameter_files
They are not very big (<100Kb), but they are pretty large from a human-editing perspective, so we provide the elm-fates user with some python-based tools to help modify them to minimize errors.
As long as we can automate and test the workflow for generating the parameter files, I think there wouldn't be really much other downside. I think there a bunch of ways we could automate this and I like your idea. This was just the most direct option taking advantage of pre-existing fates tooling for tests that @rgknox and I only run during development on perlmutter. The benefit that I see in driving the generation from the testmod shell_commands is that it tests the workflow of modifying the parameter file itself (in addition to the fates run mode) and as such provides a clear reference for the developer about what went into creating the parameter file specifically for this test.
The intent is to have 1:1 correspondence between a parameter file and testmod for the cases in which we have fates run mode switches that are set by the fates parameter file only. Two stream is a good example which is controlled by the fates_rad_model parameter. We have about 10 similar switches, some of which we will want to create more testmods for to expand coverage. That said, many of the fates testmods use the parameter file from the namelist defaults for those fates run modes that are controlled by ELMBuildNamelist.pm. So in that case, it is one stored param file to many testmods since what is being exercised is the namelist option and we work to make sure the default works for all those options. As an slight detour: the distinction between what in fates is controlled by the fates param file and what is controlled by the elm namelist is something we on the fates dev team reflect upon often, but my perspective is if the code associated with the fates run mode is entirely internal to fates, it should be in the fates parameter file.
💯 agree. I think planning for the staged inclusion of more fates tests into the nightly is something that I would ask for guidance on from @jenniferholm and @rgknox based on the current and future E3SM land phase plans. I'll fire up a discussion thread on this issue. |
|
Started up a discussion in the "feature request" section: #6681 |
Updates the FATES default parameter file to include improvements to arctic shrub pft represetation per NGEET/fates#1236 conducting per the NGEE-Arctic project. The parameter file update also includes improvements in understory survival rates per NGEET/fates#1136. [non-BFB] for fates
|
merged to next. |
|
merged to master -- should be more DIFFs on mappy tomorrow |
Updates the FATES default parameter file to include improvements to arctic shrub pft represetation per NGEET/fates#1236 conducting per the NGEE-Arctic project. The parameter file update also includes improvements in understory survival rates per NGEET/fates#1136.
[non-BFB] for fates