Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multilayer UCMs to YSU PBL parameterization #1196

Closed
wants to merge 6 commits into from

Conversation

davegill
Copy link
Contributor

@davegill davegill commented May 16, 2020

TYPE: enhancement

KEYWORDS: BEP, BEP+BEM, YSU PBL scheme, vertical diffusion solvers

SOURCE: Eric A. Hendricks (NCAR/RAL/NSAP)

DESCRIPTION OF CHANGES:
(Replaces #1195 Add multilayer UCMs to YSU PBL parameterization)
Physics enhancement where multilayer BEP and BEP+BEM UCMs are added to the YSU PBL scheme. 1. The implicit (A) and explicit (B) BEP and BEP+BEM source terms are added as forcing to the YSU implicit vertical diffusion equation solvers. Forcing is applied for zonal momentum, meridional momentum, potential temperature, and water vapor mixing ratio. The A sources are added to the tridiagonal matrix and the B sources are added to the explicit term vectors.
2. The finite differencing is modified using the BEP volume fraction and surface fraction not occupied by buildings.
3. The BEP and BEP+BEM TKE tendency terms are added to the diagnostic TKE in YSU when topographic drag is turned on.

References:
Martilli et al. (2009), description of the modifications made in WRF.3.1 and short user's manual of BEP, NCAR technical note.
Hendricks, E. A., J. C. Knievel, and Y. Wang, 2020: Addition of multilayer urban canopy models to a nonlocal planetary boundary layer parameterization and evaluation using ideal and real cases, J. Appl. Met. Clim., submitted.

LIST OF MODIFIED FILES:
M phys/Makefile
A phys/module_bl_ysuurb.F
M phys/module_pbl_driver.F
M phys/module_physics_init.F

TESTS CONDUCTED:
Real-case 48-h simulations of Houston urban area were conducted (10/05/2017-10/07/2017) with an innermost nest grid spacing of 1 km. The following test suite was set up based upon discussions with Mike Barlage:

Baseline tests (version before physics enhancement):

  1. MYJ BEP
  2. MYJ BEP+BEM
  3. YSU BULK
  4. YSU SLUCM
    New code tests (new version with enhancement):
  5. YSU BULK
  6. YSU SLUCM
  7. YSU BEP
  8. YSU BEP+BEM
  9. MYJ BEP
  10. MYJ BEP+BEM

Hendricks et al. (2020) demonstrated that the scheme works properly and produces expected behaviors. Results are qualitatively similar to MYJ and BOULAC with BEP and BEP+BEM. Minor differences are due to the different handling of vertical mixing among the schemes.

  • Tests 7) and 8) were shown to produce the same behaviors as Hendricks et al. (2020).
  • Tests 5) and 6) were identical to tests 3) and 4), demonstrating that the new code did not affect the BULK and SLUCM existing code with YSU.
  • Tests 9) and 10) were identical to tests 1) and 2), demonstrating that the new code did not affect any existing BEP and BEP+BEM code in MYJ.

Since YSU is now functional with BEP and BEP+BEM, this line was added to replace the old lines in module_physics_init.F for all other PBL routines:

IF ((SF_URBAN_PHYSICS.eq.2).OR.(SF_URBAN_PHYSICS.EQ.3)) CALL wrf_error_fatal &
            ( 'module_physics_init: use ysu (option 1), myj (option 2), or boulac (option 8) with BEP/BEM urban scheme' )

This printout was shown to occur when attempting to run PBL schemes without BEP and BEP+BEM functionality.

The restart capability was demonstrated to function properly with the new YSU BEP and YSU BEP+BEM code.

RELEASE NOTE: The multilayer BEP and BEP+BEM UCMs are added to the YSU PBL scheme (joining MYJ and Boulac). Reference: Hendricks, E. A., J. C. Knievel, and Y. Wang, 2020: Addition of multilayer urban canopy models to a nonlocal planetary boundary layer parameterization and evaluation using ideal and real cases, J. Appl. Met. Clim., submitted.

@davegill davegill requested a review from a team as a code owner May 16, 2020 05:15
@davegill
Copy link
Contributor Author

@eahendricks
Eric,
You PR had some failures in the auto testing. Those were related to having a non-ideal starting point to cut your branch from (not really your fault, we introduced an inconsistency between the develop branch and the bug-fix branches). I took the existing develop branch from wrf-model and replayed your commits on top.

Here is the git log of this branch:

48f69bd45cc111fded8916f08f923926830d409a Few revisions to introductory comments.
dc994a8c4daed37f9125700c68b730664a156a6c Reference title correction.
6858afcc8ef19c93ffbb8744fe7df724e1f4bd8f Added some more comments to module description and rename a variable.
aff3d80462b2bceaa30d9c0ebc42b18741fb8ba3 Remove all unnecessary comments from YSU BEP/BEP+BEM code prior to adding to develop
4e4ab22880e53a46536c64b482dd4315d05b69ce Modify print statements in module_physics_init. Bug fix in module_bl_ysuurb.
063508943ece39d14ab3d215545d2997004fd88e Final YSU changes, comments still included
0f61c1f0edb2cc53beec74c2426c6c6cf3cd0574 Merge remote-tracking branch 'origin/master' into develop
0d88b5800186d892a882e118425d2e684ded756f Merge remote-tracking branch 'origin/release-v4.2'

Here is a git log of the original PR branch:

624b7482ed7bfe9c5688574cf81f2bfccec62c24 Few revisions to introductory comments.
754e8d9b1b64ebdf68e16e7667319ed3e166d4fb Reference title correction.
7f8653593537920db195ef7997abee7ce27190c9 Added some more comments to module description and rename a variable.
5d487666bc99bbe987c41a682562339ac2d6f37c Remove all unnecessary comments from YSU BEP/BEP+BEM code prior to adding to develop
33424ce958aa5ca93a8a40ded67257cd8f5d53e5 Modify print statements in module_physics_init. Bug fix in module_bl_ysuurb.
d273e81419caa8255ca2bc8315888b0ede0b25c5 Final YSU changes, comments still included
d96531bb27652a766930ad877506c5fb97f1b8a5 AHI Cloud Detection based on IR Cloud Mask (#1139)
877c17c42be812091331b732a87085233c1c97a8 Remove git conflict markers in phys/module_surface_driver.F (#1144)
88aea06014db6b066eef7ca587038e2d3a6ecb5c Merge remote-tracking branch 'origin/master' into develop
7df4461d0c7e2dd86fa83309172054bd9a396100 Merge remote-tracking branch 'origin/release-v4.1.5'

Note that the top several hashes have changed, but your commits are all intact.

If you are interested in reconstructing what I have done:

  1. I picked up your repo and branch
git clone https://github.com/eahendricks/WRF
cd WRF
git checkout ysubepbem
  1. I picked up the current develop branch from WRF
git remote add wrf https://github.com/wrf-model/WRF
git fetch wrf
git checkout wrf/develop
git checkout -b wrf-develop
  1. I make a new branch to modify
git checkout -b wrf-develop+ysubepbem
  1. Use the cherry pick range feature (first hash is not included). From the above list, this is the AHI Cloud Detection based on IR Cloud Mask (#1139) commit (again, not used, but just prior to the start of your mods) through the Few revisions to introductory comments. commit (which is your last mod).
git cherry-pick d96531bb27652a7..624b7482ed7bfe9c

@davegill
Copy link
Contributor Author

@eahendricks
Eric,
This code will not give bit-wise identical results to your branch since there are missing updates in your original PR. However, If you could do a few spot checks on this PR, that would be great.

@davegill
Copy link
Contributor Author

davegill commented May 16, 2020

@kkeene44 @eahendricks @weiwangncar @dudhia

The restart capability was demonstrated to function properly with the new YSU BEP and YSU BEP+BEM code.

Kelly,
Would you get the specifics from Eric on the testing that was used to validate the restart capability.

Wei and Jimy,
Where do we add information to the User's Guide about this update?

CALL shinhonginit(RUBLTEN,RVBLTEN,RTHBLTEN,RQVBLTEN,&

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an unnecessary added blank line. I'd remove it, but I don't want to trigger a full regression test just for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry about that, Dave. Must have accidentally added that space when modifying the wrf_error_fatal print.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eahendricks
Eric,
This change could be handled entirely on github. However, go ahead and clone this repo, make the change, do the git add - commit - push.

@davegill
Copy link
Contributor Author

jenkins:

    Test Type              | Expected  | Received |  Failed
    = = = = = = = = = = = = = = = = = = = = = = = =  = = = =
    Number of Tests        : 19           19
    Number of Builds       : 48           48
    Number of Simulations  : 166           166        0
    Number of Comparisons  : 105           105        0

    Failed Simulations are: 
    None
    Which comparisons are not bit-for-bit: 
    None

@eahendricks
Copy link
Contributor

@davegill Dave, thanks for the information and creating a new pull request. Should we close out #1195, or leave it as is for now? This is my first pull request to WRF develop, and also not the most savvy GitHub user. Appreciate your help.

@davegill
Copy link
Contributor Author

@eahendricks
Eric,
Yep, let's close #1195.

If there are mods to make (and there always are), you want to make them to my fork and this branch. Let me know if you have questions.

@weiwangncar
Copy link
Collaborator

@eahendricks @davegill Very nice...

@weiwangncar
Copy link
Collaborator

@eahendricks As a general question, is it possible to make this one ysu call instead of two (one with urban and one without)? This could in general be a good idea so that one module is not diverging from another.

@davegill
Copy link
Contributor Author

@eahendricks

is it possible to make this one ysu call instead of two

Eric,
I agree with Wei. If this is not an unrealistic request, a single routine is better. Otherwise, you are maintaining two files every time there is a mod to the YSI PBL scheme.

@eahendricks
Copy link
Contributor

@weiwangncar @davegill Yes, it could be one routine. I thought about doing that initially but elected not to for the following reasons: 1) the YSU developers may not want all the urban variables and a bunch of IFDEFS throughout their routine, 2) there is past precedent for separation of urban PBL schemes in WRF, e.g., for MYJ, module_bl_myjpbl.F (without BEP) and module_bl_myjurb.F (with BEP).

@davegill
Copy link
Contributor Author

@eahendricks @weiwangncar @dudhia

Yes, it could be one routine.

Eric,
Let's get more of a quorum so we know what the larger group is thinking. So, hold off on any substantive mods right now.

@weiwangncar
Copy link
Collaborator

weiwangncar commented May 17, 2020

@eahendricks Good point. But looking at module_bl_myjurb.F, it hasn't been modified since Mar 2010, while module_bl_myjpbl.F has with last change made in Sept 2012. So the two routines are already different. We could consult the YSU authors to see what they think.

@eahendricks
Copy link
Contributor

@davegill @weiwangncar Sounds great, Dave and Wei. I'm happy to add the mods to the original YSU routine if the larger group agrees. I thought about this a little today, and I could do this quite efficiently with just a few if statements. My vote would be one routine for the reasons you guys mentioned (diverging, maintaining two files).

@kkeene44
Copy link
Collaborator

@eahendricks
Can you send me (or attach) the namelist that you used for the restart test? Thanks!

@eahendricks
Copy link
Contributor

@kkeene44 Yes, I can do this. Let's hold off for now until we hear back from the larger group. A decision will be made whether put the urban changes in the main YSU routine or the separate urban YSU routine. If the decision is the former, this PR will be closed, and a new one will be opened in the future.

@davegill
Copy link
Contributor Author

@dudhia @weiwangncar
Folks,
What is the status of this? Are we waiting for Hong's group to weigh in?

@dudhia
Copy link
Collaborator

dudhia commented May 19, 2020 via email

@eahendricks
Copy link
Contributor

Hello, did we hear back from Hong's group yet?

@dudhia
Copy link
Collaborator

dudhia commented Sep 16, 2020 via email

@weiwangncar
Copy link
Collaborator

Is there a way for us to see how much code is modified in YSU?

@dudhia
Copy link
Collaborator

dudhia commented Sep 16, 2020

I think our suggestion is to propose a commit in an existing scheme. Maybe Shin-Hong. But it would be good to know the size of the differences in that case.

@eahendricks
Copy link
Contributor

@weiwangncar I attached a diff file. The changes are not too substantial, but many new arrays are passed into YSU.
@dudhia These changes are for YSU only (option 1), not Shin-Hong (option 11). The multilayer UCMs could be added to Shin-Hong in the future perhaps, but that would be a different commit.

If you think the size of the difference is not too great, then I agree, it would be best from a code maintenance perspective to have one routine. I can ask Hailey her opinion as well.

diff_ysu_ysuurb.log

@dudhia
Copy link
Collaborator

dudhia commented Sep 16, 2020 via email

@eahendricks
Copy link
Contributor

@dudhia Good to know. I can definitely do that in the future.

@eahendricks
Copy link
Contributor

@cloudyti This pull request was replaced by #1305. This one (#1196) can be canceled/deleted. The pull request needing approval is #1305. I just want to make sure there is no confusion, and the right code changes get implemented into WRF. Sorry for any confusion.

@davegill davegill closed this Feb 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants