fix frootfrac re-adjustment so frootfrac cannot be nan #720
Conversation
amullen01
changed the title
There was a problem hiding this comment.
This is a really good catch, and the change in AVLN results look good! We do need the full comparison plots, whether that's something you think you can do easily or if you'd prefer to meet up and do it together. Ben wrote up a walkthrough here (#716) but I'm up for either way.
|
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? |
|
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.
|
|
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. |
|
@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? |
There was a problem hiding this comment.
Looks good. I think it would be good to add the guard against working with PFTs as in #624
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):
Output after fix (See AVLN row):