Gfs ddt removal #493
Gfs ddt removal #493
Conversation
|
This supercedes #382. |
…M), GFS_rrtmg_post
There was a problem hiding this comment.
I am not sure whether we should make changes to physics/GFS_phys_time_vary.scm.{F90,meta} and physics/GFS_rad_time_vary.scm.{F90,meta} at this point. For FV3, we will need to create timestep_init and timestep_final phases and run the time_vary group in the timestep_init phase (without threading over blocked data structures). It would probably be better to hold off changing the SCM version of those files and make the same changes as for FV3 later. The rest looks good. Changing several of the standard names (time_step to timestep) requires an fv3atm PR.
Yes, I debated whether or not to do anything with the SCM time_vary files, but I was thinking that since the SCM isn't passing in the "all blocks" versions of the DDTs that it was OK to clean these up like other files. I know that you are going to fix the time_vary routines, but I had forgotten that you were going to implement them as the new phase. So, I was trying to save you some work, but I'll go ahead and remove these files. As far as propagating GFS_typedef changes to fv3atm, I am planning on doing this. There are other changes besides the time_step -> timestep one that need to propagate. |
|
@climbfuji, I'm still in the process of cleaning this up, BTW, hence the draft status. Regarding the time_vary stuff, the code will still need to exist even in the new timestep_init phase right? Will this phase still rely on GFS DDTs? If not, then perhaps some of the changes in these files can be used. I'll clean these up (formatting), commit that so that the files could be used if necessary and there will be a record of their existence in their cleaned up state, then commit the originals back. |
|
Two questions: One for @climbfuji: Based on your comment in another PR, every array in every CCPP entry point should be assumed shape going forward? One for @dustinswales: This PR is adding the diagnostic fluxr array as a CCPP variable. If I were to uncomment the sections in GFS_rrtmgp_[sw,lw]_post.F90, and pass fluxr in to populate, would all of the antecedent variables be populated by RRTMGP, as far as you know, such that fluxr could be populated correctly? |
It would be safer to use assumed-size arrays everywhere, yes. But it is not mandatory. |
|
@grantfirl |
|
@grantfirl now that the "RRTMG cloud overlap method update" PR #487 was merged we need to update this PR. Do you have time to do this, or should I do it as part of merging your PR into the newly created feature/transition-to-capgen-1 branch? |
|
I'm happy to do this, but does it need to be done ASAP? I would like to
take the afternoon off.
…On Fri, Sep 25, 2020 at 9:51 AM Dom Heinzeller ***@***.***> wrote:
@grantfirl <https://github.com/grantfirl> now that the "RRTMG cloud
overlap method update" PR #487
<#487> was merged we need to
update this PR. Do you have time to do this, or should I do it as part of
merging your PR into the newly created feature/transition-to-capgen-1
branch?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#493 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGCO5UB3FNFWPMM7R7AIRZDSHS4B5ANCNFSM4QHYFRRA>
.
--
*Grant J. Firl*
Project Scientist I
Joint Numerical Testbed
Research Applications Laboratory
National Center for Atmospheric Research
FL3-1016: (303) 497-2872
|
|
Monday morning would be ok, too. If I get to it over the weekend, I'll let you know - enjoy your free afternoon, second last day of summer!
… On Sep 25, 2020, at 11:31 AM, grantfirl ***@***.***> wrote:
I'm happy to do this, but does it need to be done ASAP? I would like to
take the afternoon off.
On Fri, Sep 25, 2020 at 9:51 AM Dom Heinzeller ***@***.***>
wrote:
> @grantfirl <https://github.com/grantfirl> now that the "RRTMG cloud
> overlap method update" PR #487
> <#487> was merged we need to
> update this PR. Do you have time to do this, or should I do it as part of
> merging your PR into the newly created feature/transition-to-capgen-1
> branch?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#493 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGCO5UB3FNFWPMM7R7AIRZDSHS4B5ANCNFSM4QHYFRRA>
> .
>
--
*Grant J. Firl*
Project Scientist I
Joint Numerical Testbed
Research Applications Laboratory
National Center for Atmospheric Research
FL3-1016: (303) 497-2872
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#493 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RNGJIOUSKHIPBRUOKDSHTHXJANCNFSM4QHYFRRA>.
|
|
@grantfirl I just realized that updating this PR isn't necessary, I'll do that anyway when I pull it into the feature/transition-to-capgen-1 branch. But I need to ask you to look at the CCPP tendencies PR ... will post a note there. Thanks! |
Even though there are conflicting files in this PR? |
I can solve these conflicts when I pull in the code, no problem. |
|
|
||
| real(kind=kind_phys), dimension(:), intent(inout) :: coszen, coszdg | ||
|
|
||
| real(kind=kind_phys), dimension(:,:), intent(inout) :: phy_f3d_leffr, & |
There was a problem hiding this comment.
Why do these effective radii carry the prefix phy_f3d_ - this routine shouldn't care at all where/how they are stored in the host model?
There was a problem hiding this comment.
Very good question. I had no idea what I was thinking until I looked at the whole file again. I believe that my thought process was along the lines of this: due to the fact that this is an interstitial scheme that came from GFS_physics_driver, where DDTs were used nearly everywhere, for code continuity reasons, it may make sense to put where these variables came from in their Fortran names. This is a pretty weak argument, though. From a CCPP point of view, it probably makes sense to drop the phy_f3d part. I'll push a commit to remove the phy_f3d in Fortran names unless you buy the original reasoning.
There was a problem hiding this comment.
I agree with dropping the phy_f3d part, thanks for the explanation. Can you please wait until my next PR is out there that merges your GFS DDT removal into the feature/transition-to-capgen-1 branch? This way I won't have to fix merge conflicts again. Thanks!
There was a problem hiding this comment.
Yes, I'll look out for that.
|
This PR was merged into #506 for NCAR's feature/transition-to-capgen-1 branch. These changes will be merged into the authoritative branch in a couple of weeks, and this PR should be flagged as "merged' automatically. |
This pull request is part of a cleanup of ccpp-physics required to transition to the new CCPP code generator. It removes references to GFS-based (host-dependent) derived date types (DDTs) for all EXCEPT the following files (reasons in parentheses):
This PR requires associated PRs in the host models since new metadata (and a few new array indices) are required:
NOAA-EMC/fv3atm#162
NCAR/ccpp-scm#197
A few other partial cleanup tasks were undertaken when editing files, such as, changing horizontal_domain to horizontal_loop_extent in the dimensions of the metadata, a few changed names (time step -> timestep), fixing ??? units, and using assumed-shape for some variables that I touched.