CNFUNInit appears to have some problems

[billsacks]

Brief summary of bug

From a read through of CNFunInit and other relevant code, it looks like this subroutine has some problems.

General bug information

CTSM version you are using: latest master

Does this bug cause significantly incorrect results in the model's science? Yes, potentially, but hopefully only minor (not investigated carefully yet).

Configurations affected: All configurations with FUN active

Details of bug

I see two issues with CNFunInit. The good news is that problem (1) makes problem (2) only show up rarely (if I understand things correctly).

(1) The logic that claims to be deciding if FUN will be called on this timestep:

CTSM/src/biogeochem/CNFUNMod.F90

Lines 177 to 183 in 449345e

	!--------------------------------------------------------------------
	!---
	! Decide if FUN will be called on this timestep.
	!--------------------------------------------------------------------
	!---
	numofyear = nstep/nstep_fun
	if (mod(nstep,nstep_fun) == 0) then

This uses the nstep_fun variable, which is calculated a few lines up:

CTSM/src/biogeochem/CNFUNMod.F90

Line 172 in 449345e

nstep_fun = int(secspday * dayspyr / dt)

Based on this logic, it looks like this code is only executed once a year, implying that FUN is only called once a year. But in fact FUN seems to be called every time step.

There are a number of other time-related variables calculated in this subroutine that are unused and should be removed. Furthermore, the fun_period parameter defined in clm_varctl is unused, except in this line, which computes an unused variable:

CTSM/src/biogeochem/CNFUNMod.F90

Line 171 in 449345e

timestep_fun = real(secspday * fun_period)

Incidentally, timestep_fun as defined there (but not used anywhere) suggests that FUN is called on a daily time step (since fun_period is set to 1), but that also doesn't seem correct.

I'm not sure what, if any, problems are caused by this initialization code only happening once per year. It's possible that this entire initialization code could be changed to only be done in model initialization rather than being called in the driver loop. I haven't looked into this, other than for leafcn_offset, as discussed in point (2).

(2) I am less sure that this is a problem, and even if it is, given (1), it should only cause problems once a year: I think the setting of leafcn_offset here:

CTSM/src/biogeochem/CNFUNMod.F90

Line 184 in 449345e

leafcn_offset(bounds%begp:bounds%endp) = leafcn(ivt(bounds%begp:bounds%endp))

is problematic: The relevant driver ordering is:

• CNFUNInit:

  CTSM/src/biogeochem/CNDriverMod.F90

  Line 384 in 449345e

  call CNFUNInit(bounds,cnveg_state_inst,cnveg_carbonstate_inst,cnveg_nitrogenstate_inst)

• CNFun, where leafcn_offset is used, called via SoilBiochemCompetition:

  CTSM/src/biogeochem/CNDriverMod.F90

  Line 411 in 449345e

  call SoilBiogeochemCompetition (bounds, num_soilc, filter_soilc,num_soilp, filter_soilp, &

• CNOffsetLitterfall and CNBackgroundLitterfall, where leafcn_offset is set to its real value, called via CNPhenology phase 2, here:

  CTSM/src/biogeochem/CNDriverMod.F90

  Line 479 in 449345e

  call CNPhenology (bounds, num_soilc, filter_soilc, num_soilp, &

That is, CNFun appears to use leafcn_offset from the previous time step, since it is used before it is (typically) set in a time step. On occasions when the CNFunInit code actually operates, it seems like it resets leafcn_offset to a fixed value, and so it seems like CNFun would use the wrong C:N ratio on these occasions.

[wwieder]

@billsacks I know it's been a while, but do you think there's an easy fix for this that we could investigate?

[billsacks]

do you think there's an easy fix for this that we could investigate?

Is FUN in fact called every time step? If so, I think the first step towards (1) would be to investigate the impact of changing this:

CTSM/src/biogeochem/CNFUNMod.F90

Lines 177 to 183 in 449345e

	!--------------------------------------------------------------------
	!---
	! Decide if FUN will be called on this timestep.
	!--------------------------------------------------------------------
	!---
	numofyear = nstep/nstep_fun
	if (mod(nstep,nstep_fun) == 0) then

to happen every time step rather than what I think is just once per year. Based on comments, it seems like this is supposed to happen for every FUN call, though it's possible that I'm wrong about the intent or my hand-check of what this logic is doing (I don't think I ever actually ran a case with prints / debugging to verify my understanding of its behavior).

[ekluzek]

@wwieder and @billsacks this seems like something we should fix for the release right? What milestone should this go into?

[wwieder]

The logic of what's being done here kind of makes my head hurt. @billsacks' suggestion of investigating this seems like a good idea, but I'm not sure how high a priority this should be, which makes assigning a milestone challenging. Let's discuss at our next SE meeting.