WRF
WRF copied to clipboard
wrf-model
Modified/Corrected LECH's stability functions in the SL Urban Canopy Model following the formulation.
TYPE: Bug fix
KEYWORDS: LECH stability functions, Latent heat flux, Sensible heat flux, SL Urban Canpy Model
SOURCE: Parag Joshi (Brookhaven National Lab), Katia Lamer (Brookhaven National Lab)
DESCRIPTION OF CHANGES: Problem: The stabvility functions originally proposed by Lech Lobocki which are used in calculating the exchange coefficients between surface layer and roof, building wall, Green roof, and ground are implemented incorrectly. The error surfaced while comparing the formulation suggested in Lobocki 1992 with the equations in commands/lines 3056 and 3062 (WRF-v4.5.2).
Solution: The code has been corrected by referring to the equations 48 and 49 of the article "A procedure for the derivation of surface layer bulk relationahips from simplified second-order closure models" by Lobocki 1992 (Journal of Applied Meteorology).
ISSUE: For use when this PR closes an issue. Fixes #123
LIST OF MODIFIED FILES: M phys/module_sf_urban.F
TESTS CONDUCTED:
- The significance of the corrections can be highlighted by comparing a test run with the corrected module and current version of the WRF model.
- The fix will correct the evaluation of the exchange coefficients between the ground, roof, green roof etc and air.
- The Jenkins tests are all passing.
RELEASE NOTE: Lech's stability functions are corrected following the original formulation.
The regression test results:
Test Type | Expected | Received | Failed
= = = = = = = = = = = = = = = = = = = = = = = = = = = =
Number of Tests : 23 24
Number of Builds : 60 57
Number of Simulations : 158 150 0
Number of Comparisons : 95 86 0
Failed Simulations are:
None
Which comparisons are not bit-for-bit:
None
OK, this is indeed a bug and the proposed fix looks correct to me. I guess this error came from the original code of Noah LSM SFCDIF subroutine based on Chen et al., (1997, BLM): https://link.springer.com/article/10.1023/A:1000531001463 which was copied and pasted to the urban part. The paper shows the correct equations (A7 and A8 in the appendix) but the code implementation is wrong. The current Noah-MP SFCDIF2 subroutine (based on Chen et al., 1997) also has this bug: https://github.com/NCAR/noahmp/blob/noahmp_v4.5_develop/src/module_sf_noahmplsm.F#L4810-L4813 which needs to be fixed too. @dudhia @barlage
@cenlinhe Thanks for reviewing this PR. Would you approve this?
Yes, I approve this. @tslin2 could you please also take a look when you have time?
@cenlinhe Should I go ahead an make changes in the MoahMP-SFCDIF2 as well or you are going to update the subroutine?
@cenlinhe Should I go ahead an make changes in the MoahMP-SFCDIF2 as well or you are going to update the subroutine?
No, you do not need to do it. I will fix them later.
@cenlinhe Thanks for reviewing this PR. Would you approve this?
Yes, I approve this. @tslin2 could you please also take a look when you have time?
I will try to look at this next week!
@joshi994 do you have a reference showing the equations for your latest bug fix commit?
@joshi994 do you have a reference showing the equations for your latest bug fix commit?
@cenlinhe I messed up in my latest commit. I have submitted a different pull request. Please refer to the link: https://github.com/wrf-model/WRF/pull/2038
@joshi994 do you have a reference showing the equations for your latest bug fix commit?
@cenlinhe I messed up in my latest commit. I have submitted a different pull request. Please refer to the link: #2038
OK, I think you may need to revert back to the original commit by removing the latest one, otherwise this PR seems being messed up.
Your previous changes seems gone. They were replaced by your latest code changes.
Your previous changes seems gone. They were replaced by your latest code changes.
@cenlinhe I am new to GitHub commit. I may need some time to correct the error. What do you suggest?
Your previous changes seems gone. They were replaced by your latest code changes.
@cenlinhe I am new to GitHub commit. I may need some time to correct the error. What do you suggest?
Sounds good. It's OK for you to take some time to correct this error. Thanks.
Your previous changes seems gone. They were replaced by your latest code changes.
@cenlinhe I am new to GitHub commit. I may need some time to correct the error. What do you suggest?
Sounds good. It's OK for you to take some time to correct this error. Thanks.
@cenlinhe I tried reverting the error. Can you please confirm if the changes are restored and the new PR is still there?
looks like this PR is reverted back and correct now. the new PR (2038) is also there. When you make any changes to your new PR (2038), please make sure you are working on your "my_changes2" branch instead of "my_changes1" branch. The "my_changes1" branch is for this PR here.
Thanks for confirming. Please let me know if you need further clarification.
From: Cenlin_He @.> Date: Wednesday, April 17, 2024 at 6:10 PM To: wrf-model/WRF @.> Cc: Joshi, Parag @.>, Mention @.> Subject: Re: [wrf-model/WRF] Modified/Corrected LECH's stability functions in the SL Urban Canopy Model following the formulation. (PR #2032)
looks like this PR is reverted back and correct now.
— Reply to this email directly, view it on GitHubhttps://github.com/wrf-model/WRF/pull/2032#issuecomment-2062531294, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BGF4AMW34XMKR7QKUWZGJVDY53XMJAVCNFSM6AAAAABFR3BMUGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRSGUZTCMRZGQ. You are receiving this because you were mentioned.Message ID: @.***>
@joshi994 just to double check. please see if this is your original PR bug fix code changes: https://github.com/wrf-model/WRF/pull/2032/files I think it is.
@joshi994 just to double check. please see if this is your original PR bug fix code changes: https://github.com/wrf-model/WRF/pull/2032/files I think it is.
@cenlinhe Yes, this is the original PR bug fix changes. I accidentally hit my_changes1 for the new PR (https://github.com/wrf-model/WRF/pull/2038) and that's what caused the issue. Thanks for your response.
Hi, you might want to take the opportunity to move the misplaced Lech equation to its place, as in my last commit above https://github.com/CORDEX-WRF-community/WRF/commit/1a2492c3647fc9612e47e4f185c7e6c0f05cf4f6
@cenlinhe Thanks for reviewing this PR. Would you approve this?
Yes, I approve this. @tslin2 could you please also take a look when you have time?
I will try to look at this next week!
yes, the changes is consistent with the papers.
The regression test results:
Test Type | Expected | Received | Failed
= = = = = = = = = = = = = = = = = = = = = = = = = = = =
Number of Tests : 23 24
Number of Builds : 60 57
Number of Simulations : 158 150 0
Number of Comparisons : 95 86 0
Failed Simulations are:
None
Which comparisons are not bit-for-bit:
None