-
Notifications
You must be signed in to change notification settings - Fork 120
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
[develop] Adding support for FV3_RAP CCPP suite #811
[develop] Adding support for FV3_RAP CCPP suite #811
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkavulich The changes look good to me!
I ran the coverage tests on Orion and all tests successfully passed. Coverage tests have also been submitted on Cheyenne GNU (wanted to make sure that at least one of the new RAP tests was run as part of my testing). All tests, with the exception of nco_grid_RRFS_CONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15_thompson_mynn_lam3km
, passed on Cheyenne GNU as well. The grid_RRFS_CONUS_13km_ics_FV3GFS_lbcs_FV3GFS_suite_RAP
test was also ran on Cheyenne GNU and it successfully passed.
Approving this PR.
Thanks for the approval @MichaelLueken. Once #799 is merged I will rebase this branch on the latest changes from develop and resolve the resulting conflicts. |
@mkavulich PR #799 hasn't been merged yet. I'm now investigating a new failure of the
None of the change in PR #799 would affect this test, but that PR is the only one where this test is currently failing now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran the new RAP suite WE2E tests on Jet and they all passed. Code checks out as well. Approving.
@mkavulich PR #799 has been successfully merged. Please rebase this branch on the latest changes from develop and resolve the resulting conflicts. Once complete, I will run the automated Jenkins tests and then move forward with merging this work. Thanks! |
4c7122a
to
4172938
Compare
@MichaelLueken Thanks for the heads up, I rebased the commits and ran the Hera Intel coverage suite and all tests passed. This PR is ready to be re-tested. |
@mkavulich The coverage WE2E tests were manually ran on Orion earlier this morning and all tests successfully passed. The Jenkins tests are failing in the
in the log files. This wasn't an issue from testing PR #799 in Jenkins (nor any of the other changes that have been tested since it was merged). Running your changes manually, this test is successfully passing for me. The correct DT_ATMOS value is being used (150), so this shouldn't be an issue. Please see:
for the results of the failing test. Additionally, the
If you could take a look at these two tests, I would be greatly appreciative. Thanks! |
@MichaelLueken I have no insight into those errors. I re-ran the coverage tests on Jet and all were successful. Perhaps this was a random system hiccup? Can we kick off the automated tests again to see if they succeed? If this sort of thing continues to happen on Jet specifically we may want to investigate if the model is having different behavior on certain partitions. If this is the case we may need to modify the failing test(s) to request a certain amount of memory. This page details the different specs for the various Jet partitions; I wonder if the failing jobs landed on xJet, which has the least memory per node. |
@mkavulich The rerun this morning on Jet has successfully passed. Moving forward with the merging this PR now. |
DESCRIPTION OF CHANGES:
The release committee has decided on including FV3_RAP as a supported CCPP suite in the SRW for the next release. This PR introduces the necessary changes to the workflow, documentation, and tests to add FV3_RAP as a supported suite in the app.
Type of change
TESTS CONDUCTED:
Three tests are updated to use the new FV3_RAP suite:
Ran fundamental test suite on Hera, and comprehensive suite on Jet. All tests (including the above changes) passed except for three:
MET_verification_only_vx
grid_RRFS_AK_3km_ics_FV3GFS_lbcs_FV3GFS_suite_HRRR
GST_release_public_v1
The first failure is due to missing data; this is documented in #808. The latter two are due to the updated weather model hash: this may require further changes to either namelist or config files. See this discussion on PR 799 for more info. For now I have added the "DO_NOT_MERGE" label until these failures can be resolved, but note that this is unrelated to the FV3_RAP suite.
DEPENDENCIES:
DOCUMENTATION:
Updated users guide to include references to new supported suite
ISSUE:
CHECKLIST