Skip to content

Re-work test verification and add pre-test option in rt.sh for developers to use in PR process. #2058

Closed
@BrianCurtis-NOAA

Description

@BrianCurtis-NOAA

Description

There are a lot of pieces to putting a pull request together and the information that code management needs isn't always readily available and can take developers a long time to gather. The logs are overly verbose and don't provide developers a quick way to find what went wrong with their code changes.

Solution

The logs should be more succinct. Having information on all file comparisons kept in the history of the repository seems overkill. Simplify the logs to provide information that all tests are pass/fail and provide a log for failed tests for developers to easily find out what went wrong.

Ideas

  • Creates a synopsis that clarifies:
    • number of tests ran to completion (double checking all tests are run)
      • Listing all failed comparisons.
    • number of compilations ran to completion (double checking all compiles are run).
      • Listing all failed compiles.
  • Write synopsis to RegressionTests_.log
  • fail_test uses number while fail_compile uses compile name, changing fail_test to use test name.

Extras

  • The resulting PR will also need to adjust the PR template.

Activity

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 20, 2023

@BrianCurtis-NOAA
CollaboratorAuthor

As I start working on this, adding tags for @junwang-noaa @DusanJovic-NOAA @DeniseWorthen. Is there anything else you think this should do, or not do, based on what I have in the issue?

DeniseWorthen

DeniseWorthen commented on Dec 20, 2023

@DeniseWorthen
Collaborator

Do you envision that the pre-test log is short (just summarizing and flagging failed tests etc) or would it be like the existing log with a summary attached?

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 20, 2023

@BrianCurtis-NOAA
CollaboratorAuthor

Do you envision that the pre-test log is short (just summarizing and flagging failed tests etc) or would it be like the existing log with a summary attached?

My goal is to limit it to the information we need, and skip the individual file comparisons. If it's not easily fit into the code, it may be larger.

DeniseWorthen

DeniseWorthen commented on Dec 20, 2023

@DeniseWorthen
Collaborator

And is there anyway to have Github understand the pre-test log and apply some sort of "green light" on a PR?

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 20, 2023

@BrianCurtis-NOAA
CollaboratorAuthor

And is there anyway to have Github understand the pre-test log and apply some sort of "green light" on a PR?

I think this would be implemented through a script that the github action uses. We would have to write it, though.

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 21, 2023

@BrianCurtis-NOAA
CollaboratorAuthor

I've added code to the script that creates a symbolic link called run_dir that links to the current rt_###### for your running job and put it into the tests dir. It will update that for each new job.

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 21, 2023

@BrianCurtis-NOAA
CollaboratorAuthor

This should close #1821

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 22, 2023

@BrianCurtis-NOAA
CollaboratorAuthor
briancurtis@derecho4:/glade/work/briancurtis/git/BrianCurtis-NOAA/ufs-weather-model/rt_pretest/tests> cat logs/RegressionTests_pretest.log 
Thu 21 Dec 2023 07:21:43 PM MST
Start Regression test

Testing UFSWM Hash: 4d1c08c60c5026945bd4013384e55cb7f8999087
Testing With Submodule Hashes:
 37cbb7d6840ae7515a9a8f0dfd4d89461b3396d1 AQM (v0.2.0-37-g37cbb7d)
 2aa6bfbb62ebeecd7da964b8074f6c3c41c7d1eb CDEPS-interface/CDEPS (cdeps0.4.17-38-g2aa6bfb)
 50aa2c97882fbc9d4918813a22169fe97b424564 CICE-interface/CICE (CICE6.0.0-444-g50aa2c9)
 e5d08d4233b0c783a4840dcbc3252a170e3c3bb1 CMEPS-interface/CMEPS (cmeps_v0.4.1-2299-ge5d08d4)
 cabd7753ae17f7bfcc6dad56daf10868aa51c3f4 CMakeModules (v1.0.0-28-gcabd775)
 f221fc5ce66cee86160efa4bc4deb9c861959e19 FV3 (remotes/origin/production/HREF.v3beta-287-gf221fc5)
 041422934cae1570f2f0e67239d5d89f11c6e1b7 GOCART (sdr_v2.1.2.6-119-g0414229)
 35789c757766e07f688b4c0c7c5229816f224b09 HYCOM-interface/HYCOM (2.3.00-121-g35789c7)
 a36cb73d6924f6cf56a72b5799bef3d75fe4dd61 MOM6-interface/MOM6 (dev/master/repository_split_2014.10.10-9871-ga36cb73d6)
 569e354ababbde7a7cd68647533769a5c966468d NOAHMP-interface/noahmp (v3.7.1-303-g569e354)
 02693d837f2cd99d20ed08515878c2b5e9525e64 WW3 (6.07.1-343-g02693d83)
 37e740526993fd162d092266ced19c625fbc3d8e stochastic_physics (ufs-v2.0.0-193-g37e7405)
Verifying: Compile s2swa_32bit_intel
Verifying: Test cpld_control_p8_mixedmode_intel
Verifying: Compile s2swa_32bit_pdlib_intel
Verifying: Test cpld_control_gfsv17_intel
Verifying: Test cpld_control_gfsv17_iau_intel
Verifying: Test cpld_restart_gfsv17_intel
Verifying: Test cpld_mpi_gfsv17_intel
Verifying: Compile s2swa_32bit_pdlib_debug_intel
Verifying: Test cpld_debug_gfsv17_intel
Verifying: Compile s2swa_intel
Verifying: Test cpld_control_p8_intel
Verifying: Test cpld_restart_p8_intel
Verifying: Test cpld_control_qr_p8_intel
Verifying: Test cpld_restart_qr_p8_intel
Verifying: Test cpld_decomp_p8_intel
Verifying: Test cpld_mpi_p8_intel
Verifying: Test cpld_control_ciceC_p8_intel
Verifying: Test cpld_control_c192_p8_intel
Verifying: Test cpld_restart_c192_p8_intel
Verifying: Test cpld_s2sa_p8_intel
Verifying: Compile s2sw_intel
Verifying: Test cpld_control_noaero_p8_intel
Verifying: Test cpld_control_nowave_noaero_p8_intel
Verifying: Compile s2swa_debug_intel
Verifying: Test cpld_debug_p8_intel
Verifying: Compile s2sw_debug_intel
Verifying: Test cpld_debug_noaero_p8_intel
Verifying: Compile s2s_aoflux_intel
Verifying: Test cpld_control_noaero_p8_agrid_intel
Verifying: Compile s2s_intel
Verifying: Test cpld_control_c48_intel
Verifying: Compile s2sw_pdlib_intel
Verifying: Test cpld_control_pdlib_p8_intel
Verifying: Test cpld_restart_pdlib_p8_intel
Verifying: Test cpld_mpi_pdlib_p8_intel
Verifying: Compile s2sw_pdlib_debug_intel
Verifying: Test cpld_debug_pdlib_p8_intel
Verifying: Compile atm_dyn32_intel
Verifying: Test control_flake_intel
Verifying: Test control_CubedSphereGrid_intel
Verifying: Test control_CubedSphereGrid_parallel_intel
Verifying: Test control_latlon_intel
Verifying: Test control_wrtGauss_netcdf_parallel_intel
Verifying: Test control_c48_intel
Verifying: Test control_c192_intel
Verifying: Test control_c384_intel
Verifying: Test control_c384gdas_intel
Verifying: Test control_stochy_intel
Verifying: Test control_stochy_restart_intel
Verifying: Test control_lndp_intel
Verifying: Test control_iovr4_intel
Verifying: Test control_iovr5_intel
Verifying: Test control_p8_intel
Verifying: Test control_p8_ugwpv1_intel
Verifying: Test control_restart_p8_intel
Verifying: Test control_noqr_p8_intel
Verifying: Test control_restart_noqr_p8_intel
Verifying: Test control_decomp_p8_intel
Verifying: Test control_p8_lndp_intel
Verifying: Test control_p8_rrtmgp_intel
Verifying: Test control_p8_mynn_intel
Verifying: Test merra2_thompson_intel
Verifying: Test regional_control_intel
Verifying: Test regional_restart_intel
Verifying: Test regional_decomp_intel
Verifying: Test regional_noquilt_intel
Verifying: Test regional_netcdf_parallel_intel
Verifying: Test regional_2dwrtdecomp_intel
Verifying: Test regional_wofs_intel
Verifying: Compile rrfs_intel
Verifying: Test rap_control_intel
Verifying: Test regional_spp_sppt_shum_skeb_intel
Verifying: Test rap_decomp_intel
Verifying: Test rap_restart_intel
Verifying: Test rap_sfcdiff_intel
Verifying: Test rap_sfcdiff_decomp_intel
Verifying: Test rap_sfcdiff_restart_intel
Verifying: Test hrrr_control_intel
Verifying: Test hrrr_control_decomp_intel
Verifying: Test hrrr_control_2threads_intel
Verifying: Test hrrr_control_restart_intel
Verifying: Test rrfs_v1beta_intel
Verifying: Test rrfs_v1nssl_intel
Verifying: Test rrfs_v1nssl_nohailnoccn_intel
Verifying: Compile csawmg_intel
Verifying: Test control_csawmg_intel
Verifying: Test control_csawmgt_intel
Verifying: Test control_ras_intel
Verifying: Compile wam_intel
Verifying: Test control_wam_intel
Verifying: Compile atm_faster_dyn32_intel
Verifying: Test control_p8_faster_intel
Verifying: Test regional_control_faster_intel
Verifying: Compile atm_debug_dyn32_intel
Verifying: Test control_CubedSphereGrid_debug_intel
Verifying: Test control_wrtGauss_netcdf_parallel_debug_intel
Verifying: Test control_stochy_debug_intel
Verifying: Test control_lndp_debug_intel
Verifying: Test control_csawmg_debug_intel
Verifying: Test control_csawmgt_debug_intel
Verifying: Test control_ras_debug_intel
Verifying: Test control_diag_debug_intel
Verifying: Test control_debug_p8_intel
Verifying: Test regional_debug_intel
Verifying: Test rap_control_debug_intel
Verifying: Test hrrr_control_debug_intel
Verifying: Test hrrr_gf_debug_intel
Verifying: Test hrrr_c3_debug_intel
Verifying: Test rap_unified_drag_suite_debug_intel
Verifying: Test rap_diag_debug_intel
Verifying: Test rap_cires_ugwp_debug_intel
Verifying: Test rap_unified_ugwp_debug_intel
Verifying: Test rap_lndp_debug_intel
Verifying: Test rap_progcld_thompson_debug_intel
Verifying: Test rap_noah_debug_intel
Verifying: Test rap_sfcdiff_debug_intel
Verifying: Test rap_noah_sfcdiff_cires_ugwp_debug_intel
Verifying: Test rrfs_v1beta_debug_intel
Verifying: Test rap_clm_lake_debug_intel
Verifying: Test rap_flake_debug_intel
Verifying: Test gnv1_c96_no_nest_debug_intel
Verifying: Compile wam_debug_intel
Verifying: Test control_wam_debug_intel
Verifying: Compile rrfs_dyn32_phy32_intel
Verifying: Test regional_spp_sppt_shum_skeb_dyn32_phy32_intel
Verifying: Test rap_control_dyn32_phy32_intel
Verifying: Test hrrr_control_dyn32_phy32_intel
Verifying: Test hrrr_control_decomp_dyn32_phy32_intel
Verifying: Test rap_restart_dyn32_phy32_intel
Verifying: Test hrrr_control_restart_dyn32_phy32_intel
Verifying: Compile rrfs_dyn32_phy32_faster_intel
Verifying: Test conus13km_control_intel
Verifying: Test conus13km_2threads_intel
Verifying: Test conus13km_restart_mismatch_intel
Verifying: Compile rrfs_dyn64_phy32_intel
Verifying: Test rap_control_dyn64_phy32_intel
Verifying: Compile rrfs_dyn32_phy32_debug_intel
Verifying: Test rap_control_debug_dyn32_phy32_intel
Verifying: Test hrrr_control_debug_dyn32_phy32_intel
Verifying: Test conus13km_debug_intel
Verifying: Test conus13km_radar_tten_debug_intel
Verifying: Compile rrfs_dyn64_phy32_debug_intel
Verifying: Test rap_control_dyn64_phy32_debug_intel
Verifying: Compile hafsw_intel
Verifying: Test hafs_regional_atm_intel
Verifying: Test hafs_regional_atm_thompson_gfdlsf_intel
Verifying: Test hafs_regional_atm_ocn_intel
Verifying: Test hafs_regional_atm_wav_intel
Verifying: Test hafs_regional_atm_ocn_wav_intel
Verifying: Test hafs_regional_1nest_atm_intel
Verifying: Test hafs_regional_telescopic_2nests_atm_intel
Verifying: Test hafs_global_1nest_atm_intel
Verifying: Test hafs_global_multiple_4nests_atm_intel
Verifying: Test hafs_regional_specified_moving_1nest_atm_intel
Verifying: Test hafs_regional_storm_following_1nest_atm_intel
Verifying: Test hafs_regional_storm_following_1nest_atm_ocn_intel
Verifying: Test hafs_global_storm_following_1nest_atm_intel
Verifying: Test gnv1_nested_intel
Verifying: Compile hafsw_debug_intel
Verifying: Test hafs_regional_storm_following_1nest_atm_ocn_debug_intel
Verifying: Compile hafsw_faster_intel
Verifying: Test hafs_regional_storm_following_1nest_atm_ocn_wav_intel
Verifying: Compile hafs_all_intel
Verifying: Test hafs_regional_docn_intel
Verifying: Test hafs_regional_docn_oisst_intel
Verifying: Test hafs_regional_datm_cdeps_intel
Verifying: Compile datm_cdeps_intel
Verifying: Test datm_cdeps_control_cfsr_intel
Verifying: Test datm_cdeps_restart_cfsr_intel
Verifying: Test datm_cdeps_control_gefs_intel
Verifying: Test datm_cdeps_iau_gefs_intel
Verifying: Test datm_cdeps_stochy_gefs_intel
Verifying: Test datm_cdeps_ciceC_cfsr_intel
Verifying: Test datm_cdeps_bulk_cfsr_intel
Verifying: Test datm_cdeps_bulk_gefs_intel
Verifying: Test datm_cdeps_mx025_cfsr_intel
Verifying: Test datm_cdeps_mx025_gefs_intel
Verifying: Test datm_cdeps_multiple_files_cfsr_intel
Verifying: Test datm_cdeps_3072x1536_cfsr_intel
Verifying: Test datm_cdeps_gfs_intel
Verifying: Compile datm_cdeps_debug_intel
Verifying: Test datm_cdeps_debug_cfsr_intel
Verifying: Compile datm_cdeps_faster_intel
Verifying: Test datm_cdeps_control_cfsr_faster_intel
Verifying: Compile datm_cdeps_land_intel
Verifying: Test datm_cdeps_lnd_gswp3_intel
Verifying: Test datm_cdeps_lnd_gswp3_rst_intel
Verifying: Compile atml_intel
Verifying: Test control_p8_atmlnd_sbs_intel
Verifying: Compile atmw_intel
Verifying: Test atmwav_control_noaero_p8_intel
Verifying: Compile atmwm_intel
Verifying: Test control_atmwav_intel
Verifying: Compile atmaero_intel
Verifying: Test atmaero_control_p8_intel
Verifying: Test atmaero_control_p8_rad_intel
Verifying: Test atmaero_control_p8_rad_micro_intel
Verifying: Compile atmaq_intel
Verifying: Test regional_atmaq_intel
Verifying: Compile atmaq_faster_intel
Verifying: Test regional_atmaq_faster_intel

REGRESSION TEST WAS SUCCESSFUL

The above is output from the new (current state of) RegressionTests_pretest.log with a successful test.

DeniseWorthen

DeniseWorthen commented on Dec 22, 2023

@DeniseWorthen
Collaborator

Don't we just need to say that something failed, rather than listing everything that passed? So if everything is OK

All compilations were successful
All tests ran to completion
All baseline comparisons were successful

And only if something fails, logging specifics about what did not work (a compile, a run, a comparison).

BrianCurtis-NOAA

BrianCurtis-NOAA commented on Dec 22, 2023

@BrianCurtis-NOAA
CollaboratorAuthor

Part of the verification function is to spit out more if there's a failure/missing test/compile. I think having the output above is important, and it cuts down a lot from the verbose RegressionTests_<machine>.log

DeniseWorthen

DeniseWorthen commented on Dec 22, 2023

@DeniseWorthen
Collaborator

Hm, well maybe others can chime in. I find the above too verbose. From the perspective of someone making a PR, all I care about is what doesn't work---either it fails to compile, it fails to run or it fails to compare. No news is good news.

junwang-noaa

junwang-noaa commented on Dec 22, 2023

@junwang-noaa
Collaborator

Maybe just me, I feel it's convenient to see a list of verified tests to have a full picture.

28 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

enhancementNew feature or request

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

    Re-work test verification and add pre-test option in rt.sh for developers to use in PR process. · Issue #2058 · ufs-community/ufs-weather-model