wrf-model / WRF

The official repository for the Weather Research and Forecasting (WRF) model
Other
1.18k stars 658 forks source link

PX LSM Update for MODIS 61 class LCZ #2020

Closed coastwx closed 3 months ago

coastwx commented 3 months ago

Pleim-Xiu LSM Compatibility with MODIS LCZ NUM_LAND_CAT 61

TYPE: bug fix

KEYWORDS: MODIS, LCZ, P-X LSM, NUM_LAND_CAT, 61

SOURCE: Robert Gilliam, US EPA

DESCRIPTION OF CHANGES: Problem: Approached that P-X LSM errored for MODIS LCZ 61 NUM_LAND_CAT configuration.

Solution: Logic checks in module_physics_init.F and module_sf_pxlsm.F were adjusted for 61 category inputs. P-X LSM data table, module_sf_pxlsm_data.F was updated for MODIS 61 categories. Default for LCZ 51-61 was set to MODIS urban class. We also added some default output settings for key P-X LSM variables for CMAQ AQ modeling.

ISSUE: For use when this PR closes an issue. Fixes #123

LIST OF MODIFIED FILES: M Registry/Registry.EM_COMMON M phys/module_physics_init.F M phys/module_sf_pxlsm.F M phys/module_sf_pxlsm_data.F

TESTS CONDUCTED:

  1. Tested 61 class LCZ with PX LSM for a 1 day simulation with updated codes. Ran base MODIS 21 class scheme with same code before and after LCZ update. The results were identical after a 24 hour simulation. This confirms updates do not impact other MODIS settings in the P-X LSM. The MODIS 21 was by nature not identical to the MODIS 61, but similar enough and differences follow underlying differences in MODIS datasets.
  2. Are the Jenkins tests all passing? No Jenkins test

RELEASE NOTE: Pleim-Xiu LSM is now compatible with 61 category MODIS LCZ landuse dataset.

weiwangncar commented 3 months ago

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 commented 3 months ago

@coastwx Is there a reason to put LANDUSEF, RA and RS in the history file? Note this will show up in all history output even if a user is not using PXLSM.

coastwx commented 3 months ago

I had modded the Registry for our internal testing with PX as we typically change so mainly RA and RS are forced in the output as those are key in the two CMAQ dry deposition schemes. We had some folks do complete annual simulations and realize RA and RS were not available. It is certainly understandable if we pull this from this pull request. Would this require a new pull request or can NCAR just reject one of the file changes? We're trying to update documentation so WRF-CMAQ modelers change default WRF outputs as well using either the Registry or run-time iofields_filename.

dudhia commented 3 months ago

I think it needs to be done at your end.

On Mon, Mar 18, 2024 at 7:23 AM coastwx @.***> wrote:

I had modded the Registry for our internal testing with PX as we typically change so mainly RA and RS are forced in the output as those are key in the two CMAQ dry deposition schemes. We had some folks do complete annual simulations and realize RA and RS were not available. It is certainly understandable if we pull this from this pull request. Would this require a new pull request or can NCAR just reject one of the file changes? We're trying to update documentation so WRF-CMAQ modelers change default WRF outputs as well using either the Registry or run-time iofields_filename.

— Reply to this email directly, view it on GitHub https://github.com/wrf-model/WRF/pull/2020#issuecomment-2003894446, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIZ77C4EX7TMIRDGHY6RKDYY3TD3AVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBTHA4TINBUGY . You are receiving this because your review was requested.Message ID: @.***>

weiwangncar commented 3 months ago

@coastwx No need to close this PR. I was going to check, but I think RA and RS are only used with PX LSM. If so, they can be packaged on this line: package pxlsmscheme sf_surface_physics==7 and as for LANDUSEF, without it being in the history file, I know it would fail when people use ndown using this scheme. So that may be an argument to add it to the history file.

If we agree that RA and RS can be packaged, you can simply do that in your local sandbox and do git push again.

coastwx commented 3 months ago

Wei,

I'm in progress on this. Thanks for the guidance and sorry for any waste of time on your side.

For clarity, I'm going to add Jon Pleim's PX update from some testing over the last year. And a bug in PX where some NaNs were found in GRDFLX at Arctic grid cells where lakes or sea turn to ice. A soil parameter initially zero is used in the denominator of a calc.

Do you think it would be ok to include all in the update? It only changes a few lines within PX. Nothing outside of that module. If ok, I'll do in the morning after this last round of unit testing within the current master branch code. I was going to do two pull request. But now do not want to waste time of NCAR on two vs. one pull request.

Thanks,

Rob

From: weiwangncar @.> Sent: Monday, March 18, 2024 12:46 PM To: wrf-model/WRF @.> Cc: Gilliam, Robert @.>; Mention @.> Subject: Re: [wrf-model/WRF] PX LSM Update for MODIS 61 class LCZ (PR #2020)

Caution: This email originated from outside EPA, please exercise additional caution when deciding whether to open attachments or click on provided links.

@coastwxhttps://github.com/coastwx No need to close this PR. I was going to check, but I think RA and RS are only used with PX LSM. If so, they can be packaged on this line: package pxlsmscheme sf_surface_physics==7 and as for LANDUSEF, without it being in the history file, I know it would fail when people use ndown using this scheme. So that may be an argument to add it to the history file.

If we agree that RA and RS can be packaged, you can simply do that in your local sandbox and do git push again.

- Reply to this email directly, view it on GitHubhttps://github.com/wrf-model/WRF/pull/2020#issuecomment-2004426045, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADVGWWHUBTFZRWCZJWHZSY3YY4K3JAVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBUGQZDMMBUGU. You are receiving this because you were mentioned.Message ID: @.**@.>>

dudhia commented 3 months ago

I agree with Wei's ideas.

On Mon, Mar 18, 2024 at 11:52 AM coastwx @.***> wrote:

Wei,

I'm in progress on this. Thanks for the guidance and sorry for any waste of time on your side.

For clarity, I'm going to add Jon Pleim's PX update from some testing over the last year. And a bug in PX where some NaNs were found in GRDFLX at Arctic grid cells where lakes or sea turn to ice. A soil parameter initially zero is used in the denominator of a calc.

Do you think it would be ok to include all in the update? It only changes a few lines within PX. Nothing outside of that module. If ok, I'll do in the morning after this last round of unit testing within the current master branch code. I was going to do two pull request. But now do not want to waste time of NCAR on two vs. one pull request.

Thanks,

Rob

From: weiwangncar @.> Sent: Monday, March 18, 2024 12:46 PM To: wrf-model/WRF @.> Cc: Gilliam, Robert @.>; Mention @.> Subject: Re: [wrf-model/WRF] PX LSM Update for MODIS 61 class LCZ (PR

2020)

Caution: This email originated from outside EPA, please exercise additional caution when deciding whether to open attachments or click on provided links.

@coastwxhttps://github.com/coastwx No need to close this PR. I was going to check, but I think RA and RS are only used with PX LSM. If so, they can be packaged on this line: package pxlsmscheme sf_surface_physics==7 and as for LANDUSEF, without it being in the history file, I know it would fail when people use ndown using this scheme. So that may be an argument to add it to the history file.

If we agree that RA and RS can be packaged, you can simply do that in your local sandbox and do git push again.

- Reply to this email directly, view it on GitHub< https://github.com/wrf-model/WRF/pull/2020#issuecomment-2004426045>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/ADVGWWHUBTFZRWCZJWHZSY3YY4K3JAVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBUGQZDMMBUGU

. You are receiving this because you were mentioned.Message ID: @.**@.>>

— Reply to this email directly, view it on GitHub https://github.com/wrf-model/WRF/pull/2020#issuecomment-2004572476, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIZ77FJNJRUHLXJQ4JD7KTYY4SUFAVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBUGU3TENBXGY . You are receiving this because your review was requested.Message ID: @.***>

weiwangncar commented 3 months ago

@coastwx Please go ahead, and add appropriate description in the PR. Thanks.

coastwx commented 3 months ago

Wei,

I recompiled WRF with RA and RS added in the PX package def. Then ran a quick 1 day sim. RA and RS do not show in the output. I turned other variables in PX like VEGF_PX and LAI_PX off in the WRF output via Registry.EM_COMMON as well. None of these show up in the output. I was under the impression that the package def would put those variables in the output stream.

package pxlsmscheme sf_surface_physics==7 - state:t2_ndg_new,q2_ndg_new,t2_ndg_old,q2_ndg_old,t2obs,q2obs,vegf_px,imperv,canfra,lai_px,wwlt_px,wfc_px,wsat_px,clay_px,csand_px,fmsand_px,ra,rs

Not a big deal if this does not work. We'll just have to be more clear on using the dynamic output control at runtime using iofields_filename.

Just wanted to get your view on the package issue in case I missed a step somewhere else. It seems these are just making variable available for the model and not necessarily output.

Thanks, I'll get the pull request issued today.

Rob

From: weiwangncar @.> Sent: Monday, March 18, 2024 2:08 PM To: wrf-model/WRF @.> Cc: Gilliam, Robert @.>; Mention @.> Subject: Re: [wrf-model/WRF] PX LSM Update for MODIS 61 class LCZ (PR #2020)

Caution: This email originated from outside EPA, please exercise additional caution when deciding whether to open attachments or click on provided links.

@coastwxhttps://github.com/coastwx Please go ahead, and add appropriate description in the PR. Thanks.

- Reply to this email directly, view it on GitHubhttps://github.com/wrf-model/WRF/pull/2020#issuecomment-2004603992, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADVGWWDQ5GRY5FWWP4URNNLYY4UP7AVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBUGYYDGOJZGI. You are receiving this because you were mentioned.Message ID: @.**@.>>

dudhia commented 3 months ago

You still need the h in the Registry. The advantage of package is that options not using PX will not have them.

On Tue, Mar 19, 2024 at 5:02 AM coastwx @.***> wrote:

Wei,

I recompiled WRF with RA and RS added in the PX package def. Then ran a quick 1 day sim. RA and RS do not show in the output. I turned other variables in PX like VEGF_PX and LAI_PX off in the WRF output via Registry.EM_COMMON as well. None of these show up in the output. I was under the impression that the package def would put those variables in the output stream.

package pxlsmscheme sf_surface_physics==7 - state:t2_ndg_new,q2_ndg_new,t2_ndg_old,q2_ndg_old,t2obs,q2obs,vegf_px,imperv,canfra,lai_px,wwlt_px,wfc_px,wsat_px,clay_px,csand_px,fmsand_px,ra,rs

Not a big deal if this does not work. We'll just have to be more clear on using the dynamic output control at runtime using iofields_filename.

Just wanted to get your view on the package issue in case I missed a step somewhere else. It seems these are just making variable available for the model and not necessarily output.

Thanks, I'll get the pull request issued today.

Rob

From: weiwangncar @.> Sent: Monday, March 18, 2024 2:08 PM To: wrf-model/WRF @.> Cc: Gilliam, Robert @.>; Mention @.> Subject: Re: [wrf-model/WRF] PX LSM Update for MODIS 61 class LCZ (PR

2020)

Caution: This email originated from outside EPA, please exercise additional caution when deciding whether to open attachments or click on provided links.

@coastwxhttps://github.com/coastwx Please go ahead, and add appropriate description in the PR. Thanks.

- Reply to this email directly, view it on GitHub< https://github.com/wrf-model/WRF/pull/2020#issuecomment-2004603992>, or unsubscribe< https://github.com/notifications/unsubscribe-auth/ADVGWWDQ5GRY5FWWP4URNNLYY4UP7AVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBUGYYDGOJZGI

. You are receiving this because you were mentioned.Message ID: @.**@.>>

— Reply to this email directly, view it on GitHub https://github.com/wrf-model/WRF/pull/2020#issuecomment-2006853453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIZ77BB52FVZCEGXRFTWRDYZALMNAVCNFSM6AAAAABES4KPKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBWHA2TGNBVGM . You are receiving this because your review was requested.Message ID: @.***>