[develop] fixes to enable stochastic physics #870
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
willmayfield
requested review from
mkavulich,
gsketefian,
JeffBeck-NOAA,
RatkoVasic-NOAA,
BenjaminBlake-NOAA,
ywangwof,
chan-hoo,
panll,
christinaholtNOAA,
christopherwharrop-noaa,
danielabdi-noaa,
mark-a-potts,
jkbk2004,
dmwright526,
gspetro-NOAA,
natalie-perlin,
EdwardSnyder-NOAA and
MichaelLueken
as code owners
gsketefian
approved these changes
Jul 28, 2023
MichaelLueken
approved these changes
Jul 28, 2023
There was a problem hiding this comment.
Thank you very much for addressing the stochastic physics issues! I was able to run both a deterministic and ensemble test with stochastic physics activated and both behaved as they should.
As part of this PR, would you mind changing FV3_NML_FP on line 103 to fv3_nml_ensmem_fp? Seeing that the specified seeds were being updated in the parent level input.nml was confusing and isn't correct.
I will go ahead and approve this PR now and kick off the automated Jenkins tests (the above modification won't affect the automated testing).
MichaelLueken
added
the
run_we2e_coverage_tests
|
@MichaelLueken and @gsketefian thank you for your reviews! I fixed the comment that Michael requested. And I want to thank @JeffBeck-NOAA for his help with this PR! |
|
The Cheyenne Intel WE2E tests were manually ran on Hera and all tests successfully passed: The Cheyenne GNU WE2E tests were manually ran on Hera and all tests successfully passed: Automated Jenkins tests passed for Hera GNU: Automated Jenkins tests passed for Hera Intel: Automated Jenkins tests passed for Jet: Automated Jenkins tests passed for Orion: Moving forward with merging this work now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DESCRIPTION OF CHANGES:
These changes allow ensemble mode to use seeds generated to be unique to the forecast cycle and ensemble member, and they also allow stochastic physics to be turned on in deterministic mode.
Type of change
TESTS CONDUCTED:
A configuration was created which turned on all stochastic physics schemes (SPP, SPPT, SKEB, SHUB) except land surface perturbations. This test was successful and shown to use differing stochastic physics seeds for each ensemble member in the output forecasts. Other tests were run by @JeffBeck-NOAA showing that a deterministic forecast may now be run with stochastic physics.
Note: Land surface perturbations were left off because the current default settings for it in config_defaults.yaml no longer seem to work.
DOCUMENTATION:
It was previously discussed with @MichaelLueken and @gspetro-NOAA that stochastic physics should be noted to no longer work in the documentation. Hopefully this PR does away with that need.
ISSUE:
This PR fixes issue #818.
CONTRIBUTORS (optional):
@JeffBeck-NOAA