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

Add support for Stochastically Perturbed Parameterizations (SPP) in FV3 #820

Merged
merged 40 commits into from
Feb 23, 2022

Conversation

JeffBeck-NOAA
Copy link
Contributor

@JeffBeck-NOAA JeffBeck-NOAA commented Dec 27, 2021

Description

This PR adds the necessary code in ccpp-physics to run the FV3 with Stochastically Perturbed Parameterizations (SPP). The associated code uses the stochastic_physics pattern generator (same as for SKEB, SHUM, and SPPT) to create a random pattern based on scales, magnitudes, etc., from the FV3 input.nml namelist and perturbs RAP/HRRR-based physics scheme parameters in selected parameterizations. This PR to ccpp-physics connects the SPP flag and weights to the specific RAP/HRRR parameterizations.

Issue(s) addressed

This PR address Issue #978 in ufs-weather-model.

Testing

Regression testing for ufs-weather-model was conducted on Hera and Cheyenne. All passed, but not bit-for-bit for RAP and HRRR cases (tests 41 and 42 on cheyenne, 44 and 45 on hera); although unlikely that this B4B discrepancy is related to code in this PR. Debugging revealed several errors in the GSL GWD interfaces, might be the cause of the B4B issue (unallocated, mis-matched array sizes, etc.) that are caught with a DEBUG compile. These cases do not run in debug mode, and there are no debug tests in the RT, so maybe this is a known issue? The unique element for these two cases is the GSL GWD scheme (drag_suite.F90). @DomHeinzeller described an optimization/bug in RRTMGP that caused B4B issues, maybe reducing optimization on drag_suite.F90 would fix the issue?

Dependencies

Depends on the following PRs from fv3atm, ufs-weather-model, and stochastic_physics.

@llpcarson, @jwolff-ncar, @willmayfield, @bluefinweiwei, @michelleharrold, @judithberner, @pjpegion

@JeffBeck-NOAA JeffBeck-NOAA changed the title Add support for Stochastically Perturbed Parameterizations (SPP) in the FV3 Add support for Stochastically Perturbed Parameterizations (SPP) in FV3 Dec 27, 2021
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.

A few small changes here and there.

The main concern I have is that half of this PR deals with passing stochastic perturbations through the various interfaces to the gravity wave drag, but then these variables don't get used at all?

Also, regarding the b4b mismatches and the speculations as to why, please see my comment in the ufs-weather-model PR.

physics/GFS_rrtmg_pre.F90 Outdated Show resolved Hide resolved
physics/drag_suite.F90 Outdated Show resolved Hide resolved
physics/mp_thompson.F90 Outdated Show resolved Hide resolved
physics/mp_thompson.F90 Show resolved Hide resolved
physics/mp_thompson.F90 Show resolved Hide resolved
@climbfuji climbfuji requested a review from mdtoy December 27, 2021 14:51
@climbfuji
Copy link
Collaborator

The last update of this code from the authoritative repos is 18 days ago - definitely behind.

@JeffBeck-NOAA
Copy link
Contributor Author

JeffBeck-NOAA commented Dec 28, 2021

The last update of this code from the authoritative repos is 18 days ago - definitely behind.

@climbfuji, pulled in the latest code.

Copy link
Collaborator

@joeolson42 joeolson42 left a comment

Choose a reason for hiding this comment

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

I only checked the mynn wrapper and the drag suite. My comment for the MYNN wrapper should be addressed because this PR will cause the functionality of SPP to be different between CCPP and WRF. I'm not sure that is whats intended.

physics/module_MYNNPBL_wrapper.F90 Show resolved Hide resolved
physics/drag_suite.F90 Outdated Show resolved Hide resolved
@joeolson42
Copy link
Collaborator

joeolson42 commented Dec 28, 2021 via email

@JeffBeck-NOAA
Copy link
Contributor Author

JeffBeck-NOAA commented Dec 29, 2021

@joeolson42, you're correct, if do_spp is .true., then spp_pbl = 1 (even if 'pbl' isn't listed in spp_var_list and SPP is being used for another parameterization). In that case, those do loops are still activated, but with zero perturbation values. It's not ideal, and I agree, we should base the activation of SPP in MYNN PBL on whether 'pbl' is listed in spp_var_list. I would need to take a look at how Laurie implemented spp_var_list to see if it can be queried within MYNN PBL. We need something like:

if ( do_spp ) .AND. ( spp_var_list contains 'pbl' )
    spp_pbl=1
endif

I realize that the use of do_spp may not be necessary in this case.

The problem is that spp_var_list can be any combination of 'pbl', 'sfc', 'mp', 'gwd', or 'rad', so I'm not sure how to "check" whether 'pbl' is listed within Fortran. Do you have an idea?

@climbfuji
Copy link
Collaborator

climbfuji commented Dec 29, 2021 via email

@climbfuji
Copy link
Collaborator

These PRs need more work before they can be considered for merging. I will add labels "do not merge" and "work in progress".

drag_suite.F90 Outdated
@@ -0,0 +1,1381 @@
!> \File drag_suite.F90
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JeffBeck-NOAA It looks like this file was misplaced. Could you remove it please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @grantfirl! Removed.

[spp_rad]
standard_name = control_for_radiation_spp_perturbations
long_name = control for radiation spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't the right unit for this, but this can be cleaned up in the future since there are a bunch of wrong units for stuff like this, at least according to https://github.com/ESCOMP/CCPPStandardNames/blob/main/StandardNamesRules.rst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'm happy to make those changes in a future PR if it's not urgent right now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not urgent right now and can be merged as-is since we're so far along in the process.

[spp_gwd]
standard_name = control_for_gravity_wave_drag_spp_perturbations
long_name = control for gravity wave drag spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment RE: units.

[spp_pbl]
standard_name = control_for_pbl_spp_perturbations
long_name = control for pbl spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment RE: units

[spp_sfc]
standard_name = control_for_surface_layer_spp_perturbations
long_name = control for surface layer spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment RE: units

[spp_mp]
standard_name = control_for_microphysics_spp_perturbations
long_name = control for microphysics spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment RE: units

[spp_gwd]
standard_name = control_for_gravity_wave_drag_spp_perturbations
long_name = control for gravity wave drag spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment RE: units

[spp_gwd]
standard_name = control_for_gravity_wave_drag_spp_perturbations
long_name = control for gravity wave drag spp perturbations
units = count
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment RE: units

@grantfirl
Copy link
Collaborator

@JeffBeck-NOAA I'll approve this so that it can be merged once the extraneous drag_suite.F90 file is removed.

Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

This looks good to me. @JeffBeck-NOAA did a nice job addressing previous comments/suggestions, IMO.

@grantfirl
Copy link
Collaborator

@climbfuji Since you requested changes, could you take a look at this again to see if your request was addressed? It looks like merging is blocked until you review again.

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 only looked at the two places where I had comments before, and those have been addressed or seem to not be an issue. I did not revisit all the changes that were made between my initial review and today.

physics/mp_thompson.F90 Show resolved Hide resolved
@JeffBeck-NOAA
Copy link
Contributor Author

I only looked at the two places where I had comments before, and those have been addressed or seem to not be an issue. I did not revisit all the changes that were made between my initial review and today.

Thanks, @climbfuji! I appreciate your reviews. I'll keep an eye on things regarding the use of SPP in Thompson MP and any potential failures.

@grantfirl grantfirl merged commit ff6395c into NCAR:main Feb 23, 2022
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