Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue of crown depth #674

Open
JunyanDing opened this issue Jul 20, 2020 · 22 comments
Open

Issue of crown depth #674

JunyanDing opened this issue Jul 20, 2020 · 22 comments

Comments

@JunyanDing
Copy link
Contributor

In fire module the parameter fates_fire_crown_depth_frac is used to calculate the crown depth
https://github.com/NGEET/fates/blob/master/fire/SFMainMod.F90#L885-L898

But in allometry module the crown depth is calculated as the smaller value of tree height and 0.1m:
https://github.com/NGEET/fates/blob/master/biogeochem/FatesAllometryMod.F90#L1910

Should we calculate the crown depth the same way as in fire module? Also, in fire module this parameter is named as EDPftvarcon_inst%crown, which seems informative.

@ckoven
Copy link
Contributor

ckoven commented Jul 20, 2020

Looking at the code, the subroutine CrownDepth is only used by the hydraulics code, does it have the same physical/ecological meaning as in the fire code? If so, I agree that we ought to unify those calculations.

@JunyanDing
Copy link
Contributor Author

It affects the length of the stem, hence the total hydraulic conductance of the tree, thus the leaf water potential.

@rosiealice
Copy link
Contributor

That seems to me like it's -not- the same definition as the fire module,which is trying to calculate the vertical height over which there are leaves (as opposed to stem). This might be an issue of definitions and the hydraulics one sounds more like just 'tree height' to me. Although, admittedly, most water doesn't have to actually reaxh the very top of the tree so you could imagine unifying the concepts a little more....

@ckoven
Copy link
Contributor

ckoven commented Jul 21, 2020

ok, @rosiealice, I see your point. But I still think there may be an error on https://github.com/NGEET/fates/blob/master/biogeochem/FatesAllometryMod.F90#L1910:
shouldn't it be crown_depth = max(height,0.1_r8)?

@rosiealice
Copy link
Contributor

Ah yes, indeed...

@rosiealice
Copy link
Contributor

That might cause quite a bit of answer-changing...

@ckoven
Copy link
Contributor

ckoven commented Jul 21, 2020

yeah. @xuchongang, @bchristo, @rgknox: thoughts on this?

@JunyanDing
Copy link
Contributor Author

@ckoven and @rosiealice , I disagree with both of you 😉
That line of code is on allometry module, which means FATES treats crown depth conceptually differently for fire and the rest of other processes though ut is only used in hydro currently.
Stem height is calculated as tree height - crown depth. If set to max(height, 0.1), the height of stem will either be zero if tree is shorter than 0.1m or negative if the tree is taller than 0.1m

@JunyanDing
Copy link
Contributor Author

For hydro , I will set the height of stem to be the middle of the crown. I think this is the appropriate height(or the length of flow path) for all the leaves in average

@jkshuman
Copy link
Contributor

jkshuman commented Jul 21, 2020

I agree that these are fundamentally different concepts. @rosiealice is right that fire is explicitly looking at crown depth as the actual depth of foliage or distance between top of tree and clear branch bole height which we are calling the base of the crown. This is made more clear in the implementation of crown fire, but perhaps I will clean this up in the base code before finishing the crown fire implementation.

`real(r8) ::  crown_depth          ! depth of crown (m)
real(r8) ::  height_cbb           ! clear branch bole height or crown base height (m)

 crown_depth   = currentCohort%hite*EDPftvarcon_inst%crown(currentCohort%pft) 
 height_cbb     = currentCohort%hite - crown_depth`

https://github.com/jkshuman/fates/blob/339aca2172839cc3aed5427db8af6982e8170d9a/fire/SFMainMod.F90#L934-L935

It sounds like the Hydro use of crown depth is different, and perhaps this name should be updated so that it is more clear. From @JunyanDing it sounds like this is a stem height to middle of the crown?

@jkshuman
Copy link
Contributor

Also, the way this is currently written it seems that this value will give you a value that is just less than the total plant height? (this is in line with @JunyanDing assessment, and her suggested change to the middle of the crown.)
with crown depth = min(height, 0.1)

real(r8) :: z_stem ! the height of the plants stem below crown [m]

z_stem = plant_height - crown_depth

so the use of crown_depth for Hydro taken from allometry.mod is not the same as in fire, but the comments on z_stem suggest it should be using crown_depth to get base crown height (same as in fire).

The parameterization of EDPftvarcon_inst%crown(currentCohort%pft) is at the pft level, and is set in the param file.

double fates_fire_crown_depth_frac(fates_pft) ;
fates_fire_crown_depth_frac:units = "fraction" ;
fates_fire_crown_depth_frac:long_name = "the depth of a cohorts crown as a fraction of its height" ;

Is this in line with how it is intended to be used in Hydro? @xuchongang @JunyanDing @bchristo @rgknox

@JunyanDing
Copy link
Contributor Author

Your are right @jkshuman The crown depth is used in the same way in Hydro as it used in fire module.
I think to change code of line 674 in FatesPlantHydraulicsMod.F90
z_stem = plant_height - crown_depth
to
z_stem = currentCohort%hite - 0.5* (currentCohort%hite*EDPftvarcon_inst%crown(currentCohort%pft))

@jkshuman
Copy link
Contributor

ok. If the change that @JunyanDing is proposing is implemented, I suggest we also change the name of the parameter "fates_fire_crown_depth_frac" in the parameter file to remove the "fire" tag, and just make it generic to "fates_crown_depth_frac"

@JunyanDing
Copy link
Contributor Author

I agree with @jkshuman. And we can just remove the subroutine
subroutine CrownDepth(height,crown_depth) in FatesAllometryMod.F90
https://github.com/NGEET/fates/blob/master/biogeochem/FatesAllometryMod.F90#L1893

As in Hydro module, if it is more appropriate to set the stem height to close to the top of the tree, we can just set z_stem = currentCohort%hite - 0.2(or 0.1)* (currentCohort%hite*EDPftvarcon_inst%crown(currentCohort%pft))

@rgknox
Copy link
Contributor

rgknox commented Jul 29, 2022

The hydraulics code needs the height of the center of volume for each compartment. It is used in calculating the flux between compartments, by 1) setting the mean distance* of conductance between compartments, it also 2) sets the elevation potential that has to be overcome by suction. If the leaf area density were uniformly distributed in the vertical coordinate, it would seem reasonable to set the center of volume at halfway down the depth of the crown. But that seems unlikely, no? Is this why you are suggesting to have the center of mass at about 10-20% of the total depth of the crown, away from the top @JunyanDing ?

After reviewing the code, as of sci.1.59.1_api.24.1.0, @ckoven @glemieux and I saw that fire is using the CrownDepth function and file-based parameter, yet hydro is still using 0.1 (here and here).

*Also, the distance that must be overcome for conductance, does not take into account the mean horizontal distance traveled from the stem to the leaf. It seems like it should take this into account.

@JunyanDing @xuchongang @bchristo @pnlfang @rosiealice @jkshuman

@JunyanDing
Copy link
Contributor Author

I think if we leave the crowndepth in hydro to be 0.1, it might just take into account the horizontal distance from stem to leaf (length of branch).

@bchristo
Copy link

bchristo commented Jul 29, 2022 via email

@rgknox
Copy link
Contributor

rgknox commented Jul 29, 2022

So are you proposing the vertical coordinate for the center of crown be:

height - 0.1 or height - crown_depth * 0.1

I'm thinking of hemlocks, firs and pines and how many of them distribute their crowns over much of their height. It seems like option one would be asking them to overcome more resistance and potential than necessary. I realize though, that we are trying to keep things simple, and not "branch out" (pun intended), into sophisticated crown models that require more information on crown geometry.

@bchristo
Copy link

bchristo commented Jul 29, 2022 via email

@rosiealice
Copy link
Contributor

Is it possible those trees make branches so low down because of hydraulic limitation??

Or probably more likely due to mechanical stability. Still interesting point. I agree with Brad that this crown geometry thing might be its own issue...

@rgknox
Copy link
Contributor

rgknox commented Nov 17, 2022

I think the way around this is to change our language for how this "depth" impacts hydraulics. Instead of defining this depth as something about biomass, its really used to assign the depth to the center of hydraulic potential, or rather where we prognose pressure in the leaves at a point.
Proposal, add a new parameter and change an old parameter:

Name change: fates_allom_crown_depth_frac -> fates_fire_crown_depth_frac

New parameter: fates_hydro_leafpressure_depth

@JunyanDing
Copy link
Contributor Author

@rgknox I like this idea. That makes sense.

@rgknox rgknox mentioned this issue Jan 15, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ❕Todo
Development

No branches or pull requests

6 participants