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. |
glemieux
moved this from Hold
to Final Testing
in FATES Pull Request Planning and Status
…b bug on cbalance errors
This gives access to ncgen to enable on-the-fly generation of the fates parameter file
This namelist option was brought in with API 38
glemieux
force-pushed
the
fates_nbp_totecosysc
branch
from
288ff0e to
5f1e4e4
Compare
|
FYI @peterdschwartz, final testing on perlmutter is underway. |
glemieux
moved this from Final Testing
to Ready to Integrate
in FATES Pull Request Planning and Status
|
@peterdschwartz regression testing against Results: |
|
thanks, this may have to wait till maint3.1 branch is created. |
|
If non-FATES case are BFB, it can go in. |
|
Tested this merged to next and got FATES only diffs: |
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. Note 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) [non-BFB] for FATES only
|
merged to next |
|
merged to master |
7 participants
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.
Note 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)
[non-BFB] for FATES only