Should allom_h2cd have a check to make sure that it is a value between 0 and 1?

[glemieux]

Reviewing the CrownDepth subroutine and the default parameter file, I'm assuming that the allom_h2cd1 value should be between 0 and 1 such that the crown depth can't exceed the cohort height. I don't see a check in PRTCheckParams however.

[glemieux]

@rgknox and I talked about this and the consensus is that we should for future proofing and user convienence.

[ekluzek]

Having such checks ensures the model is working the way we think it is. And as such I always endorse having appropriate checks. Sometimes it's hard to get them right (see @mpaiao below), but in the long term worth can help save time on debugging down the line, or when changes are made to the code.

If checks are deeply nested there can be small impacts on performance, so you could do things like only do it when DEBUG mode is turned on for example.

[mpaiao]

This is correct if fates_allom_dmode=1, but not necessarily if fates_allom_dmode=2 and allom_h2cd2 < 1. I think this was the reason why I never implemented such check.

However, on a second thought, I think it is possible to determine the reasonable range for allom_h2cd1 given allom_h2cd2 and the maximum plant height.

[glemieux]

Thanks @mpaiao, and good point. I'll whip up a simple PR to address the allom_dmode=1 case and we can further discuss if we want to bring in more in that PR as well.

[mpaiao]

Giving a bit more thought in this, when fates_allom_dmode=2 and allom_h2cd2 ≠ 1, then there is no straightforward bound check to prevent crown depth to exceed height, even if we force allom_h2cd1 < 1. To document the math behind it, if h is height and f is the ratio between crown depth and height:

f = allom_h2cd1 × h^(allom_h2cd2-1)

This means that f is a function of h and f=1 will happen at h1 = allom_h2cd1^[1/(1 - allom_h2cd2)]. If allom_h2cd2 < 1, f will exceed one when h < hc, and if allom_h2cd2 > 1, f will exceed one when h > hc. The only case when we can ensure f < 1 for any h is when allom_h2cd2=1 and allom_h2cd1 < 1. One test would be to make sure hc is less than the minimum recruitment height if allom_h2cd2 < 1, or hc is greater than the maximum plant height if allom_h2cd2 > 1.

[mpaiao]

I was checking the code again, and when fates_allom_dmode=2, we already ensure that crown depth does not exceed height:

fates/biogeochem/FatesAllometryMod.F90

Lines 2544 to 2550 in 1689466

	case (2) ! Power law, akin to Poorter et al. (2006).
	!---~---
	! Apply the two coefficients, but make sure crown depth does not exceed
	! the plant's height.
	!---~---
	crown_depth = min(height, p1 * height ** p2)
	!---~---

I wonder if we should (1) keep this cap as is, (2) keep the cap but limit it to 0.99*height or something like this, (3) keep the cap and add a warning in case this can happen, or (4) remove this cap and strictly ensure that the crown depth equation never exceeds height when height is between recruitment height and maximum height. Any thoughts? Thanks!