ESCOMP
CNFUNInit appears to have some problems
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:
https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNFUNMod.F90#L177-L183
This uses the nstep_fun variable, which is calculated a few lines up:
https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNFUNMod.F90#L172
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:
https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNFUNMod.F90#L171
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:
https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNFUNMod.F90#L184
is problematic: The relevant driver ordering is:
- CNFUNInit: https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNDriverMod.F90#L384
- CNFun, where
leafcn_offsetis used, called via SoilBiochemCompetition: https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNDriverMod.F90#L411 - CNOffsetLitterfall and CNBackgroundLitterfall, where
leafcn_offsetis set to its real value, called via CNPhenology phase 2, here: https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNDriverMod.F90#L479
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.
@billsacks I know it's been a while, but do you think there's an easy fix for this that we could investigate?
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:
https://github.com/ESCOMP/CTSM/blob/449345ee9a534316cba0d6b4fd696b8d57e60428/src/biogeochem/CNFUNMod.F90#L177-L183
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).
@wwieder and @billsacks this seems like something we should fix for the release right? What milestone should this go into?
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.