Fates two stream API #2265
Fates two stream API #2265
Conversation
ekluzek
added
enhancement
There was a problem hiding this comment.
OK, it looks like the guts of this is just in FATES. These changes are small and moving from a specific ED interface to a generic FATES one. So there's nothing to really see here....
It adds an important capability that we want to bring in right away.
From looking this over I'm guessing you change it from Norman to 2-stream in the FATES parameter file? Is that right? If so we should add some tests for this similar to the seed dispersal tests that modify the FATES parameter file. We should have at least one if not a few tests that run with two stream.
|
Thanks @ekluzek. Just on the tests, it's likely that w will shift to twostream being the default asap, and probably advise against using the Norman scheme, so maybe we don't need to have a test proliferation in the long run... |
Ah, OK so 2-stream is to become the default, and Norman will be deprecated. That's useful to know. I still assume that you want both to work. And certainly now want 2-stream to work and be tested until it does become the default. Sometimes we think those transitions will happen more quickly then they actually do. So having at least one test during the transition seems critical to me. In the longer run I'm also betting you still want to keep Norman around and ensure it works. First as an option to go back to if 2-stream has a problem. But, second is likely you'll want to reproduce results of previous FATES versions to compare to. So you might want to keep it around longer than you think. And if so you'll want to keep some testing for it. Anything that doesn't continue to get tested is going to get broken is a reliable axiom. But, if you don't care about it much, you can just reverse the testing so that everything is 2-stream, and you have one Norman. Verses in the transition you maybe most of the tests are Norman because that's been the standard, but if 2-stream is becoming the standard you do want to test it enough to not break it. |
|
The other suggestion if 2-stream is to become the standard is try running the FATES test suite tests with 2-stream as the default to see if that brings up any problems. The test suite might show a problem in an obscure configuration that you won't see without running it that way. |
|
Good point on the backwards compatibility. I hadn't thought of that. But I guess most of the tests should be using twostream in the nearish future with a smaller set for the Norman? Anyway, this isn't really my thing, so I should probably be quiet ;) |
|
Yes, based on this we'll want most tests with 2-stream, but some at least for Norman. But, @rosiealice there is an important partnership between science and SE in regard to testing. You don't need to know some of the finer details about the test list. But, we need to know that the test list is testing all the important science options for the code. If something isn't tested I'm going to want to argue that we remove it as an option. But, the same is true if we are testing something that isn't scientifically important -- it might be a good candidate to remove from the testing as well as from the code. What we test regularly is the true list of what we expect to work we should all agree that the list is correct. If we aren't testing something important it'll end up broken and take more time to fix. But, if we are testing something NOT important we are wasting resources. So getting that list right requires us working together... |
Adds capability for cropland soil tillage and post-harvest residue removal. Tillage: This PR brings in the tillage code written by Sam Levis and Michael Graham and used in Graham et al. (2021, ERL, doi:10.1088/1748-9326/abe6c6). Low- and high-intensity tillage here work by increasing the decomposition rate of different soil carbon pools. These "decomposition multipliers" vary based on soil pool and how long it's been since the crop was planted; they are set with new paramfile variables till_decompk_multipliers and mimics_till_decompk_multipliers. Note that tillage is off by default. Residue removal: Adds a parameter hat represents what fraction of post-harvest crop residues (stems and leaves) should be removed to the crop product pool rather than being transferred to litter.
|
After running the fates test suite, things are nominal, except I'm generating small diffs with the base on this test: SMS_Ld3.f09_g17.I2000Clm51FatesSpCruRsGs.derecho_intel.clm-FatesColdSatPhen_prescribed The diffs are very odd. When I plot variables that are flagged in the cprnc.out file, I can't see any visible differences in the maps on the day of interest with ncview. It is a very dense map (for fates standards) so perhaps its just an issue that my eye can't identify the rogue cell with the diffs. Along with diffs in variables for soil moisture and gpp, I'm also seeing a difference in fates_lai, which is dictated by the satphen, so I just don't see how the changes to the radiation scheme could possibly generate this difference... |
|
Since there is evidence that SMS_Ld3.f09_g17.I2000Clm51FatesSpCruRsGs.derecho_intel.clm-FatesColdSatPhen_prescribed has uninitialized variables, I re-ran the test with the gnu compiler and debug flags. This combination checks and fails with unitiatlized variables. I was able to trigger the failure, in both this branch and with dev163 (base). See #2321. Note with the base I also found initialized errors in the dust module, see log here: In light of this, I think we should:
|
|
Test output:
|
|
The FATES test list wasn't run on izumi. So @rgknox is going to run a baseline for ctsm5.1.dev163 and for this update. In the meantime I'm going to go ahead and make the tag as it is. If we see any issues we'll document them as issues to be fixed later, since the testing on Derecho was fine. We'll also talk about if we should run the izumi FATES test list for all tags with fates updates tomorrow.... |
Description of changes
This set of changes supports the addition of two-stream radiation in fates. The changes are mostly cosmetic. Radiation drivers in fates have been re-named to use FATES naming conventions instead of ED, and they no longer imply only Norman radiation.
Specific notes
Contributors other than yourself, if any: On the FATES side of code: @mpaiao @adrifoster @rosiealice @ckoven @JessicaNeedham Gordon Bonan and Jennifer Kowalczyk, to name a few
This should be synchronized with: NGEET/fates#1141
CTSM Issues Fixed (include github issue #):
Fixes #2305
Are answers expected to change (and if so in what way)? Some FATES-specific radiation diagnostics have been changed, removed or added.
Any User Interface Changes (namelist or namelist defaults changes)? No, a fates parameter file switch fates_rad_model is already in the parameter file, it had previously been inconsequential.
Testing performed, if any:
I've run numerous multi-decade smoke tests, site level analysis and comparisons have been made with ilamb.
TBD