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

iovr=4 cleanup in RRTMG #780

Merged
merged 6 commits into from
Dec 28, 2021
Merged

iovr=4 cleanup in RRTMG #780

merged 6 commits into from
Dec 28, 2021

Conversation

mzhangw
Copy link
Collaborator

@mzhangw mzhangw commented Nov 16, 2021

It is a DTC effort to address issue #1 in #748:
1. The EXP cloud overlap option (iovr=4) is currently only allowed to go down a separate path in the code that is labeled in comments as "HWRF" with the initials "mz" (in radlw_main.F90 and radsw_main.F90). This path follows an older coding structure that derives the correlation parameter, alpha, where it is used and that only allows a constant decorrelation length. This appears to me to be close to what was originally coded for HWRF several years ago and went operational in HWRF in 2018. In other words, the existing implementation of iovr=4 is obsolete. HWRF is currently operational with ER (iovr=5), using coding prepared for WRF and not using ccpp-physics.

The associated PRs:
ufs-community/ufs-weather-model#963
NOAA-EMC/fv3atm#445

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 see that you are removing the icloud=3 and iovr=4 paths completely. Are there any additional changes needed so that iovr option 4 follows the same path as the other iovr options?

@mzhangw
Copy link
Collaborator Author

mzhangw commented Nov 16, 2021 via email

@climbfuji
Copy link
Collaborator

@mjiacono I just invited you to "collaborate" on ccpp-physics so that I can add you as reviewer to PRs. When you get a chance, can you please have a look at this PR and check if it does what you expect it to do to address issue 1 in #748? Thanks!

@mjiacono
Copy link
Collaborator

@climbfuji I took a quick look at the changes in PR #780, and I spotted a couple of things that I recommend changing.

  1. First, in radlw_main.F90 line numbers 912 and 1042 are identified for removal, but these lines must be retained to convert alpha from a 2-D array to the 1-D array alph for later use in the code. In radsw_main.F90 the comparable line numbers 1043 and 1156 must also be retained.
  2. Also, please retain the entire subroutine cldprmc in radlw_main.F90 for now, since it may be needed to address item 3 in issue The need for testing various combinations of radiation levels and physics levels #784.

@mzhangw
Copy link
Collaborator Author

mzhangw commented Nov 18, 2021 via email

@mzhangw mzhangw marked this pull request as draft December 10, 2021 16:35
…rt_iovr4

* 'main' of https://github.com/NCAR/ccpp-physics:
  Add -O1 for RRTMGP file physics/rte-rrtmgp/rrtmgp/kernels/mo_gas_optics_kernels.F90 back in
  Remove Laurie from CODEOWNERS
  permit few max ice number conc; scale back cloud fraction a little; reduce assumed snow as ice in IWP for radiation
  Fixed error in rte-rrtmgp submodule hash.
  Updated rte-rrtmgp submodule. Performance improvements.
  Update physics/GFS_debug.F90 with additional changes from recent commits
  Update physics/GFS_debug.F90 with changes to surface emissivity variables
  fix the vfrac over glacier
  Fixed typo.
  fix snow depth issue in Noah MP
  limit pahi to Noah MP only
  fix the typo
  address some comments from reviewers
  address some comments from reviewer
  Update sfc_noahmp_drv.F90
  Update GFS_surface_generic.F90
  Update module_sf_noahmp_glacier.f90
  Update sfc_noahmp_drv.F90
  Update sfc_diag_post.F90
  Update module_sf_noahmp_glacier.f90
  Update GFS_surface_loop_control.F90
  Update GFS_surface_generic.F90
  update rte-rrtmgp repository
  Update GFS_surface_generic.meta
  Update GFS_surface_generic.F90
  Update GFS_surface_loop_control.meta
  Update GFS_surface_loop_control.meta
  minor rearrangement of cloud fraction with very low mixing ratios
  disallow rain in Thompson to be used in radiation scheme; also set fractional cloudy LWP and IWP inside progcld6
  per review by climbfuji, make suggested changes
  Add 'in Pa' to three long names to clarify the variables
  re-use icloud=3 option for progcld_thompson
  tiny adjustment of indent/format consistency
  per discussions with Ruiyu, increasing min snow and graupel size seems advantageous
  fix a few comment lines per review of pull request 781
  silly stray comment/exclamation point
  Change units of relative humidity variables in CCPP metadata from 1 or none to frac
  Revert unnecessary whitespace changes in physics/module_mp_thompson.F90
  Use correct vertical dimension for several cloud arrays in physics/GFS_rrtmg_post.meta and physics/GFS_rrtmg_pre.meta
  minor adjustments to reduce coverage of partly cloudy conditions
  Move total albedo to right place in physics/GFS_rrtmg_post.meta, make intent(inout)
  Avoid dividing by zero in physics/GFS_rrtmg_post.F90
  Fix compilation error with gfortran in physics/radiation_clouds.f
  remove trailing whitespace
  Add @mkavulich to CODEOWNERS
  update the namelist variables to control semi-lagrangian sedimentation
  bug fix lwp_ex and iwp_ex inside progcld6; factor of 1000
  Some housekeeping
  make icloud=3 call cldfra3 compatible with updated subroutine
  Updated rte-rrtmgp submodule. Small change to accommodate in.meta files in ccpp-physics.
  correct a dumb mistake
  added a new cloud fraction scheme (actually upgraded cal_cldfra3) designed with Thompson-MP for GFSv17-prototype8
  adding a debug option for reading a new month data
  put a few lines of code in read_netfaer
  Modified 2 ugwp metadata files to fix ufs-community/ufs-weather-model 'fhzero reproducibility' issue NCAR#908
  Fix issue with surface-albedo in RRTMGP.
  Fix aerosol diagnostics.
  Fix omission from conflict.
  Expanded format fields for percentages to allow 100% to be output.
  Meta data fix.
  Bug fix for decomposition tests.
  add a output information for reading a new month merra2
  handle a case with n2=13
  delete some debug stuff
  Replace units 'various' with 'mixed', update several invalid units of non-physical quantities, fix units of humidity diagnostic variables
  decreases memory usage by 6 times for MERRA2
  to address reviews
  add semi-Lagrangian sedimentation of graupel into Thompson MP
  Some cleanup for P8.
  Update rte-rrtmgp
  remove some false values over the glacial ice
  correct the inout issue of smc in sflx.f
  pass the right zvfun from noahmp to PBL
  Adding GFS surface scheme to Noah MP as one of opt_sfc options
  remove the iteration loop for Noah MP and implement a new coupling interface between atmos. and Noah MP
  modification to add extra land fields to the output
  modification to add extra land fields to the output
  add Moorthis gcycle fix for tiled fixed file
@mzhangw mzhangw marked this pull request as ready for review December 10, 2021 19:09
@mzhangw
Copy link
Collaborator Author

mzhangw commented Dec 10, 2021

@mjiacono I made changes as you requested.

@mjiacono
Copy link
Collaborator

@mzhangw The changes you made in response to my requests look OK to me.

…rt_iovr4

* 'main' of https://github.com/NCAR/ccpp-physics:
  Remove a non-existing file from list of SCHEMES_OPENMP_OFF, compile mo_gas_optic_kernels.F90 with -O1 in PROD/REPRO mode
  Update DDTs in physics/GFS_debug.F90
  Address reviewer comments
  Remove variables that no longer exist from physics/GFS_debug.F90
  Bug fix in the computation of soil moisture conductivity. The bug was causing cold/moist bias.
  Update minimum cmake version to remove legacy code, reduce chatter
  Major cleanup of cmake build config
  Remove additional newline in CODEOWNERS
  Fix wrong dimensions in physics/drag_suite.meta
  Update GFS_debug.F90, add GFS_checktracers debugging routine
  Adjust format statement in physics/mp_thompson.F90
  Fixed sign error in SSGWD component of drag_suite.F90
  1. Pass in flag_iter to MYNNSFC_wrapper for consistency with iterations in LSM, ice and ocean. ! when iter = 1, flag_iter = .true. for all grids ! when iter = 2, flag_iter = .true. when wind < 2 for both land and ocean (when nstf_name1 > 0) 2.Added flag_iter to all computations in MYNN surface layer scheme to avoid inconsistencies with iterations in LSM, ice andocean models. 3.Removed averaging: 0.5*(tsurf + tskin) and used tskin instead, because tsurf could be not defined. 4.Added checks on QSFC for non-defined values in LSM, ice and ocean models. If there are not defined values, compute QSFC as at itimestep=1.
  Move PSFC computation to the beginning before it is used.
  Added computations of local TSK_wat, TSK_ice and TSK_lnd at ITIMESTEP=1 before they get used.
  Fixed the bug in horizontal dimension for xlon_d.
  Restored modified by accident print statement.
  Fixed bugs in MYNN surface layer scheme in land/ice/water parts of the code to avoid using in computations "huge" numbers of qsfc_lnd, qsfc_ice and qsfc_wat. Also, fixed bugs in MYNN surface layer scheme for initializattion and consistent use of surface QV from RUC LSM over land and ice.
  Pass into the RUC driver lat/lons for debugging.
  Bug fix in physics/GFS_rrtmg_pre.meta, use correct vertical dimension
  Fix bugs in sfc_drv_ruc.F90 introduced by the merge process
  bug fix: must initialize dth prior to moisture_check routine
  Updates to MYNN-EDMF
  Turn on GF aerosol-awareness Tune clwdet (cloud-water detrainment) Make evfact (evaporation factor) and radiation tuning factors scale-aware
  Update to physics/cu_gf_driver.meta following CCPP standard names update
  Fix intent of variable qci_conv in physics/GFS_rrtmg_pre.meta
  Bug fixes for uninitialized variables in physics/cu_gf_deep.F90 and physics/cu_gf_driver.F90
  Update physics/GFS_debug.F90 with latest changes to GFS_interstitial DDT
  Update standard names mid_conv_activity_counter and gf_mid_memory_counter
  Clean up code Fix definition of xland1
  Clean up of code syntax
  Slight cleanup to kklev
  1. Changes for fracional grid (frac_grid=.true.) 2. Replace logical variable for lakes from 'lake' to 'use_flake' 3. Added spp code from WRF version of RUC LSM in case it would be needed for RRFS. So far it is not hooked up to SPP weights.
  Fix for uninitialized kklev for mid clouds
  Fix cactiv_m error
  Update to the entrainment formula - allows removal of contraint on accelerating plumes.
  MYNN-EDMF updates and bug fixes
  Fix component issue with xland
  Updates to prepare for merger with latest version of master
  Code clean up in GFS_rrtmg_pre.F90
  Code clean up for GFS_rrtmg_pre.F90
  Remove an extra space
  - Turn off aerosols in GF - Improve GF-radiation coupling when GF substituted for SAS in the GFS physics suite
  Fix b4b differences for GSD v0 (RUC LSM, tiice)
  Revert change to CODEOWNERS
  Add GF variables to physics/GFS_debug.F90
  Remove trailing whitespaces in physics/cu_gf_driver.F90; bug fixes in physics/cu_gf_driver.F90 to achieve b4b identical results in restart runs
  Correct wrong standard names in physics/cu_gf_driver_post.meta and physics/cu_gf_driver_pre.meta
  Taking out OPTIONAL declaration, as it was before.
  Modified aod_gf and cactiv_m so that work appriopriately with restart Minor code clean up
  Removing some commented out code.
  Another bugfix in physics/cu_gf_deep.F90 to prevent qc(i,k) being zero
  Bug fix for clw_allh
  Remove unneeded variable
  fix spelling
  more doxygen complaints (I think), but they dont make sense. Testing changes by trial and error...
  more doxygen complaints (I think), but they dont make sense. Testing changes by trial and error...
  Bug fixes - complaining about comments again...
  Bug fixes - removing a 3rd doxygen bug
  Bug fixes - removing a 2nd doxygen bug
  Bug fixes/clean up: (1) removing doxygen bug, (2) removing j indices from the driver.
  Bug fix
  Update of GF aerosol treatment and tunings
  Final updates for the MYNN-EDMF for HFIP
  Updating MYNN-EDMF
@mzhangw mzhangw changed the title ivor=4 cleanup in RRTMG iovr=4 cleanup in RRTMG Dec 16, 2021
@grantfirl
Copy link
Collaborator

Other than my comment about cleaning up now-unused modules, these changes look OK to me. Since they address an issue raised by @mjiacono and he has approved, then it is good enough for me. @mzhangw Do you know if the removed code had been used in any of the UFS regression tests? I need to know whether these changes will require new baselines or not.

@climbfuji
Copy link
Collaborator

Other than my comment about cleaning up now-unused modules, these changes look OK to me. Since they address an issue raised by @mjiacono and he has approved, then it is good enough for me. @mzhangw Do you know if the removed code had been used in any of the UFS regression tests? I need to know whether these changes will require new baselines or not.

There are four regression tests in rt.conf that exercise this code. They will be removed in one of the next ufs-weather-model PRs if I remember correctly (look for fv3_HAFS_*). Once those tests have been removed, this PR should have no impact on the regression tests (unless the compiler chooses to optimize sth differently).

…rt_iovr4

* 'main' of https://github.com/NCAR/ccpp-physics:
  change snow thermal conductivity to verseghy
  change eta0 to slow snow compaction
  remove comments and add back the missing if (.not. lmfshal) then block
  fix the tsurf issue in Noah MP driver
  update GFS_phys_time_vary.scm.F90/meta to work with latest ccpp/physics and SCM
  Bug fixes for getting convectice cloud water into Thompson MP without crashes
  Modification of momentum roughness length over ice in the surface layer scheme
  Modification of momentum roughness length over ice in the surface layer scheme
  Add primary codeowners to all files
  Add more dependency files, and three missing code owners
  aligned with if block
  abort model when external surface emissivity file not existing for iemslw=1
  Fixed bug in cloud diagnostics, removed over counting.
  Add codeowners for files that have known Points of Contact
  cloud cover change associated with xu_randall
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.

The changes look good. I will wait for the results of the regression testing before I approve. The changes in this PR should not change the results of any of the existing regression tests, since the only tests that were using the iovr==4 option are removed in the associated ufs-weather-model PR.

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.

All regression tests passed against the existing baseline (after removing the old fv3_*_HAFS_* tests, which used iovr==4).

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.

5 participants