Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RRTMGP in fv3atm #26

Closed
wants to merge 988 commits into from
Closed

Conversation

dustinswales
Copy link
Collaborator

This PR contains changes to the fv3atm required for CCPP enabled with the RRTMGP radiation scheme: NCAR/ccpp-physics#400

akubaryk and others added 30 commits April 17, 2019 15:27
Henry Juang's changes for MULTI_GASES, incorporating
variable R and Cp for virtual temperature calculations.

Also adds an option for tau (used in Rayleigh damping) to be negative,
which allows for the GSM calculation of the time constant.
Roughly, FV3 tau=0.022 is GSM tau=-10 (10-day time constant in GSM).

Change-Id: I59edc57e8c0ec04390bc18158a96798e6c34f5a8
ccpp_prebuild version of multi-suite static build, cleanup validation of SDF used
…90419

gsd/develop: bugfix for sncovr (compilation error)
… calculate Sfcprop%sr for GFDL-MP in PROD+TRANSITION mode
…b_master_20190417_BASED_ON_static_build_xml_validation_20190326

GitHub CCPP update with VLab master changes 20190419
…cumulated diagnostic fields and #61740 Incorrect units in calculation of Sfcprop%srflag for GFDL-MP; adapted for CCPP version of the code
…e variable for calculation of sr (Sfcprop%sr) to achieve b4b-identical results in PROD+TRANSITION mode
…_fhgoc3d

gsd/develop: fix units fhzero fhcyc fhgoc3d
…b_master_20190429

GitHub CCPP update to VLab master 2019/04/29
…tests_20190502

Update/cleanup gsd/develop: rename Thompson MP
…-precision constant in TRANSITION mode to avoid b4b mismatches of Sfcprop%sr in fv3_streched_nest versus fv3_ccpp_stretched_nest regression tests
…mtb-ccpp_20190503

gsd/develop merge in gmtb/ccpp 2019/05/03
…mtb-ccpp_20190503

gsd/develop merge in gmtb/ccpp 2019/05/03
…gsd-develop_20190506

Merge gsd/develop into gmtb/ccpp 2019/05/06
… changes in results if current default non-frac landmask is used.

CCPP modifications by @grantfirl and @climbfuji
…ice to replicate functionality in GFS_physics_driver
…r GFS_surface_composites_{pre,post} schemes, comment out debugging print statements
…b_master_20190507

Fractional landmask changes in GFS physics
…specific namelist sections from general gfs_physics namelist section, set MYNN EDMF namelist options automatically, depending on do_mynnedmf
Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if the ccpp-physics and ccpp-framework are up to date. For ccpp-framework, your PR should point to the head of the dtc/develop branch. For ccpp-physics, your corresponding branch should be updated with the head of the dtc/develop branch.

ccpp/config/ccpp_prebuild_config.py Show resolved Hide resolved
@@ -5908,6 +6071,52 @@ subroutine interstitial_create (Interstitial, IM, Model)
allocate (Interstitial%zorl_land (IM))
allocate (Interstitial%zorl_ocean (IM))
allocate (Interstitial%zt1d (IM))
! RRTMGP
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these arrays are used exclusively by RRTMGP, they should only be allocated if RRTMGP is used. Need to make sure that if these arrays get passed into subroutines that are also used by other physics (without using the arrays inside, then its dimensions must be specified as assumed-size in the intent-declaration of the subroutines).

@@ -23,6 +23,11 @@ CPPDEFS += -DNEW_TAUCTMAX -DSMALL_PE -DNEMS_GSM -DINTERNAL_FILE_NML

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes should be required to this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to get FV3 to build unless the rte-rrtmgp source files were added here. This was something @grantfirl and I came up with back in the fall to get it to build.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember this now ... we need to find a better way to do this. In particular because the make build is phased out as we speak in favor of a cmake build, hence the proposed changes would be meaningless.

gfsphysics/physics/GFS_debug.F90 Outdated Show resolved Hide resolved
io/FV3GFS_io.F90 Show resolved Hide resolved
ccpp/suites/suite_FV3_GFS_2017_RRTMGP.xml Outdated Show resolved Hide resolved
ccpp/suites/suite_FV3_GFS_2017_gfdlmp_rrtmgp.xml Outdated Show resolved Hide resolved
ccpp/suites/suite_FV3_GFS_2017_rrtmgp.xml Outdated Show resolved Hide resolved
gfsphysics/GFS_layer/GFS_typedefs.F90 Outdated Show resolved Hide resolved
gfsphysics/GFS_layer/GFS_typedefs.F90 Show resolved Hide resolved
Copy link
Collaborator Author

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All requested changes have been reverted. Question still remains regarding gfsphysics/makefile.

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes. Besides the makefile question, we still have to fix the conditional allocation of the RRTMGP arrays in GFS_typedefs.F90 (i.e. allocate only if RRTMGP is used).

Regarding the timeline for the commit. We are scheduled to send dtc/develop to EMC for inclusion in the authoritative branches mid of this month. I prefer to merge the RRTMGP PRs afterwards so that the current PR going to EMC will be more digestible. I guess we will need the time to find a better way for the RRTMGP types and for setting up the ufs-weather-model regression tests?

@@ -23,6 +23,11 @@ CPPDEFS += -DNEW_TAUCTMAX -DSMALL_PE -DNEMS_GSM -DINTERNAL_FILE_NML

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember this now ... we need to find a better way to do this. In particular because the make build is phased out as we speak in favor of a cmake build, hence the proposed changes would be meaningless.

Copy link
Collaborator Author

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a switch to only allocate the GP fields when GP is used.

@climbfuji
Copy link
Collaborator

I added a switch to only allocate the GP fields when GP is used.

Thanks! Looking at this particular commit, I realized that there is no reset-to-zero of these arrays in GFS_typedefs.F90's interstitial_rad_reset call. Is this intended?

@climbfuji
Copy link
Collaborator

Great! Now we are down to the typedefs issue. Let me look at this beginning of next week (huge note on my desk).

@dustinswales
Copy link
Collaborator Author

@climbfuji

@climbfuji
Copy link
Collaborator

This PR has been merged as part of #32, closing it.

@climbfuji climbfuji closed this Mar 26, 2020
SamuelTrahanNOAA added a commit to SamuelTrahanNOAA/fv3atm that referenced this pull request Sep 24, 2020
PBL tendencies were missing in two schemes; now fixed. Squashed commit of the following:

* fix bugs found in pbl and ozone 3d diagnostic tendencies
* remove debugging prints
* implied shape arrays for five variables
* more block labels in CCPP
* make the logging less wordy
* point to hash 238c84c of ccpp/physics
@dustinswales dustinswales deleted the rrtmgp-dev branch October 1, 2020 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.