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

[develop] Reorganize verification tasks (rename tasks and files; split tasks into two) #618

Merged

Conversation

gsketefian
Copy link
Collaborator

@gsketefian gsketefian commented Feb 17, 2023

DESCRIPTION OF CHANGES:

  1. Rename verification tasks and corresponding j-jobs, ex-scripts, and log files as described in Issue Rename vx tasks so they follow METplus-tool based convention #619.
  2. Separate combined METplus vx tasks for surface and upper air into separate tasks. This is so that there is only one call to a METplus tool per ROCOTO task and helps simplify the workflow and debugging.
  3. Ensure that each vx task has a separate section in config_defaults.yaml with its own resource variables (WTIME_..., PPN_..., etc).

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

TESTS CONDUCTED:

  • hera.intel
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss2.intel
  • NOAA Cloud (indicate which platform)
  • Jenkins
  • fundamental test suite
  • comprehensive tests (specify which if a subset was used)

I have run the following fundamental WE2E tests on Hera with the Intel compiler. All were successful. Also, all task names and log file names are as described in Issue #619.

MET_verification_only_preproc_vx
MET_verification_only_preproc_makeics_vx
MET_verification
MET_verification_only_vx
community_ensemble_2mems
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional_plot
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR
grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2
grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional_plot
MET_ensemble_verification
community_ensemble_2mems_stoch
pregen_grid_orog_sfc_climo

DOCUMENTATION:

This PR requires changes to the documentation. However, I have not yet made changes to the documentation. There will likely be further changes required to the documentation due to several upcoming vx PRs. I prefer to make all the documentation changes in one go after all the vx PRs have been merged.

ISSUE:

This resolves Issue #619.

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes do not require updates to the documentation (explain).
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

  • Work In Progress
  • bug
  • enhancement
  • documentation
  • release
  • high priority
  • run_ci
  • run_we2e_fundamental_tests
  • run_we2e_comprehensive_tests
  • Needs Cheyenne test
  • Needs Jet test
  • Needs Hera test
  • Needs Orion test
  • help wanted

CONTRIBUTORS (optional):

@michelleharrold @JeffBeck-NOAA @willmayfield @mkavulich

@MichaelLueken MichaelLueken linked an issue Feb 17, 2023 that may be closed by this pull request
@gsketefian gsketefian changed the title [develop] Rename vx tasks [develop] Rename verification tasks, j-jobs, scripts, and log files as described in Issue #619 Feb 17, 2023
@gsketefian gsketefian marked this pull request as ready for review February 17, 2023 23:15
…ks; Add ensemble member name (if running ensemble vx) to the names of the vx tasks in the ROCOTO xml.
…t convention; fix bugs in the two ex-scripts exregional_run_met_pointstat_ens[mean|prob].sh that call METplus twice.
@gsketefian gsketefian changed the title [develop] Rename verification tasks, j-jobs, scripts, and log files as described in Issue #619 [develop] Reorganize verification tasks (rename tasks and files; split tasks into two) Feb 27, 2023
@gsketefian
Copy link
Collaborator Author

@MichaelLueken @gspetro I just created Issue #630 for updating the vx documentation.

@MichaelLueken
Copy link
Collaborator

Thank you very much, @gsketefian, for creating issue #630!

Copy link
Collaborator

@danielabdi-noaa danielabdi-noaa left a comment

Choose a reason for hiding this comment

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

@gsketefian Thanks for making the changes, approving now.

Copy link
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@gsketefian Thank you very much for addressing @danielabdi-noaa's concerns! Following the renaming of the ex- scripts to be lower case, per NCO guidelines, these changes look good to me. I will go ahead and approve these changes now.

@MichaelLueken MichaelLueken added ci-hera-intel-WE Kicks off automated workflow test on hera with intel run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests labels Feb 27, 2023
@venitahagerty venitahagerty removed the ci-hera-intel-WE Kicks off automated workflow test on hera with intel label Feb 27, 2023
@gsketefian
Copy link
Collaborator Author

@danielabdi-noaa @MichaelLueken Thanks for the approvals!

I am retesting now on Hera to make sure all the fundamental tests pass. Also, I'd like to wait until tomorrow (morning, let's say) to get some feedback on Issue #619 (so far I haven't gotten any) before proceeding further. Assuming my tests pass and there is no or minor feedback on #619 by tomorrow, could @MichaelLueken launch the automated tests then? Thanks.

@MichaelLueken
Copy link
Collaborator

@danielabdi-noaa @MichaelLueken Thanks for the approvals!

I am retesting now on Hera to make sure all the fundamental tests pass. Also, I'd like to wait until tomorrow (morning, let's say) to get some feedback on Issue #619 (so far I haven't gotten any) before proceeding further. Assuming my tests pass and there is no or minor feedback on #619 by tomorrow, could @MichaelLueken launch the automated tests then? Thanks.

@gsketefian After I gave my approval, I went ahead and submitted the Jenkins tests. If any additional changes are pushed to your feature/rename_vx_tasks branch, I'll try and rerun the automated tests when I get back from jury duty tomorrow.

@gsketefian
Copy link
Collaborator Author

@MichaelLueken Ok, thanks for letting me know. One question -- does the automated testing system first merge develop into the PR and then run the tests? I just realized I haven't yet merged in develop.

@MichaelLueken
Copy link
Collaborator

@gsketefian Yes, the first thing that the automated Jenkins tests do is merge the latest develop branch into the branch associated with the PR. So, no worries with that. If discussion from issue #619 leads to additional modifications, then the tests will need to be rerun, but you don't need to worry about the PR being up-to-date with develop until just before merging.

@venitahagerty
Copy link
Collaborator

venitahagerty commented Feb 27, 2023

Machine: hera
Compiler: intel
Job: WE
Repo location: /scratch1/BMC/zrtrr/rrfs_ci/autoci/pr/1245532184/20230227205016/ufs-srweather-app
Build was Successful
Rocoto jobs started
Long term tracking will be done on 10 experiments
If test failed, please make changes and add the following label back:
ci-hera-intel-WE
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional_plot
Experiment Failed on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
2023-02-27 22:16:06 +0000 :: hfe10 :: Task make_ics, jobid=42409022, in state DEAD (FAILED), ran for 40.0 seconds, exit status=256, try=2 (of 2)
Experiment Succeeded on hera: community_ensemble_2mems_stoch
Experiment Failed on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
2023-02-27 22:12:09 +0000 :: hfe07 :: Task make_ics, jobid=42408698, in state DEAD (FAILED), ran for 39.0 seconds, exit status=256, try=2 (of 2)
Experiment Succeeded on hera: pregen_grid_orog_sfc_climo
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
Experiment Succeeded on hera: MET_ensemble_verification
All experiments completed

@gsketefian
Copy link
Collaborator Author

@MichaelLueken Ok, thanks!

@gsketefian
Copy link
Collaborator Author

@MichaelLueken All the fundamental tests I ran locally on Hera passed.

…leStat for point-obs, ensure that METplus is called only once and for the correct variable (instead of for both SFC and UPA).
@gsketefian
Copy link
Collaborator Author

gsketefian commented Feb 28, 2023

@MichaelLueken FYI unfortunately I found bugs in a couple of the vx ex-scripts. They only affect verification. I fixed them and reran the three vx tests (MET_verification, MET_verification_only_vx, and MET_ensemble_verification). They were successful, so I pushed the changes to the PR.

@gsketefian
Copy link
Collaborator Author

@MichaelLueken A couple of people are going to take a look at Issue #619 by COB today, so could you hold off on rerunning the automated tests until then? I can let you know when the PR is ready for the re-testing. Thanks!

@MichaelLueken
Copy link
Collaborator

@gsketefian I'm on PTO today, so if it is okay with you, I will resubmit the automated tests first thing tomorrow morning. They should run fast enough so that this work can be merged early in the day. Thanks!

@gsketefian
Copy link
Collaborator Author

@gsketefian I'm on PTO today, so if it is okay with you, I will resubmit the automated tests first thing tomorrow morning. They should run fast enough so that this work can be merged early in the day. Thanks!

@MichaelLueken Yes, that sounds good.

@MichaelLueken
Copy link
Collaborator

@gsketefian I have just resubmitted the automated Jenkins tests. Are there any other changes that you will be making, or will these changes be ready to be merged once the tests successfully pass?

@gsketefian
Copy link
Collaborator Author

@MichaelLueken No more changes, so please go ahead and merge it if the tests pass. Thanks!

@MichaelLueken
Copy link
Collaborator

@gsketefian Great! I will move forward with these changes once they pass the automated tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename vx tasks so they follow METplus-tool based convention
4 participants