Bug fix for Noah-MP snow, vegetation and urban

[cenlinhe]

Sync with updated nonrefactored Noah-MP branch for a few bug fixes for snow, vegetation, and urban

TYPE: bug fix

KEYWORDS: Noah-MP, snow combine, vegetation fraction scaling, urban ground heat flux

SOURCE: Cenlin He (NCAR/RAL)

DESCRIPTION OF CHANGES:
There are a few bug fixes for Noah-MP related processes:
(1) the snow layer index in snow COMBINE module was wrong, causing model crash when snow layer changes by more than 1 layer within one timestep; This has been fixed by using the correct layer index.
(2) the ground heat flux sign in Noah-MP column model (positive: downward) is inconsistent with that in urban canopy model (positive: upward), leading to wrong diagnostic grid-mean ground heat flux calculation for urban grid. This only affects the diagnostic value for output. See this issue: #1921 and NCAR/hrldas#114
(3) there is a bug in vegetation fraction (FVEG) scaling for stomata resistance calculation (NCAR/noahmp#92) and canopy interception calculation (NCAR/noahmp#91).

LIST OF MODIFIED FILES: list of changed files (use git diff --name-status master to get formatted list):
phys/noahmp/src/module_sf_noahmplsm.F
phys/noahmp/src/module_sf_noahmpdrv.F

TESTS CONDUCTED:

1. Tested successfully in NCAR Cheyenne HPC for 13-km run over the entire CONUS region
2. The Jenkins tests are all passing.

RELEASE NOTE: Noah-MP bug fix for snow combination, vegetation fraction scaling, and urban ground heat flux sign.

[weiwangncar]

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

[weiwangncar]

@cenlinhe Do you want this fix to be in develop branch (as it is now) or release-v4.5.2?

[cenlinhe]

release-v4.5.2 is better since it is just bug fix

[weiwangncar]

@cenlinhe Can you please take a look at the last commit by @jesusff on this PR? Also why is configure.default changed in this PR? It should be removed.

[jesusff]

Hi @weiwangncar, I just mentioned this PR in my commit, but it was applied to a different branch in our WRF fork for CORDEX, where we are already using the patches from @cenlinhe for these bugs. No additional commit from my side in this PR.

[weiwangncar]

@jesusff Thanks for letting me know.

[dudhia]

@cenlinhe If this affects the ground flux sign in the output, it seems it should go into v4.6 instead of v4.5.2.
Can we separate the bug-fix for 4.5.2?

[jesusff]

@cenlinhe, more specifically: is this just a change in the convention of the sign for ground flux (breaking the compatibility with previous model output)? or is it a fix in the sign of some components of the ground flux, leading to changes in the absolute value of the ground flux?

[cenlinhe]

@cenlinhe, more specifically: is this just a change in the convention of the sign for ground flux (breaking the compatibility with previous model output)? or is it a fix in the sign of some components of the ground flux, leading to changes in the absolute value of the ground flux?

This is only a fix for the sign of ground flux output itself (as a diagnostic) not its components or its absolute value. This does not affect any energy calculations in Noah-MP, just the sign for output.

[dudhia]

@cenlinhe could you check my comment above. Wondering if this sign change should be put off until V4.6?

[cenlinhe]

@cenlinhe could you check my comment above. Wondering if this sign change should be put off until V4.6?

I am fine with putting off this entire PR until V4.6. I do not know an easy way to separate the commit out without re-doing those changes one by one again.

[dudhia]

Seems like only the two changes in noahmpdrv would need to be removed from the PR, but we can also target develop if you prefer.

…

On Tue, Dec 12, 2023 at 9:42 AM Cenlin_He ***@***.***> wrote: @cenlinhe <https://github.com/cenlinhe> could you check my comment above. Wondering if this sign change should be put off until V4.6? I am fine with putting off this entire PR until V4.6. I do not know an easy way to separate the commit out without re-doing those changes one by one again. — Reply to this email directly, view it on GitHub <#1929 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77C6KDY7RWJXFPV5Y3DYJCCPJAVCNFSM6AAAAAA6LHLTJOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJSGQYTANZYHA> . You are receiving this because your review was requested.Message ID: ***@***.***>

The base branch was changed.

[weiwangncar]

@cenlinhe I have removed the changes in arch/configure_defaults in this PR.