search

uaf-arctic-eco-modeling / dvm-dos-tem

A process based Dynamic Vegetation, Dynamic Organic Soil, Terrestrial Ecosystem Model.
MIT License
22 stars 24 forks source link

fix frootfrac re-adjustment so frootfrac cannot be nan #720

Open amullen01 opened 5 months ago

amullen01 commented 5 months ago

Remove potential for divide by zero resulting in cd->m_soil.frootfrac = nan. Previously this would occur in the moss layer where rootfracsum = 0. Nans propagated, preventing AVLN from being updated

Added a few checks to avoid this, some of which may be redundant, but fixes the problem.

Output before fix (See AVLN row):

Pools_fire_exps

Output after fix (See AVLN row):

Pools_fire_exps_AVLN_fix

hgenet commented 5 months ago

Terrific - THANK YOU Andrew for catching this. I'm wondering about the great increase in LTRFAL - Any chance it is a mean --> sum difference? Would you have GPP also?

amullen01 commented 5 months ago

Valeria just pointed out that this is essentially the same fix as #624, so Ruth already caught it! Good catch on the LTRFALC increase Helene. It does appear to be a mean vs. sum difference, since the newer plots were generated after you told me (I think) that 'sum' is the best way to visualize LTRFALC.

The fix helps GPP and VegC recover to pre-fire levels for the Birch CMT, however Black Spruce VegC/GPP is still an issue. Also still some issues with RH which I am about to start debugging.

C_Fluxes_fire_exps_post_fix

VEGC_tree_fire_exps_post_fix

amullen01 commented 5 months ago

I generated comparison plots, and as expected there is no difference between the branch and master. The reason being there was no fire for the test cases.

result.pdf

tobeycarman commented 2 months ago

@rarutter, @hgenet after a re-read of this issue and #624, it looks like we should use both fixes? In this PR, a guard is put around summing non-existent layers, and in #624 there is a guard for both non-existent layers and non-existent PFTs. In #624, it looks like tests were done by manually effecting the change, but no code was ever committed. So maybe adding a commit to this PR that implements the non existent PFT guard as per #624, and then we can merge this and close #624?