Fixing the order in which patch-level canopy_layer_tlai is calculated

[glemieux]

Description:

This PR resolves #823.

Introduces new subroutine for potential additional future use elsewhere in the code.

Collaborators:

@rgknox

Expectation of Answer Changes:

This will changes answers (although regression tests may only show this occurring in a subset of tests).

Checklist:

• My change requires a change to the documentation.
• I have updated the in-code documentation .AND. (the technical note .OR. the wiki) accordingly.
• I have read the CONTRIBUTING document.
• FATES PASS/FAIL regression tests were run
• If answers were expected to change, evaluation was performed and provided

Test Results:

TBD

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

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

FATES baseline hash-tag:

Test Output:

[glemieux]

The refactor of the relevant canopy_layer_tlai section is complete (as of d451203) and was tested out against a single exact restart fates testmod on cheyenne with b4b results:
/glade/u/home/glemieux/scratch/ctsm-tests/tests_pr828_erpshortcompare1

Now that I'm confident the refactor is good, I'll add the additional canopy layer loop for the final fix to this issue.

[glemieux]

Talking about this with @rgknox, hes suggested reviewing the code to find other areas where we can utilize the new UpdatePatchLAI subroutine. I'm going to take a look at this in the next couple days before we conduct the final regression tests.

[glemieux]

Commit 1962a7d updates this branch to remove the redundant call to tree_lai and tree_sai in update_hlm_dynamics.

[glemieux]

6ae65be and 428d679 were manually replicated from @rgknox coema branch which is based off of this branch. I created a separate testing branch to make sure that removing these calls is truly redundant. Said branch reverts 3a593ad to test against baseline fates-sci.1.57.0_api.23.0.0-ctsm5.1.dev091. So far, the individual regression tests I ran are coming back b4b. I will run the full fates test suite as well as a test a longer term case.

UPDATE: the redundant check branch passed all expected tests b4b against the baseline in the fates suite:

/glade/u/home/glemieux/scratch/ctsm-tests/tests_redcheck-dev091-revertrevert2-full

A longer term, 5 year 1x1_brazil case ran b4b as well.

[glemieux]

Regression tests are underway. Note that due to ESCOMP/CTSM#1764 the P32x2 test will come back as a run failure due to hitting the wall clock limit.

[glemieux]

I ran regression tests on Cheyenne: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr828

Most tests pass b4b with the following exceptions:

SMS_Lm13.1x1_brazil.I2000Clm50FatesCruRsGs.cheyenne_gnu.clm-FatesColdDef
SMS_Lm13.1x1_brazil.I2000Clm50FatesCruRsGs.cheyenne_intel.clm-FatesColdDef
ERS_Lm12.1x1_brazil.I2000Clm50FatesCruRsGs.cheyenne_intel.clm-FatesFireLightningPopDens
SMS_Lm6.f45_f45_mg37.I2000Clm50FatesCruRsGs.cheyenne_intel.clm-Fates
ERS_Ld60.f45_f45_mg37.I2000Clm50FatesCruRsGs.cheyenne_intel.clm-FatesColdDefLogging

Given the correction to the canopy_layer_tlai calculation, seeing differences in longer runs in which the patches can start to become more disturbed make sense. That said, the two SMS_Lm13.1x1_brazil tests have differences on the first time step in FATES_AREA_PLANTS and FATES_AUTORESP. I'm not sure yet why this is.

Note that there are two 'known' failures right now due to #861 and ESCOMP/CTSM#1764

[rgknox]

Looks good!

[glemieux]

That said, the two SMS_Lm13.1x1_brazil tests have differences on the first time step in FATES_AREA_PLANTS and FATES_AUTORESP. I'm not sure yet why this is.

I'm a silly goose: the test only runs cprnc on the nc output file that covers the end of the test run. As such, its checking the 2001 nc file for the 13th month, which of course should not be b4b on the first time step of that file. Running cprnc manually on the first year output file shows that the test case doesn't become non-b4b until approximately 2/3rds of the way through the year as expected.