[develop] Add checks for correct SPP settings for the selected CCPP suite, resolve four more issues#1259
Conversation
mkavulich
requested review from
BenjaminBlake-NOAA,
BruceKropp-Raytheon,
EdwardSnyder-NOAA,
JeffBeck-NOAA,
MichaelLueken,
RatkoVasic-NOAA,
WeirAE,
chan-hoo,
christinaholtNOAA,
christopherwharrop-noaa,
danielabdi-noaa,
dmwright526,
gsketefian,
gspetro-NOAA,
jkbk2004,
mark-a-potts,
natalie-perlin,
panll,
rickgrubin-noaa,
willmayfield and
ywangwof
as code owners
mkavulich
changed the title
mkavulich
changed the title
|
Following the merging of PR #1204, there are two conflicts in |
benkozi
left a comment
benkozi
left a comment
There was a problem hiding this comment.
I made a few minor suggestions but otherwise looks good to me!
[feature/spp_settings_check] Update Ben's GitHub username in CODEOWNERS file
MichaelLueken
added
the
run_we2e_coverage_tests
…viously this check would error out
|
@benkozi I think I addressed your comments, let me know what you think. |
@mkavulich - Looks good to me. Thanks for making the changes. |
|
The UFS_Fire WE2E tests have successfully passed: The AQM WE2E test has successfully passed: |
|
The The Jenkins tests have passed for Azure, Derecho, Gaea C5, Gaea C6, Google, Hera GNU, Hercules, and AWS PW. Two tests are still sitting in queue on Orion. I will merge this PR once those two tests have completed. |
|
Following the updates to I'll check and see if this issue is appearing on other platforms as well. |
|
@MichaelLueken I am assuming this is not related to the updates I made today, but the original changes. I moved some logic that should only be necessary when running a run_fcst task so that it's only executed in that case, but now some other logic in generate_workflow (specifically the logic for generating the run_fcst namelist) has some unset variables. This logic should also be omitted when run_fcst task is not executed. I'm running a test now but I think the fix is simple. |
|
Thanks for the quick fix, @mkavulich! After pulling your modification, I was able to generate all experiments on Orion: |
|
I think I realize now what is happening - the Jenkins tests automatically merge the latest develop into a branch before building and running the coverage WE2E tests. The Jenkins tests were able to run without issue for all platforms this morning because changes that have been merged to develop since Christina's PR #1204 allowed the experiments to generate properly. While in a standalone state, as noted above, your branch is now running smoothly, merging the latest changes in develop causes experiments to fail to generate: I was testing the |
|
I kicked off a quick test (deactivate_tasks) on Gaea C6 via Jenkins this morning and the test failed to generate the experiment: It looks like the fix you applied to get experiment generation to work in your branch is causing issues when the HEAD of develop is merged to your branch in the Jenkins testing process. Please merge the latest HEAD of develop into your branch at your earliest convenience and try and correct this final issue. Thank you very much for your time! |
|
@MichaelLueken turns out this is another issue with my original changes: I did not realize that in cases where |
|
Thanks, @mkavulich, for finding and addressing this last issue! I have kicked off the Jenkins test for Gaea C6 and am rerunning the tests on Orion as well. Once these tests pass, I will move forward with merging this PR. |
|
The manual run of the WE2E coverage tests on Orion have successfully passed: |
|
The Gaea C6 Jenkins rerun has successfully passed: Tests were also run on Azure (the coverage suite that contains the The rerun of the |
|
All WE2E coverage tests are now passing for this PR. The previous issues with the Moving forward with merging this PR now. |
3 participants
DESCRIPTION OF CHANGES:
In discussions with @gspetro and @JeffBeck-NOAA about documentation for SPP settings, we determined that the current documentation for SPP-related settings is insufficient, and that since the SPP capabilities for specific variables are tied to specific CCPP schemes, those checks should be added to
setup.pyand user settings adjusted accordingly.This PR adds a check in setup.py that ensures the appropriate physics schemes are being used for the selected SPP settings, and if not, removes the SPP settings for that scheme. If after this pruning there are no SPP settings left, an exception is raised. Note that many of the changes in setup.py are just re-arranging of existing code; the removed logic from lines 1029 to 1141 are moved downwards so that all of these CCPP checks are done only when a run_fcst task is specified in the workflow.
While making these changes to setup.py, I also added this file to the list of pytest-ed files. This required updating pylint to a newer version, since 2.17 results in an error (and this older version is no longer supported anyway).
Additionally, this PR resolves four more issues:
smoke_dust_grid_RRFS_CONUS_25km_suite_RRFS_sasis addedparm/field_table.RRFS_sas--expt_basedirargument torun_we2e_tests.pyno longer works correctly #1275, fixing the order of operations error in setup.py after [develop] Integrate uwtools into the config layer #1204 was mergedget_from_HPSS_ics_GDAS_lbcs_GDAS_fmt_netcdf_2022040400_ensembleto test SPP_LSM, also change grid from CONUS 3km to CONUS 13km to make it much cheaper (we already have plenty CONUS 3km tests)Type of change
TESTS CONDUCTED:
Ran a number of tests on Hera (Intel). Ran test
grid_SUBCONUS_Ind_3km_ics_HRRR_lbcs_HRRR_suite_HRRRand modified to use every supported suite, observing that the appropriate SPP variables were deactivated ininput.nmlfor each suite that did not contain all needed schemes (FV3_GFS_v16andFV3_WoFS_v0):FV3_GFS_v16
FV3_WoFS_v0
Also ran smoke (old and new) and fundamental WE2E tests; all passed.
DEPENDENCIES:
None
DOCUMENTATION:
Documentation is updated accordingly for latest SPP options.
ISSUE:
Resolves #1228, #1258, #1275, #1276
CHECKLIST