add Ferrier-Aligo MP scheme changes on host model side#2
Conversation
mzhangw
requested review from
climbfuji,
grantfirl and
llpcarson
as code owners
| <group name="radiation"> | ||
| <subcycle loop="1"> | ||
| <scheme>GFS_suite_interstitial_rad_reset</scheme> | ||
| <!-- <scheme>HAFS_update_moist</scheme> --> |
There was a problem hiding this comment.
It's probably best to remove commented out schemes before merging. Also, I don't see that HAFS_update_moist is ever actually called. Based on the name of the suite, is that the intended behavior?
| <scheme>samfdeepcnv</scheme> | ||
| <scheme>GFS_DCNV_generic_post</scheme> | ||
| <scheme>GFS_SCNV_generic_pre</scheme> | ||
| <!-- <scheme>samfshalcnv</scheme> |
There was a problem hiding this comment.
It would be good to add in the PR description why the shallow convection scheme is left out.
| Diag%zmtnblck = zero | ||
|
|
||
| #ifdef CCPP | ||
| if (Model%imp_physics == Model%imp_physics_fer_hires) then |
There was a problem hiding this comment.
If variables are conditionally-allocated here, they should probably have assumed-shape within the scheme. We've run into problems with conditionally-allocated variables with explicit shape in the schemes.
| !--- F-A scheme | ||
| elseif (Model%imp_physics == Model%imp_physics_fer_hires) then | ||
| if (Model%spec_adv) then | ||
| Interstitial%nvdiff = 5 !qv, qc, qr, qi, qrime |
There was a problem hiding this comment.
I'm guessing this is temporary? Why would you have an 'if test' when you're setting nvdiff to the same value anyway?
| type = real | ||
| kind = kind_phys | ||
| [train] | ||
| standard_name = accumulated_tendency_of_air_temperature_due_to_FA_scheme |
There was a problem hiding this comment.
Others may want to weigh in, but if you're just accumulating the change in a variable, call it "accumulated_change_of...". If it's really a tendency, its units would need to be K s-1.
| name = GFS_interstitial_type | ||
| type = ddt | ||
| [qv_r] | ||
| standard_name = humidity_mixing_ratio |
There was a problem hiding this comment.
why not "water_vapor_mixing_ratio"?
2. in GFS_typedefs.F90, correct nvdiff definition for FA.
Update CAPS physics to master (second round) for final testing
climbfuji
left a comment
climbfuji
left a comment
There was a problem hiding this comment.
This looks ok to me. I noted checking that all conditionally-allocated arrays must have assumed-size declaration in subroutines (e.g. train(:) instead of train(1:im)). Given that all regression tests pass, it is ok to me to merge this set of PRs (provided that @grantfirl approves it as well) and make the changes later (but an issue should be created to check and if necessary fix the problem).
| @@ -0,0 +1,87 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
|
|
|||
| <suite name="FV3_HAFS_ferhires_update_moist" lib="ccppphys" ver="3"> | |||
There was a problem hiding this comment.
I am just wondering if this is a suite name we want to keep in the future. How much information needs to go into the suite name / suite definition filename? Not something to address for this PR, but in general in the near future.
| <scheme>GFS_DCNV_generic_pre</scheme> | ||
| <scheme>get_phi_fv3</scheme> | ||
| <scheme>GFS_suite_interstitial_3</scheme> | ||
| <!-- <scheme>samfdeepcnv</scheme> --> |
There was a problem hiding this comment.
I think @grantfirl was asking to remove those commented-out schemes.
| Diag%zmtnblck = zero | ||
|
|
||
| #ifdef CCPP | ||
| if (Model%imp_physics == Model%imp_physics_fer_hires) then |
| allocate (Interstitial%cnv_ndrop (IM,Model%levs)) | ||
| allocate (Interstitial%cnv_nice (IM,Model%levs)) | ||
| end if | ||
| if (Model%imp_physics == Model%imp_physics_fer_hires) then |
There was a problem hiding this comment.
Same here, all these variables should be passed in as assumed-size (:,:)
Add README.md containing disclaimer
Adding Eddy Diffusivity Output
The HAFS related developments for the write_grid_component (NCAR#10)
3 participants
This PR includes changes of host model side for F-A MP scheme.