CIRES UGWP (unified gravity wave physics scheme) to be added

[bluefinweiwei]

No description provided.

[grantfirl]

A couple of comments that will need to be addressed at some point:

1. It looks like file permissions were changed on all of the files in the physics dir. Why?
2. When the PR is ready to merge, there should be no autogenerated code (caps) in the repo. The only changes should be the files that UGWP is bringing in. So, rather than there being 240 files changed, there should be maybe on the order of 10, I'm guessing.

[grantfirl]

associated PRs:
https://github.com/NCAR/FV3/pull/130

[grantfirl]

Also, you'll want to update this branch to the latest master. I see that it is behind NCAR/master by 56 commits.

[grantfirl]

This should probably be removed until v1 is being worked on. Otherwise, it is confusing to have two cires_ugwp modules in here.

[bluefinweiwei]

done

[grantfirl]

We need to understand what Valery is trying to do with tendencies. It seems like what was added is different than what other schemes have done, so we need to understand if he really needs to do something different or he didn't understand how tendencies were done previously for other schemes. According to what he added to GFS_physics_driver, PdXdt are zeroed out every timestep within the driver. If we're keeping what he has done, these variables should be added to the GFS_interstitial_type in GFS_typedefs.F90 (they are zeroed out before every physics call in a reset routine, so the initialize statements would not need to be here).

[grantfirl]

The same comment applies to these (TdXdt) variables too (need to understand what Valery intends with these). It appears that these variables should be diagnostics that are zeroed out periodically but not every timestep. If so, they belong in the GFS_diag_type of GFS_typedefs.F90 and they should be zeroed out with the other diagnostics periodically in GFS_typedefs.F90/diag_phys_zero.

[grantfirl]

GFS derived type instances should not be passed into schemes for portability. We should be passing in components of these types directly as needed. This has been relaxed in the past due to expediency concerns, but we should not be doing this going forward, especially for new CCPP schemes.

[bluefinweiwei]

Done. All GFS derived types (Statein, Stateout, Sfcprop, and Diag) have been replaced with the relevant components.

[grantfirl]

GFS derived type instances should not be passed into schemes for portability. We should be passing in components of these types directly as needed. This has been relaxed in the past due to expediency concerns, but we should not be doing this going forward, especially for new CCPP schemes. (applies to Model, Statein, Sfcprop)

[bluefinweiwei]

Done. All GFS derived types (Statein, Stateout, Sfcprop, and Diag) have been replaced with the relevant components.

[grantfirl]

If we want to keep how VAY did diagnostics, this will need to be put in a "pre" interstitial routine.

[bluefinweiwei]

Done.

[grantfirl]

This code (except for the calculation of sgh30) is currently in gwdps_pre_run in gwdps.F. We'll need to think about where it belongs (probably moving the stuff out of gwdps.F into its own GWD_generic file now that there is more than one GWD scheme.)

[grantfirl]

There is also a bunch of diagnostics-related stuff the VAY does in GFS_physics_driver.F90 that will need to go into a "pre" interstitial (IF the way that VAY did his diagnostics is really necessary).

[grantfirl]

lines 680-765 and 1669-1683 in GFS_physics_driver.F90 from VAY

[bluefinweiwei]

What is GWD_generic file? Where is it?

[grantfirl]

It doesn't exist yet. We had been separating interstitial code into "scheme-specific" interstitial code that only gets executed when a specific scheme is active, or "scheme-generic" code that gets executed as part of a suite for any scheme of that type, i.e. for any GWD scheme. When we originally wrote the interstitial schemes (I believe Gerard Ketefian did the GWD schemes), there was only one GWD scheme, so the interstitial code was scheme-specific by default. Now that there are more than one GWD schemes, we should create a new file called GFS_GWD_generic.F90. It should at least contain what is in gwdps_pre_run in gwdps.f and maybe more, depending on what we find out from VAY about the diagnostics and tendencies code in GFS_physics_driver.F90.

[bluefinweiwei]

Make sense. thx!

[grantfirl]

This is a bit of a mess (not your fault). The updating of the state is done in an interstitial (something like GFS_suite_stateout_update in GFS_suite_interstitial.F90). I don't know why VAY didn't just add to the dtdt, dudt, dvdt variables that already had accumulated tendencies.

[grantfirl]

There are multiple sections of GFS_physics_driver.F90 that would need to go into separate "post" interstitial schemes if we were to just make what VAY wrote CCPP-compliant since they need to be called at specific times within the execution of the suite. I'd like to avoid that. We need to figure out exactly what VAY is trying to accomplish when ldiag_ugwp is T to try to do it cleaner in CCPP. Lines 3729-3731 would be yet another little interstitial scheme.

[bluefinweiwei]

Note has been taken for the comments about diag. This fork is a little messy due to my poor git skill. I've created another fork "ugwp" and will submit a PR for it and close the PR for "cires_ugwp" soon.