Add namelist options for Tiedtke CP and introduce wind and PBLH dependent mixing-length limits in GFS PBL#359
Conversation
…s option in Tiedtke CP.
…_ntiedtke.meta to add a new namelist variable, icu_zoentr, for using a different entrainment equation
physics/CONV/nTiedtke/cu_ntiedtke.F90 and physics/PBL/SATMEDMF/satmedmfvdifq.F
| if ( icu_zoentr .eq. 2 ) then | ||
| zoentr(jl) = (c1+d1*(1.0-min(1.,pqen(jl,jk)/pqsen(jl,jk))))* & | ||
| (pgeoh(jl,jk)-pgeoh(jl,jk+1))*zrg | ||
| end if |
There was a problem hiding this comment.
There is no catchall guard here for icu_zoentry not in [1,2].
Same for line 2498 below.
There was a problem hiding this comment.
Thank you for your comments. Regarding the catchall guard for the option icu_zoentr, I could modify the code so that the code can stop with warning message if the icu_zoentr is neither 1 nor 2 as shown in the below example. If you think it would be a good idea to modify the code as shown below, please let me know.
If( icu_zoentr .eq. 1) then
original entrainment method
elseif ( icu_zoentr .eq.2 ) then
new entrainment method
else
print('Error: unsupported icu_zoentry of', icu_zeontry)
stop
endif
There was a problem hiding this comment.
With CCPP, the correct way is to write the error message to the errmsg string, set errflg to a value other than zero (1 is fine), and return immediately. We shouldn't stop the model with stop.
There was a problem hiding this comment.
Ok.. I checked the cu_ntiedtke.F90 code to understand your instruction. Then, do you think the below example satisfies the CCPP standard? If you think this modification looks fine, shall I change to the code as shown below?
If( icu_zoentr .eq. 1) then
original entrainment method
elseif ( icu_zoentr .eq.2 ) then
new entrainment method
else
write(errmsg,'(*(a))') 'Error: unsupported icu_zoentr’
errflg = 1
return
endif
There was a problem hiding this comment.
Yes, that looks correct to me. Thanks!
There was a problem hiding this comment.
Thank you. Then, I will modify and will test the codes. I will push the changes and will come back to you after I complete the tests.
There was a problem hiding this comment.
I tested cu_ntiedtke.F90 after I changed the code as the example shown above to add the catchall guard for the option icu_zoentr. If I used icu_zoentr=2, which is a supported option, the code worked properly. I also tested the code with the unsupported option icu_zoentr=3, and the forecast job crashed (as expected) with the error message shown below. This error message will tell users that the code doesn't work except for options 1 and 2.
An error occurred in ccpp_physics_run for group phys_ts, block/chunk 1 and thread 1 (nt= 1):
An error occured in cu_ntiedtke_run: Error: unsupported icu_zoentr
Log file location: /scratch3/HFIP/hwrfv3/scrub/Junghoon.Shin/hafsv2p2_final_change_test/2024100512/14L/hafs_atm_init.log
If you think this looks fine, I will push this change to the feature/hafs_tiedtke_varml branch for the code review.
Thank you.
There was a problem hiding this comment.
I just pushed the changed cu_ntiedtke.F90 code to the feature/hafs_tiedtke_varml branch.
Please review the code and let me know if you have any further comments. Thank you.
| ! | ||
| implicit none | ||
| !--- input arguments: | ||
| integer, intent(in) :: scale_fac_opt,icu_zoentr |
There was a problem hiding this comment.
I think it might be useful to add links to the two issues that describe the need for these parameters and what they do as comments above line 203.
There was a problem hiding this comment.
The option scale_fac_opt (alternative scale-awareness method) is related to the issue #357 from AndrewHazelton while the option icu_zoentr is related to the issue #358. I can add these two "links" as comments in the code near line 203.
#357 (scale_fac_opt)
#358 (icu_zoentr)
Description of Changes:
One or more paragraphs describing the problem, solution, and required changes.
Tests Conducted:
Explicitly state what tests were run on these changes, or if any are still pending (for README or other text-only changes, just put "None required". Make note of the compilers used, the platform/machine, and other relevant details as necessary. For more complicated changes, or those resulting in scientific changes, please be explicit!
OR Add any links to tests conducted. For example, "See ufs-community/ufs-weather-model#<pr_number>"
Testing have been done with the UFS hurricane application (HAFS).
Dependencies:
Add any links to parent PRs (e.g. SCM and/or UFS PRs) or submodules (e.g. rte-rrtmgp). For example:
Documentation:
Does this PR add new capabilities that need to be documented or require modifications to the existing documentation? If so, brief supporting material can be provided here. Contact the CODEOWNERS if your PR requires extensive updates to the documentation. See https://github.com/NCAR/ccpp-doc for Technical Documentation or https://dtcenter.org/community-code/common-community-physics-package-ccpp/documentation for the latest Scientific Documentation.
N/A
Issue (optional):
If this PR is resolving or referencing one or more issues, in this repository or elewhere, list them here. For example, "Fixes issue mentioned in #123" or "Related to bug in NCAR/other_repository#123"