Refactor and standardize per-ageclass history

[samsrabin]

Description:

As noted in Issue #1205, some per-ageclass history variables are weighted based on a patch's area (or a cohort's density) relative to age class area, whereas others are weighted based on that relative to total site area. To avoid confusion, this should be standardized. Discussion led us to decide on standardizing to the latter to avoid order-of-operations issues.

During development, I discovered and fixed two other bugs:

• FATES_VEGC_APPF was incorrectly not being updated for new cohorts (contributes to #1261).
• FATES_MORTALITY_CANOPY_SZAP and FATES_MORTALITY_USTORY_SZAP were incorrectly excluding termination and logging mortality.

Supersedes PR #1225, where I had made the opposite change (standardizing all to use per-ageclass area).

Collaborators:

@ckoven, @rgknox

Expectation of Answer Changes:

These variables are changed to be weighted relative to site area (rather than age-class area):

• FATES_GPP_AP
• FATES_LAI_AP
• FATES_NCL_AP
• FATES_NPP_AP
• FATES_SCORCH_HEIGHT_APPF

These variables are changed to be weighted relative to site-wide canopy area (rather than age-class canopy area):

• FATES_LBLAYER_COND_AP
• FATES_STOMATAL_COND_AP

These variables are affected by bug fixes:

• FATES_MORTALITY_CANOPY_SZAP (logging and termination mortality now included)
• FATES_MORTALITY_USTORY_SZAP (logging and termination mortality now included)
• FATES_VEGC_APPF (see #1261)

Other per-ageclass variables see roundoff-level changes as a result of refactoring (absolute RMSE < 1e-12, relative RMSE < 1e-13). In my testing (SMS_Lm49.f10_f10_mg37.I2000Clm60Fates.derecho_intel.clm-FatesColdAllVarsMonthly), the following were affected:

• FATES_BURNFRAC_AP
• FATES_DDBH_CANOPY_SZAP
• FATES_DDBH_USTORY_SZAP
• FATES_FIRE_INTENSITY_BURNFRAC_AP
• FATES_FUEL_AMOUNT_AP
• FATES_FUEL_AMOUNT_APFC
• FATES_MORTALITY_CFLUX_CANOPY
• FATES_MORTALITY_CFLUX_PF
• FATES_MORTALITY_CFLUX_USTORY
• FATES_NPLANT_CANOPY_SZAP
• FATES_NPLANT_SZAP
• FATES_NPLANT_SZAPPF
• FATES_NPLANT_USTORY_SZAP
• FATES_NPP_APPF
• FATES_SCORCH_HEIGHT_PF
• FATES_VEGC_AP

Answers from that test (at 1aff666) compared to baseline can be found here. Note that FATES_NPLANT_SZAPPF was not tested to see if it sums correctly across the age-class axis because I don't have a way to handle triplexed variables; however, it was only affected at roundoff level by this work.

Checklist

All checklist items must be checked to enable merging this pull request:

Contributor

• [x] The in-code documentation has been updated with descriptive comments
• [x] The documentation has been assessed to determine if updates are necessary: Yes

Integrator

• [ ] FATES PASS/FAIL regression tests were run
• [ ] Evaluation of test results for answer changes was performed and results provided

Documentation

• Technical Note update:
• User's Guide update:

Test Results:

CTSM (or) E3SM (specify which) test hash-tag:

CTSM (or) E3SM (specify which) baseline hash-tag:

FATES baseline hash-tag:

Test Output:

[samsrabin]

Just discovered some more per-ageclass variables to move out of update_history_dyn2().

[samsrabin]

This is ready for review. The bulk of the refactoring was moving the age-class variable calculations from update_history_dyn2() and update_history_hifrq2() to update_history_dyn2_ageclass() and update_history_hifrq2_ageclass(), respectively. However, I'm now thinking that it would be better for there to be a bunch of subroutines, each focused on a single theme (e.g. biomass, C fluxes, fire, etc.). The history update subroutines—especially update_history_dyn2()—are so huge that it's easy to lose track of where one is, which may have led to mistakes like #1261, where some variables were incorrectly in a very large if block.

That said, I wouldn't want to turn this PR into that. I would rather merge this as-is, then file an issue for someone to tackle the bigger refactor in the future.

[samsrabin]

• [x] Rename and add comments to update_history_dyn1() and update_history_dyn2() to indicate they're intended for site-level vs. multiplexed outputs, respectively.

[samsrabin]

@rgknox @ckoven This is ready for re-review. The only remaining thing I need to ask about (marked by TODOs in the code) is that the reproductive biomass seems to not be included in total biomass in some parts of the history code, whereas it is included in others. Do you think there's a reason for that?