Add columns for relative test and time step to CSV file containing WE2E test info #776
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
1) Change names of variables that specify names of output variables from "output_varname_..." to "outvarname_...". 2) When setting the value of an output variable, do so only if the corresponding "outvarname_..." variable is passed into the function. 3) Bugfix: In "printf -v ..." statements, the "%s" was omitted. Include these.
…to call independently: 1) Replace the global variable VERBOSE with the input argument "verbose". 2) Remove if-statements involving RUN_ENVIR. These were needed when EMC was using GFDLgrid type grids that were not symmetric about the center of the tile, but now that EMC is using ESGgrid-type grids, they are no longer needed.
…y can be available outside of workflow generation (e.g. during the generation of the CSV file containing test information) by sourcing constants.sh. Details: 1) Move constants NH0, NH3, and NH4 needed in grid generation from setup.sh to the constants.sh. 2) During experiment generation (in setup.sh), place the contents of constants.sh at the top of var_defns.sh. 3) Define new variable CONSTANTS_FN in config_defaults.sh that defines the name of the constants definition file. 4) Source constants.sh in get_gridparams_GFDLgrid.sh in order to have access to NH0, NH3, and NH4 (since those are no longer in config_defaults.sh). 5) Bug fixes in "printf -v ..." commands in setup.sh. 6) Source constants.sh in valid_param_vals.sh so that the constant array "valid_vals_Boolean" can be used in valid_param_vals.sh.
…variable ushdir and set it within the function to allow the function to be used outside of the workflow generation (e.g. when creating a CSV file containing WE2E test information).
…utside of the workflow generation framework (e.g. to enable calling it from get_WE2Etest_names_subdirs_descs.sh to generate the CSV file containing WE2E test information):
1) Change USHDIR to the local variable ushdir and set the latter locally within the function.
2) Change PREDEF_GRID_NAME to predef_grid_name and make the latter a required input argument.
3) Change DT_ATMOS, LAYOUT_X, LAYOUT_Y, and BLOCKSIZE to dt_atmos, layout_x, layout_y, and blocksize and and make the latter set optional input arguments. If these are provided, then they remain the same within the function. If they are not provided (or are provided but their value is a null string), they get reset to the default value for the specified predefined grid (predef_grid_name).
4) Change QUILTING to quilting and make the latter a required input argument.
5) Change the grid parameters that this function sets for a given grid into local variables that get reset to null strings and instead provide as input arguments the names of output variables ("outvarname_...") that can be used to return these local variables back to the calling function or script. Such local variables are returned only if a corresponding "outvarname_..." input argument is specified by the calling function or script.
6) Edit comments; add double quotes for consistency where necessary; remove empty lines or add them as needed.
7) Remove call to the function from within the file (at the end of the file). This means the function cannot be called simply by sourcing the file containing it. Instead, the file must be sourced, but then the function must be called separately (along with an argument list). This is to bring things in line with the way other functions are called in the SRW App.
…uired with the new version of this function); (2) Rename local variables in set_predef_grid_params.sh (by adding double underscores to start and end) so they do not conflict with variable names passed to the function that will be the output variables (i.e. varaible names specified via "outvarname_..." input arguments).
…ach test (relative to running a single 6-hour forecast on the RRFS_CONUS_25km predefined grid).
…ted as a string in google sheets (and doesn't get a number format automatically applied to it).
…n to correct value.
…nd alternate test name column contents into two lines.
…if the variable being passed is empty, boolify() will think no argument is being passed to it.
… validity of the value of generate_csv_file. Use the boolify() function to set its value to "TRUE" or "FALSE".
…rstands that an argument is being passed in even if that argument happens to be an empty string.
…script run_WE2E_tests.sh in order to allow users to prevent creation of a CSV file containing WE2E test information (since this can take a while). Default value of this argument is "TRUE", so users have to explicitly set it to "FALSE" on the command line to prevent generation of the CSV file.
… EXEC_SUBDIR via the input argument exec_subdir.
gsketefian
commented
May 21, 2022
| @@ -27,29 +27,29 @@ | |||
| # Optional verbosity flag. Should be set to "TRUE" or "FALSE". Default | |||
| # is "FALSE". | |||
| # | |||
| # output_varname_test_configs_basedir: | |||
| # outvarname_test_configs_basedir: | |||
There was a problem hiding this comment.
To make variable names shorter, changed output_varname_... to outvarname_... in many places.
gsketefian
commented
May 21, 2022
| @@ -294,15 +294,17 @@ function get_WE2Etest_names_subdirs_descs() { | |||
| # | |||
| verbose=${verbose:-"FALSE"} | |||
| check_var_valid_value "verbose" "valid_vals_BOOLEAN" | |||
| verbose=$(boolify $verbose) | |||
| verbose=$(boolify "$verbose") | |||
There was a problem hiding this comment.
The argument to the boolify() function needs to be surrounded with quotes because otherwise, if the argument happens to be a null string, boolify() will think it is not being passed an argument. This is done in many places.
gsketefian
commented
May 21, 2022
| #----------------------------------------------------------------------- | ||
| # | ||
| ushdir=$( readlink -f "$WE2Edir/../../ush" ) | ||
| . $ushdir/constants.sh |
There was a problem hiding this comment.
Need to source the (new) constants.sh file to have access to the valid_vals_BOOLEAN variable.
gsketefian
commented
May 21, 2022
| # | ||
| ushdir=$( readlink -f "$WE2Edir/../../ush" ) | ||
| . $ushdir/constants.sh | ||
| . $ushdir/set_predef_grid_params.sh |
There was a problem hiding this comment.
These three files are needed in order to calculate grid parameters (nx, ny, and dt_atmos) for each test that uses a predefined grid.
gsketefian
commented
May 21, 2022
| [ "${generate_csv_file}" = "NO" ]; then | ||
| generate_csv_file="FALSE" | ||
| fi | ||
| check_var_valid_value "generate_csv_file" "valid_vals_BOOLEAN" |
There was a problem hiding this comment.
No need to define valid_vals_generate_csv_file since valid_vals_BOOLEAN is identical.
gsketefian
commented
May 21, 2022
| generate_csv_file="FALSE" | ||
| fi | ||
| check_var_valid_value "generate_csv_file" "valid_vals_BOOLEAN" | ||
| generate_csv_file=$(boolify "${generate_csv_file}") |
There was a problem hiding this comment.
The boolify() function can already do what the if-statement was doing, so use it.
gsketefian
commented
May 21, 2022
| # | ||
| prim_test_num_fcsts+=( "'$nf" ) | ||
| # | ||
| #----------------------------------------------------------------------- |
There was a problem hiding this comment.
This section calculates the grid parameters (nx, ny, and dt_atmos) needed to calculate the relative cost of each test.
gsketefian
requested review from
BenjaminBlake-NOAA,
ywangwof,
chan-hoo,
panll,
christinaholtNOAA,
christopherwharrop-noaa,
danielabdi-noaa,
mark-a-potts,
jkbk2004,
willmayfield,
dmwright526,
gspetro-NOAA,
natalie-perlin and
EdwardSnyder-NOAA
as code owners
JeffBeck-NOAA
approved these changes
May 24, 2022
|
I tested this on Hera -- looks good to me. |
danielabdi-noaa
approved these changes
May 25, 2022
danielabdi-noaa
added a commit
to danielabdi-noaa/regional_workflow
that referenced
this pull request
May 31, 2022
danielabdi-noaa
added a commit
to danielabdi-noaa/regional_workflow
that referenced
this pull request
May 31, 2022
danielabdi-noaa
added a commit
to danielabdi-noaa/regional_workflow
that referenced
this pull request
May 31, 2022
danielabdi-noaa
added a commit
to danielabdi-noaa/regional_workflow
that referenced
this pull request
Jun 2, 2022
EdwardSnyder-NOAA
added a commit
that referenced
this pull request
Jun 10, 2022
* update IC/LBC file paths to match new IC/LBC file structure (#766) * update IC/LBC file paths to match new IC/LBC file structure * change forecast length * Update config.community.sh These changes reflects the SRW team's preference for a 12-hours forecast. The gfs.t18z.pgrb2.0p25.f012 file will need to be added to all Level 1 systems. Co-authored-by: Gillian Petro <96886803+gspetro-NOAA@users.noreply.github.com> * add a machine file for MacOS (#777) Co-authored-by: Natalie Perlin <perlin.natalie@gmail.com> * Add columns for relative test and time step to CSV file containing WE2E test info (#776) ## DESCRIPTION OF CHANGES: This PR modifies the script `get_WE2Etest_names_subdirs_descs.sh` that creates the CSV file containing test information the columns for relative cost and time step (`DT_ATMOS`). This allows users to have a better idea of what the cost of each test is (the relative cost is defined such that 1 unit represents the cost of running a single 6-hour forecast on the `RRFS_CONUS_25km` grid). Other related changes: * Add the flag `generate_csv_file` to the `run_WE2E_tests.sh` script to allow users to skip the CSV file generation step (since it can take a while). * Use arguments for the function `set_predef_grid_params()` so that it can be called not just from the experiment generation scripts but also others (in this case from `get_WE2Etest_names_subdirs_descs.sh`). Necessary for this PR. * Move some constants (`NH0`, `NH3`, `NH4`) from `config_defaults.sh` to `constants.sh` to make it easier to access them. * Place the contents of `constants.sh` at the start of each experiment's `var_defns.sh` so that these constants are available to the j-jobs and ex-scripts when `var_defns.sh` is sourced. * Use variable `valid_vals_BOOLEAN` in many more places (which allows many other `valid_vals_...` variables in `valid_param_vals.sh` and `setup.sh` to be removed). * Several other bug fixes. ## TESTS CONDUCTED: The following tests (with results) were conducted on Hera. There were only three (previously existing) failures. ``` * grid_CONUS_25km_GFDLgrid_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16: SUCCESS * grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp: SUCCESS * grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional: SUCCESS * grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2: SUCCESS * grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16: SUCCESS * grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR: SUCCESS * grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_2017_gfdlmp: SUCCESS * grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2: SUCCESS * grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v16: FAILURE * grid_RRFS_CONUS_25km_ics_NAM_lbcs_NAM_suite_HRRR: SUCCESS * grid_RRFS_CONUS_25km_ics_NAM_lbcs_NAM_suite_RRFS_v1beta: SUCCESS * grid_RRFS_CONUScompact_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16: SUCCESS * grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_HRRR: SUCCESS * grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_RRFS_v1beta: SUCCESS * grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR: SUCCESS * grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1alpha: SUCCESS * grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta: SUCCESS * grid_SUBCONUS_Ind_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16: SUCCESS * grid_SUBCONUS_Ind_3km_ics_HRRR_lbcs_RAP_suite_HRRR: SUCCESS * grid_SUBCONUS_Ind_3km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta: SUCCESS * GST_release_public_v1: SUCCESS * MET_ensemble_verification: SUCCESS * MET_verification: SUCCESS * community_ensemble_008mems: SUCCESS * community_ensemble_2mems: SUCCESS * custom_ESGgrid: SUCCESS * custom_GFDLgrid: SUCCESS * custom_GFDLgrid__GFDLgrid_USE_GFDLgrid_RES_IN_FILENAMES_eq_FALSE: SUCCESS * custom_GFDLgrid__GFDLgrid_USE_GFDLgrid_RES_IN_FILENAMES_eq_TRUE: SUCCESS * deactivate_tasks: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_grib2_2019061200: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_grib2_2019101818: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_grib2_2020022518: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_grib2_2020022600: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_grib2_2021010100: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio_2019061200: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio_2019101818: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio_2020022518: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio_2020022600: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio_2021010100: SUCCESS * get_from_HPSS_ics_FV3GFS_lbcs_FV3GFS_fmt_netcdf_2021062000: SUCCESS * get_from_HPSS_ics_GSMGFS_lbcs_GSMGFS: SUCCESS * get_from_HPSS_ics_HRRR_lbcs_RAP: SUCCESS * get_from_HPSS_ics_RAP_lbcs_RAP: SUCCESS * get_from_NOMADS_ics_FV3GFS_lbcs_FV3GFS_fmt_nemsio: SUCCESS * inline_post: SUCCESS * nco_ensemble: SUCCESS * nco_inline_post: SUCCESS * pregen_grid_orog_sfc_climo: SUCCESS * specify_DOT_OR_USCORE: SUCCESS * specify_DT_ATMOS_LAYOUT_XY_BLOCKSIZE: SUCCESS * specify_EXTRN_MDL_SYSBASEDIR_ICS_LBCS: SUCCESS * specify_RESTART_INTERVAL: SUCCESS * specify_template_filenames: SUCCESS * subhourly_post: FAILURE * subhourly_post_ensemble_2mems: FAILURE ``` ## DOCUMENTATION: Documentation is updated via PR #[278](ufs-community/ufs-srweather-app#278) in the ``ufs-srweather-app`` repo. * Bug fix for calculation of number of time steps for a given WE2E test. (#782) ## DESCRIPTION OF CHANGES: This PR fixes a bug in the way the number of time steps (`num_time_steps` in `get_WE2Etest_names_subdirs_descs.sh`) is calculated for each WE2E test when creating the CSV file containing information about the available WE2E tests. ## TESTS CONDUCTED: Reran the `run_WE2E_tests.sh` script (which in turn calls `get_WE2Etest_names_subdirs_descs.sh`), and a CSV file containing the adjusted numbers was successfully created. ## DOCUMENTATION: None needed. * Bugfix: Pass domain name to python plotting scripts (#783) ## DESCRIPTION OF CHANGES: PR #[763](#763) introduced the experiment variable `POST_OUTPUT_DOMAIN_NAME` and used it to modify the names of the UPP output files. This PR adds `POST_OUTPUT_DOMAIN_NAME` as an argument to the python plotting scripts to enable those scripts to form the proper UPP output file names to read in. It also modifies the batch scripts (`qsub_job.sh`, `qsub_job_diff.sh`, `sq_job.sh`, and `sq_job_diff.sh`) to add the new argument to the calls to the python plotting scripts that are called. ## TESTS CONDUCTED: Both python plotting scripts `plot_allvars.py` and `plot_allvars_diff.py` were run on Hera on preexisting UPP output files and completed successfully. The batch scripts `sq_job.sh` and `sq_job_diff.sh` were also run successfully on Hera (`qsub_job.sh` and `qsub_job_diff.sh` were not run on e.g. Cheyenne). ## DEPENDENCIES: PR # in `ufs-srweather-app`. ## DOCUMENTATION: Modifications to the documentation are in PR #. ## ISSUE: Partially resolves issue #781 (still need to introduce tests in `regional_workflow` that run the plotting scripts). ## CONTRIBUTORS: @mark-a-potts pointed out the problem and first made the necessary fixes in the `NOAA-EPIC` fork of `regional_workflow`; @gsketefian introduced those changes to this PR and made edits to the comments and documentation. * created new non-compact grids * fixed typo * Update scripts to work with the latest hashes of UFS_UTILS and UPP (#775) * update input namelist of chgres_cube * update diag_table templates * update scripts * back to original * specify miniconda version on Jet Co-authored-by: Gillian Petro <96886803+gspetro-NOAA@users.noreply.github.com> Co-authored-by: Natalie Perlin <68030316+natalie-perlin@users.noreply.github.com> Co-authored-by: Natalie Perlin <perlin.natalie@gmail.com> Co-authored-by: gsketefian <31046882+gsketefian@users.noreply.github.com> Co-authored-by: Chan-Hoo.Jeon-NOAA <60152248+chan-hoo@users.noreply.github.com>
danielabdi-noaa
added a commit
to danielabdi-noaa/regional_workflow
that referenced
this pull request
Jun 25, 2022
danielabdi-noaa
added a commit
that referenced
this pull request
Jun 27, 2022
danielabdi-noaa
added a commit
to danielabdi-noaa/regional_workflow
that referenced
this pull request
Jul 14, 2022
danielabdi-noaa
added a commit
that referenced
this pull request
Jul 18, 2022
* Bug fix with FIELD_TABLE_FN * Modify crontab management, use config_defaults.sh. * Add status badge. * Update cheyenne crontab management. * source lmod-setup * Add main to set_predef_grid * Bug fix in predef_grid * Don't import dead params. * Fix bug in resetting VERBOSE * Minor fix in INI config. * Construct var_defns components from dictionary. * Allow also lower case variables to be exported. * Updates to python workflow due to PR #776 * Use python versions of link_fix and set_FV3_sfc in job script. * Use python versions of create_diag/model. * Some fixes addressing Christina's suggestions. * Delete shell workflow * Append pid to temp files. * Update scripts to work with the latest hashes of UFS_UTILS and UPP (#775) * update input namelist of chgres_cube * update diag_table templates * update scripts * back to original * specify miniconda version on Jet * Remove -S option from link_fix call. * Fixes due to merge * Cosmoetic changes. Co-authored-by: Chan-Hoo.Jeon-NOAA <60152248+chan-hoo@users.noreply.github.com>
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:
This PR modifies the script
get_WE2Etest_names_subdirs_descs.shthat creates the CSV file containing test information the columns for relative cost and time step (DT_ATMOS). This allows users to have a better idea of what the cost of each test is (the relative cost is defined such that 1 unit represents the cost of running a single 6-hour forecast on theRRFS_CONUS_25kmgrid).Other related changes:
generate_csv_fileto therun_WE2E_tests.shscript to allow users to skip the CSV file generation step (since it can take a while).set_predef_grid_params()so that it can be called not just from the experiment generation scripts but also others (in this case fromget_WE2Etest_names_subdirs_descs.sh). Necessary for this PR.NH0,NH3,NH4) fromconfig_defaults.shtoconstants.shto make it easier to access them.constants.shat the start of each experiment'svar_defns.shso that these constants are available to the j-jobs and ex-scripts whenvar_defns.shis sourced.valid_vals_BOOLEANin many more places (which allows many othervalid_vals_...variables invalid_param_vals.shandsetup.shto be removed).TESTS CONDUCTED:
The following tests (with results) were conducted on Hera. There were only three (previously existing) failures.
DOCUMENTATION:
Documentation is updated via PR #278 in the
ufs-srweather-apprepo.