NCAR / noahmp

Noah-MP Community Repository
Other
72 stars 81 forks source link

canopy heat storage not scaled by vegetation fraction #86

Closed barlage closed 1 year ago

barlage commented 1 year ago

when calculating canopy heat storage term in vegetation flux iteration, the term should be scaled by vegetation fraction to be consistent with the radiation scaling in the solution:

https://github.com/NCAR/noahmp/blob/ac6848be386811364c1e15337673b134993f74b5/src/SurfaceEnergyFluxVegetatedMod.F90#L302

For example, when the term is not scaled, the vegetation temperature will depend on vegetation fraction:

without canopy heat scaling, fveg = 0.1: tv = 310.48472113867035
without canopy heat scaling, fveg = 0.9: tv = 309.36242868601590

with canopy heat scaling, fveg = 0.1: tv = 309.27431763431838
with canopy heat scaling, fveg = 0.9: tv = 309.27431763431838

AlbaraaKhayat commented 1 year ago

Hello, What is the modification to the line so that we could fix it in our implementation? Thank you

barlage commented 1 year ago

I'm still working in the non-refactored code, so this is the proposed fix. Didn't want to put in a PR until I could get something working with refactored code (don't know when that will be).

Related to this line in the non-refactored code:

https://github.com/NCAR/noahmp/blob/e1b83ad4c55e5e186be1cf726a222c78d6abdcc2/src/module_sf_noahmplsm.F#L4051

 ! canopy heat capacity
-        hcv = fveg*(parameters%cbiom*vaie*cwat + canliq*cwat/denh2o + canice*cice/denice) !j/m2/k
+        hcv = parameters%cbiom*vaie*cwat + canliq*cwat/denh2o + canice*cice/denice !j/m2/k
cenlinhe commented 1 year ago

This is indeed a bug. I will fix it in the GitHub develop and master branch as well as the version 4.5 used in WRFv4.5.

cenlinhe commented 1 year ago

The canopy heat storage term needs to be scaled by vegetation fraction (FVEG) because all the other heat flux terms in the energy balance calculations are scaled by FVEG.

tslin2 commented 1 year ago

I am closing this issue based on the above discussions.

barlage commented 1 year ago

@cenlinhe @tslin2 for future reference there should be a way to automatically close the issue when the PR is merged.

dmocko commented 1 year ago

I believe if you put the following in your commit message as part of the PR, the issue will automatically close when the PR is merged into the main branch:

Resolves: #[issue number]

See the example commit message here: https://cbea.ms/git-commit/#seven-rules