Skip to content

bug fix for FVEG scaling of canopy heat storage#87

Merged
cenlinhe merged 1 commit intoNCAR:release-v4.5-WRFfrom
cenlinhe:canhs_bugfix
Jun 7, 2023
Merged

bug fix for FVEG scaling of canopy heat storage#87
cenlinhe merged 1 commit intoNCAR:release-v4.5-WRFfrom
cenlinhe:canhs_bugfix

Conversation

@cenlinhe
Copy link
Copy Markdown
Collaborator

@cenlinhe cenlinhe commented Jun 6, 2023

This is a bug fix to solve the issue: #86
Specifically, current canopy heat storage change term is not scaled by FVEG (vegetation fraction), but all the other canopy-related heat flux terms are scaled by FVEG to get the grid-level value before calculating the energy balance. So a FVEG scaling is needed to apply to the canopy heat storage change term.

@cenlinhe cenlinhe requested review from barlage and tslin2 June 6, 2023 23:00
@cenlinhe cenlinhe added the bug Something isn't working label Jun 6, 2023
Copy link
Copy Markdown
Contributor

@barlage barlage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested these changes in my version and it produces the same TV answer as my change in #86

Comment thread src/module_sf_noahmplsm.F
@@ -4052,14 +4052,14 @@ SUBROUTINE VEGE_FLUX(parameters,NSNOW ,NSOIL ,ISNOW ,VEGTYP ,VEG , &

B = SAV-IRC-SHC-EVC-TR+PAHV !additional w/m2
! A = FVEG*(4.0*CIR*TV**3 + CSH + (CEV+CTR)*DESTV) !volumetric heat capacity
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe take this as a chance to remove this commented line, but maybe it's not necessary in this legacy code

@cenlinhe cenlinhe merged commit 981d4f8 into NCAR:release-v4.5-WRF Jun 7, 2023
@tslin2
Copy link
Copy Markdown
Collaborator

tslin2 commented Jun 16, 2023

I run a case with this bug fix in WRFv4.5 with urban option = 3 (BEM) without use_wudapt_lcz in the Southeast United States, in two nested domain, 5km and 1km for eight days. The run starts from 2015-08-02_02:00:00 but crashes at 2015-08-06_21:43:00 with the following error.
Looks like the model stop at module_sf_bep_bem.F, if tr_av < 100
Note that model runs successfully before this bug fix with the same namelist. Also runs successfully with urban option = 2 (BEP). I am running to reduce time step, but please feel free to leave any comments. Thanks.

d01 2015-08-06_21:43:00 3 points exceeded v_cfl = 2 in domain d01 at time 2015-08-06_21:43:00 hours
d01 2015-08-06_21:43:00 Max W: 139 106 13 W: -10.74 w-cfl: 2.04 dETA: 0.02
d01 2015-08-06_21:43:00 4 points exceeded v_cfl = 2 in domain d01 at time 2015-08-06_21:43:00 hours
d01 2015-08-06_21:43:00 Max W: 139 106 13 W: -10.82 w-cfl: 2.07 dETA: 0.02
d01 2015-08-06_21:43:00 6 points exceeded v_cfl = 2 in domain d01 at time 2015-08-06_21:43:00 hours
d01 2015-08-06_21:43:00 Max W: 139 106 13 W: -10.68 w-cfl: 2.12 dETA: 0.02
in upward_rad 3 1 0 94.71500
in upward_rad 3 2 0 94.71500
in upward_rad 4 1 0 95.11688
in upward_rad 4 2 0 95.11688
in upward_rad 5 1 0 95.02312
in upward_rad 5 2 0 95.02312

@cenlinhe
Copy link
Copy Markdown
Collaborator Author

cenlinhe commented Jun 17, 2023

This is weird. But I did identify another urban-related bug in Noah-MP driver:

https://github.com/NCAR/noahmp/blob/release-v4.5-WRF/drivers/wrf/module_sf_noahmpdrv.F#L1464

This if-statement is not correct for urban_physics >0, because when urban_physics>0, Noah-MP main column model should simulate the rural portion of the grid as natural vegetation:

https://github.com/NCAR/noahmp/blob/release-v4.5-WRF/drivers/wrf/module_sf_noahmpdrv.F#L926

Thus, the current code still simulates a bulk urban (i.e., urban_flag = true) for rural portion in Noah-MP for urban_physics>0 even veg_type is set to natural_veg (the urban_flag overwrites a lot of surface properties in the Noah-MP lsm code).

A fix could be adding " .and. (SF_URBAN_PHYSICS == 0)" in the above if-statement for urban_flag:
https://github.com/NCAR/noahmp/blob/release-v4.5-WRF/drivers/wrf/module_sf_noahmpdrv.F#L1464

@tslin2
Copy link
Copy Markdown
Collaborator

tslin2 commented Jun 17, 2023

do you mean there is missing (i,j) even it set to natural vegetation type?!
I see here define VEGTYP
https://github.com/NCAR/noahmp/blob/981d4f859ce6c64213d38a783654c05b47b3485e/drivers/wrf/module_sf_noahmpdrv.F#LL742C17-L742C23

@cenlinhe
Copy link
Copy Markdown
Collaborator Author

cenlinhe commented Jun 17, 2023

No, I meant even the vegetation type (VEGTYP) is assigned as NATURAL_TABLE, the parameters%URBAN_FLAG is still set to true. So for the rural portion of urban grid, Noah-MP main code will treat it as "urban" instead of "NATURAL_TABLE" type. You can double-check the URBAN_FLAG variable in module_sf_noahmpdrv.F file.

@tslin2
Copy link
Copy Markdown
Collaborator

tslin2 commented Jun 17, 2023

Only looking the codes you posted here, I think urban flag is false, for urban option not equal zero and urban flag is set in TRANSFER_MP_PARAMETERS. Does I miss anything else here?! I should also set up a run to understand what is the issue.

@cenlinhe
Copy link
Copy Markdown
Collaborator Author

Sorry, I was wrong and you are right. The current code is correct. The parameter transfer is called after the VEGTYP is assigned. So everything is good in this part.

@cenlinhe
Copy link
Copy Markdown
Collaborator Author

This bug is in the version 5.0 and I have corrected it:
45f6521

Let me know if this fix looks good to you. (originally, noahmp%config%domain%FlagUrban is set to true as long as the grid is urban type no matter urban_physics_option is > 0 or =0. Now I only assign FlagUrban to be true when urban_physics_option=0).

@tslin2
Copy link
Copy Markdown
Collaborator

tslin2 commented Jun 17, 2023

The fix in version 5 looks good to me, thanks!

@barlage
Copy link
Copy Markdown
Contributor

barlage commented Jun 20, 2023

Is this a different issue? It seems that this should be moved to a different urban discussion for clarity.

@tslin2
Copy link
Copy Markdown
Collaborator

tslin2 commented Jun 23, 2023

Is this a different issue? It seems that this should be moved to a different urban discussion for clarity.

yes, should discuss elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants