Feature: CCPP compliance #1
Conversation
This reverts commit 7d2d5a5.
| @@ -1,27 +1,28 @@ | |||
| #define NEED_B4B_DURING_CCPP_TESTING 1 | |||
| !================================================================================================================= | |||
| module bl_ysu | |||
| use ccpp_kinds,only: kind_phys | |||
There was a problem hiding this comment.
@ldfowler58 My suggestion would be to use a module with a non-ccpp specific name for the working precision. Plus, ccpp_kinds has conflicts within the ccpp-physics.
Here I pulled in the ccpp working precision, just to get the code working.
|
bl_gwdo.F: Can you please clarify the addition of the logical l_norot in the subroutine bl_gwdo_run. I do not think that it helps the definitions of 1 and v1. Thanks. The rotation of the u and v winds depends on the value assigned to cosa and sina which are needed in the argument list and initialized in the host model, independently of l_norot. So the initialization of l_norot would actually depends on the values of cosa and sina themselves. Using l_norot seems to add complexity to the definition of u1 and v1. line 233: it should be v1(i,k) = vproj(i,k) |
I added l_norot so we could use this scheme in the case where the grids have already been rotated (e.g. UFS). It is harmless and doesn't change any current functionality of the scheme if l_norot is not provided. Yes, line 233 is a bug. I will fix |
|
I was not aware UFS is already rotated because the GWD input data assumes
lat/long winds.
…On Tue, Sep 12, 2023 at 11:01 AM Dustin Swales ***@***.***> wrote:
bl_gwdo.F: Can you please clarify the addition of the logical l_norot in
the subroutine bl_gwdo_run. I do not think that it helps the definitions of
1 and v1. Thanks.
The rotation of the u and v winds depends on the value assigned to cosa
and sina which are needed in the argument list and initialized in the host
model, independently of l_norot. So the initialization of l_norot would
actually depends on the values of cosa and sina themselves. Using l_norot
seems to add complexity to the definition of u1 and v1.
line 233: it should be v1(i,k) = vproj(i,k)
I added l_norot so we could use this scheme in the case where the grids
have already been rotated (e.g. UFS). It is harmless and doesn't change any
current functionality of the scheme if l_norot is not provided.
Yes, line 233 is a bug. I will fix
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77DFOS4VWKIIKWRVYODX2CIPBANCNFSM6AAAAAAYLGB7P4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Regarding bl_gwdo.F, Jimy Dudhia and I do not see the need to add the logical l_norot with regard to the wind rotation. I feel that it is implicitly defined through sina and cosa. Can you please remove that related sourcecode. Also, can you please remove the subroutines bl_gwdo_timestep_init and bl_gwdo_timestep_final since they are not used. Thanks. |
@ldfowler58 @dudhia I can remove the rotation piece, but not sure that it's "implied" that they are rotated? If sina and cosa were optional arguments, the scheme could query their presence to decide whether to rotate or not. |
|
Although Dustin made some changes in bl_gwdo* files, my understanding was that we had only completed implementing bl_ysu* and sfclayrev* but not bl_gwdo*. In fact, GWD is not part of our proposal. But if @ldfowler58 finds the mods helpful, you are welcome to adopt them. I just would like to confirm with @dustinswales: Are the modified bl_gwdo* files ready for Laura and her team to implement in their next public release? Thank you very much for your time, Dustin. |
|
Yes, since no change was made to the gwdo meta file, I think we can just
ignore the changes for this scheme,
…On Wed, Dec 6, 2023 at 1:27 PM Weiwei ***@***.***> wrote:
Although Dustin made some changes in bl_gwdo* files, my understanding was
that we had only completed implementing bl_ysu* and sfclayrev* *but not
bl_gwdo**. In fact, GWD is not part of our proposal. But if @ldfowler58
<https://github.com/ldfowler58> finds the mods helpful, you are welcome
to adopt them. I just would like to confirm with @dustinswales
<https://github.com/dustinswales>: Are the modified bl_gwdo* files ready
for Laura and her team to implement in their next public release? Thank you
very much for your time, Dustin.
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77FTRJNVQBG2YFQMPADYIDIJLAVCNFSM6AAAAAAYLGB7P6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTGYZTOMJYGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Some history... Originally, I modified all three schemes to be run from drivers that lived in the ccpp repository. This was not desired. Then I went back and made the YSU scheme a direct ccpp entrypoint, hence the heavy modifications to bl_ysu.meta. Now there is no driver in the ccpp repository for MMM YSU PBL, instead there are _pre() and _post() and bl_ysu is built into a custom SDF with these _pre and _post routines. Anyways, the GWD and surface layer schemes work in the SCM, but as (the less desirable) drivers. The pieces that are in the drivers need to be migrated into the scheme, analogous to what I did for YSU. |
@dudhia @ldfowler58 |
|
Thanks so much for the prompt clarifications, @dustinswales. Very Helpful. I will leave it up to @ldfowler58 to decide if she wants this PR to only reflect bl_ysu. Laura, if bl_gwdo* is desired, @scrasmussen or I can work to refactor the code with a structure including *pre and *post. As @dudhia said, the *meta file will need many changes. |
|
Please see #4 for only YSU changes. |
|
In bl_ysu.F90, you added the variables dusfc, dvsfc, dtsfc, and dqsfc in the argument list of subroutine bl_ysu.F whereas they are defined locally in the original sourcecode. Are they needed in other parameterizations or do you just need them as diagnostics? Thanks, |
This PR contains changes for these shared physics parameterizations to work within UFS physics suites.