24-hr running mean vegetation temperature is not b4b on threaded exact restart SatPhen test

[glemieux]

In testing ESCOMP/CTSM#1849, it has been discovered that FATES_TVEG24 is not b4b on the fates suite ERP_P36x2 satellite phenology test. #908 was also discovered in the course of testing, although the fix for that does not address this issue. This issue appears not to be an specific to threaded tests necessarily as a similar ERP_P36x2 test using FatesColdAllVars testmod is b4b.

[glemieux]

I think I've figured out what is going on here, to a certain extent. I believe the UpdateFatesRMeansTStep procedure needs a check to make sure that it's not trying to set the running mean temp for the bareground patch during satellite phenology (and presumably fbg+nocomp) mode. I noticed in printing out each patch loop that there was one more iteration in UpdateFatesRMeansTStep step than in the WrapUpdateFatesRmean procedure that calls it. This calling function, being on the clmfates_interfacemod side of things, uses a loop from one to this%fates(nc)%sites(s)%youngest_patch%patchno iteration (common in this module), where as the called update function uses a do while patch loop:

fates/main/FatesInterfaceMod.F90

Lines 1926 to 1942 in e216ca0

	ifp=0
	cpatch => sites(s)%oldest_patch
	do while(associated(cpatch))
	ifp=ifp+1
	call cpatch%tveg24%UpdateRMean(bc_in(s)%t_veg_pa(ifp))
	call cpatch%tveg_lpa%UpdateRMean(bc_in(s)%t_veg_pa(ifp))
	! (Keeping as an example)
	!ccohort => cpatch%tallest
	!do while (associated(ccohort))
	! call ccohort%tveg_lpa%UpdateRMean(bc_in(s)%t_veg_pa(ifp))
	! ccohort => ccohort%shorter
	!end do
	cpatch => cpatch%younger
	enddo
	end do

[glemieux]

Ok, adding a check to make sure the patchno is not zero results in the ERP satellite phenology test passing. That said, this brings up a few questions:

1. Why is there a consistent non-zero value for bc_in(s)%t_veg_pa(ifp) for an ifp index value beyond the allocated range during ERP test, but zero during ERS tests? I'm see how it's memory access issue, but it's weird that to me that the value would change based on the test type. Was it not for the ERP test type, this wouldn't have come up.
2. Is it possible for sites(s)%youngest_patch%patchno to be the bareground patch?
3. Is this error prevalent in fixed biogeography+nocomp mode as well as satellite phenology since they both have bareground patches?

I think I'll need to ask @ekluzek to help out with question one given that it relates to hlm-side infrastructure.

For question 2, the set_patchno procedure seem to allow the possibilty for having the bareground patch be the youngest, but it depends on how nocomp_pft_label is set. I need to look into that more.

Testing for question 3 is in progress.

[glemieux]

3. Is this error prevalent in fixed biogeography+nocomp mode as well as satellite phenology since they both have bareground patches?

It doesn't appear that this is the case. Looking at the diagnostic outputs, there are matching numbers of patches for both the iteration methods, with no patchno equal to zero. Searching to code, this is because the only time we set the value to zero directly is in the set_patchno procedure when hlm_use_sp is true:

fates/biogeochem/EDPatchDynamicsMod.F90

Lines 1257 to 1265 in e216ca0

	if(hlm_use_sp.eq.itrue)then
	patchno = 1
	currentPatch => currentSite%oldest_patch
	do while(associated(currentPatch))
	if(currentPatch%nocomp_pft_label.eq.0)then
	! for bareground patch, we make the patch number 0
	! we also do not count this in the veg. patch numbering scheme.
	currentPatch%patchno = 0
	else

So the followup question is should the check be changed to include both nocomp+fbg as well?