Add NBP, TOTECOSYSC, and related summary variables for FATES configurations #7231
Conversation
|
instead of a "Do Not Merge" label, this can be made a draft PR. Its been converted. |
|
|
||
| if (use_fates) then | ||
| call alm_fates%wrap_FatesAtmosphericCarbonFluxes(bounds, num_soilc, filter_soilc) | ||
| call alm_fates%wrap_FatesCarbonStocks(bounds, num_soilc, filter_soilc) |
There was a problem hiding this comment.
@ckoven This call to wrap_FatesCarbonStocks over-writes the values for totecosysc (and others), which are filled above in col_cs_Summary(col_cs,bounds, num_soilc, filter_soilc). We should make sure we aren't filling the same variable in two different places (at least for similar indices). Option 1 is to use the col%is_fates filter to only update values in col_cs_Summary for non fates indices, and set to NaN for fates indices. Option 2, which I like more, is to have totecosysc only updated in col_cs_Summary, but have optional logic for FATES columns...
There was a problem hiding this comment.
I gave it a go of refactoring this, but ran into circular dependencies. I don't want the perfect to get in the way of the good, so I think this is fine.
glemieux
force-pushed
the
fates_nbp_totecosysc
branch
from
d5ddd96 to
bb4287c
Compare
|
This branch has been brought up to date with API 40 by rebasing against |
|
@ckoven @rgknox I've updated this PR to use fates tag |
|
Needs approval from a reviewer. |
glemieux
moved this from Under Review
to Final Testing
in FATES Pull Request Planning and Status
glemieux
moved this from Final Testing
to Stuck
in FATES Pull Request Planning and Status
|
Per FATES software development meeting today, @ckoven noted that we should hold off on testing this due to a potential bug in the FATES-side NBP diagnostic. |
I've sorted out the issues I was having, they were unique to the specific branch I was using and so not relevant to this PR branch, so please proceed. |
glemieux
moved this from Stuck
to Final Testing
in FATES Pull Request Planning and Status
glemieux
force-pushed
the
fates_nbp_totecosysc
branch
from
0884e6e to
afce6b1
Compare
|
During regression testing I found an issue in which |
glemieux
moved this from Final Testing
to Stuck
in FATES Pull Request Planning and Status
We currently have a fix in the works on the fates-side for the exact restart issue via NGEET/fates#1448. |
|
Rerunning regression tests on perlmutter with NGEET/fates#1448. |
…b bug on cbalance errors
glemieux
moved this from Hold
to Final Testing
in FATES Pull Request Planning and Status
glemieux
moved this from Final Testing
to Hold
in FATES Pull Request Planning and Status
glemieux
force-pushed
the
fates_nbp_totecosysc
branch
from
f70ef8c to
d5bbb15
Compare
|
FYI, @peterdschwartz this needs to be held until #7665 comes in. The fates tag will need to be rebased against master and point to a fates tag that was brought in in API41. |
|
Note that this will fix #7278. |
This gives access to ncgen to enable on-the-fly generation of the fates parameter file
|
This also fixes #7720 |
This namelist option was brought in with API 38
|
Is this ready? |
This PR adds a few new summary variables that include carbon information from FATES as well as soil biogeochemistry and wood product decay when FATES is active: NBP, NEE as fluxes, and TOTECOSYSC as stocks.
This depends on bc_out variables that are introduced in FATES PR NGEET/fates#1382.
Nopte that there are a couple of other bug fixes that I've added here, one of which is conceptually related (as it fixes balance checking when wood harvest is active in FATES), and one of which is unrelated (correctly defining history dimension information for a multiplexed land use by PFT dimension)