Conversation
…ater than forcing dt
…P401 runoff modes.
|
So, the MMF scheme in Noah-MP is now ready to use? Is it already merged with the master? |
| wtddt = NOAHMP401_struc(n)%ts/60.0 ! wtddt in minute? | ||
| stepwtd = 0 | ||
| dtbl = 0.0 | ||
| dtbl = NOAHMP401_struc(n)%ts |
There was a problem hiding this comment.
I note that "dtbl" is not used anywhere else in the code (including within the "phys" sub-directory).
There was a problem hiding this comment.
@dmocko This might be an artifact from earlier code changes, before I took over the MMF coding. If this doesn't appear anywhere else in the code or affect other LIS components, I am open to removing it.
| NOAHMP401_struc(n)%row_min:NOAHMP401_struc(n)%row_max)) | ||
| allocate(NOAHMP401_struc(n)%vege3d(NOAHMP401_struc(n)%col_min:NOAHMP401_struc(n)%col_max, & | ||
| NOAHMP401_struc(n)%row_min:NOAHMP401_struc(n)%row_max, & | ||
| 20)) ! 20 is the number of surfacetype categories, hardcoded temporarily |
There was a problem hiding this comment.
This line should be changed, so 20 is not hard-coded. Please change to number of vegetation classes.
There was a problem hiding this comment.
@dmocko Consistent with the change proposed below in the NoahMP401_setup.F90 module, could we change the hard-coded value to LIS_rc%nsurfacetypes? Similarly, we could use LIS_rc%nsoiltypes for the number of soils.
There was a problem hiding this comment.
Yes, I believe that this will work.
|
|
||
| ! if the total fraction from layer 1 to layer 20 is no more than 0.5, this means there is the fraction of the 21st layer is >= 0.5, set the landcover type to be 17 as HRLDAS | ||
| if(.not. sum(NOAHMP401_struc(n)%vege3d(cidx, ridx, :))>0.5) then | ||
| NOAHMP401_struc(n)%vege2d(cidx,ridx) = 17 |
There was a problem hiding this comment.
I don't think "17" should be hard-coded here. Instead, please set to the water type within LIS, so it covers all vegetation classifications that might be used.
There was a problem hiding this comment.
Sort of. I was thinking more about "LIS_rc%waterclass", which is read in from WATERCLASS from the lis_input file.
|
Hi @TimLahmers and @jvgeiger I'm finally getting around to reviewing this PR for MMF. My apologies for taking so long to finish it. I made a few specific comments about code above, but more generally most of these changes look good to me. I do wonder, though - What is the difference between the variables “LIS_rc%nsurfacetypes” and “LIS_rc%nvegtypes”? As a test, I ran in the NLDAS-2 domain using both the latest main/master branch and using the code in this PR. @TimLahmers - Can you please take a look at this output field? This is RelSMC from the main branch: This is RelSMC from the code for this PR: This is the difference between the two fields: There are two issues:
|
| xice(:,:) = 0.0 ! ice fraction is 0 | ||
| xice_threshold = 0.5 ! be consistent with hrldas | ||
| isice = 100 ! this number should not be one of vegetation type id | ||
| isurban = 13 ! be consistent with hrldas |
There was a problem hiding this comment.
Hi @TimLahmers - this is another hard-coded value that should be generalized.
Can you please change the "13" to LIS_rc%urbanclass ?
There was a problem hiding this comment.
@TimLahmers - One other question about hard-coding the ice values in lines 1029-1032.
Hard-coding these values will run the MMF groundwater for all glacier points in LIS.
However, the code itself seems to check if it is a glacier/ice point, and possibly not run these points:
WHERE(XLAND-1.5.LT.0..AND.XICE.LT. XICE_THRESHOLD.AND.IVGTYP.NE.ISICE)
LANDMASK=1
ELSEWHERE
LANDMASK=-1
ENDWHERE
Can you please comment on why you are making MMF run for all glacier/ice points?
There was a problem hiding this comment.
@dmocko I had a chance to check on this today. It looks like the lines related to glacier points are copied from HRLDAS. Is there a reason we shouldn't run MMF for glacier/ice points? If there is a scientific justification for not running this way, we could raise this at the next Noah-MP telecon.
|
Once @TimLahmers changes the urban type from my most recent comment, can you (Tim) please re-run your testcase? I now get identical results from the RELSMC output, so that issue is resolved. Thanks! Otherwise, I am satisfied with the code, and after the urban type fix and Tim re-running the testcase, we are ready |
|
@dmocko I just re-ran the test case, and it is in the same directory as before: /discover/nobackup/projects/ualbmap21/tlahmers/MMF_DemoCases_062023/LIS_MMF_CONUSvalid_RELSMCbug/OUTPUT.SIMGM.CONUS_RelSMC_Valid The code change has been committed. |
|
Many thanks for @TimLahmers for re-running the testcase a couple times over the past couple weeks to address minor The latest testcases are here: Note that this PR is only for LIS, so you only need to run/use the LIS testcase. However, just for completeness, I successfully replicated both the LIS and the LDT testcase with identical results. Further, I re-tested other Noah-MP-4.0.1 output variables using the new code but with SIMGM groundwater, and found Finally, I did another complete code review, and believe that this PR is ready to merge - once @jvgeiger cleans up the @jvgeiger - Please finish up and merge this PR once you have the time. Thanks! |
|
@jvgeiger - as long as you are cleaning up code, can you also remove the "NS" from this line before the PR is merged? https://github.com/NASA-LIS/LISF/blob/master/lis/configs/lis.config.adoc?plain=1#L8306 Also @TimLahmers - what is the recommended number of "Halo size" that should be used? We should also specify that |
|
@jvgeiger @dmocko I am replying to this thread on PR #1418 with an update. For the manuscript related to this feature, we added a change to the MMF code that improves the realism of channel exchanges and makes the model more stable (see recent commits). The LDT test case should not be affected; however, the LIS test case is being re-run and still resides in the directory (it is in the queue at the time of this message): /discover/nobackup/tlahmers/SHARE/MMF_Testcases/LIS_run Per earlier questions, the standard halo size for MMF is 3 What are the final steps to complete this PR? Since this is a busy week, I can also wait until early 2026 to follow up on this matter. |



Description
This PR includes support for the Miguez-Macho and Fan Groundwater scheme (Miguez-Macho et al. 2007). This scheme allows 2-way sub-surface routing in the Noah-MP v.4.0.1 LSM in serial and parallel processor configurations. A halo size of at least 3 grid points is recommended for stable results in parallel. The Miguez-Macho and Fan scheme is not recommended for grid resolutions higher than 4-km due to assumptions for the channel exfiltration (QRF) calculation becoming unrealistic at high resolutions.
Resolves #1037
Requires PR #1407 to support ISRIC soils
Testcase
LDT: /discover/nobackup/tlahmers/SHARE/MMF_Testcases/LDT_run
LIS: /discover/nobackup/tlahmers/SHARE/MMF_Testcases/LIS_run