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

Task name change from 'verb-like' to 'noun-like' to remove prefix 'run_' #633

Open
chan-hoo opened this issue Mar 3, 2023 · 9 comments
Open
Labels
enhancement New feature or request

Comments

@chan-hoo
Copy link
Collaborator

chan-hoo commented Mar 3, 2023

Description

  • As discussed in PR [develop] Add RRFS product generation job to the SRW app #631, the task names of the current FV3-LAM workflow are based on the verb-like naming convention.
  • As a result, the tasks for forecast and post-processing (2 out of 10) and most of the tasks for verification (8 out of 11) have the prefix run_ .
  • Once RRFS is incorporated into the develop branch, 15 tasks (out of 26) will have the prefix run_.
  • To avoid this prefix, all the extra tasks only for AQM do not have this prefix (12 tasks out of 12).
  • This means that the majority of the tasks in the UFS SRW App (37 out of 59) will have the prefix run_ once RRFS is integrated.
  • Since the task switch name begins with run_task_, their name will be run_task_run_ for the run_ tasks where run is repeated.
  • The purpose of the tasks is to run something. Therefore, the prefix run_ seems to be superfluous.
  • This run_ makes the task name long unnecessarily. For example, the change of exregional_run_met_ensemblestat_vx_point.sh to exregional_met_ensemblestat_vx_point.sh will not make users confused.

Solution

For the consistency, I propose to use noun-like naming rather than verb-like as follows:

  • ex-scripts:
    exregional_get_extrn_mdl_files.sh ---> exregional_extrn_mdl_files.sh (task names: get_extrn_ics --> extrn_mdl_files_ics , get_extrn_lbcs --> extrn_mdl_files_lbcs)
    exregional_get_obs_ccpa.sh ---> same
    exregional_get_obs_mrms.sh ---> same
    exregional_get_obs_ndas.sh ---> same
    exregional_make_grid.sh ---> same
    exregional_make_ics.sh ---> same
    exregional_make_lbcs.sh ---> same
    exregional_make_orog.sh ---> same
    exregional_make_sfc_climo.sh ---> same
    exregional_plot_allvars.sh ---> same
    exregional_plot_allvars_diff.sh ---> same
    exregional_run_fcst.sh ---> exregional_forecast.sh
    exregional_run_met_ensemblestat_vx_grid.sh ---> exregional_met_ensemblestat_vx_grid.sh
    exregional_run_met_ensemblestat_vx_point.sh ---> exregional_met_ensemblestat_vx_point.sh
    exregional_run_met_gridstat_vx.sh ---> exregional_met_gridstat_vx.sh
    exregional_run_met_gridstat_vx_emsmean.sh ---> exregional_met_gridstat_vx_emsmean.sh
    exregional_run_met_gridstat_vx_ensprob.sh ---> exregional_met_gridstat_vx_ensprob.sh
    exregional_run_met_pointstat_vx.sh ---> exregional_met_pointstat_vx.sh
    exregional_run_met_pointstat_vx_ensmean.sh ---> exregional_met_pointstat_vx_ensmean.sh
    exregional_run_met_pointstat_vx_ensprob.sh ---> exregional_met_pointstat_vx_ensprob.sh
    exregional_run_post.sh ---> exregional_post.sh
    exregional_run_prdgen.sh ---> exregional_gen_product.sh

  • J-job scripts can be modified similarly.

  • Please let me know if you have any other noun-like names for the above tasks.

Acceptance Criteria (Definition of Done)

  • WE2E fundamental tests run successfully.
@chan-hoo chan-hoo added the enhancement New feature or request label Mar 3, 2023
@gsketefian
Copy link
Collaborator

@chan-hoo That mostly sounds good to me. A couple of things:

  1. The tasks that get the external model files from which the ics and lbcs are generated are currently called get_extrn_ics and get_extrn_lbcs. Instead of changing these to extrn_ics and extrn_lbcs, can we change them to extrn_mdl_files_[ics|lbcs] to really distinguish them from the ics and lbcs tasks and to be consistent with the name of their ex-script (exregional_extrn_mdl_files.sh)?
  2. Can you leave the changing of the vx task ex-scripts to me? In my next 1 or 2 PRs, I'm going to consolidate them into fewer scripts because several of these contain very similar code When I do that, I'll remove the run_ from the names.

Thanks.

@chan-hoo
Copy link
Collaborator Author

chan-hoo commented Mar 6, 2023

@gsketefian, thank you for your comments: 1) I'll update them 2) Sure. I am doing some urgent tasks now. I'll be able to make this change maybe next month unless someone else does this.

@gsketefian
Copy link
Collaborator

@chan-hoo Ok, sounds good. I may get to the changing of the vx task names before next month, we'll see.

@BenjaminBlake-NOAA
Copy link
Collaborator

After some internal discussion at EMC and with @christinaholtNOAA + others on the RRFS merge team, we think it makes sense to keep some verb prefixes attached to the RRFS tasks (and those currently in the SRW app) to keep them somewhat descriptive. However, we do feel that the run_* prefix should be removed, as it is superfluous and does not provide much added value. Here are some suggested names for the current SRW tasks and the RRFS tasks that will be added (excluding the verification tasks which @gsketefian plans to rename). Comments and feedback from anyone is welcomed and encouraged.

Current SRW app tasks
get_extrn_ics --> get_extrn_mdl_files_ics (or leave these two as is)
get_extrn_lbcs --> get_extrn_mdl_files_lbcs
get_obs_ccpa (no change)
get_obs_mrms (no change)
get_obs_ndas (no change)
make_grid (no change)
make_ics (no change)
make_lbcs (no change)
make_orog (no change)
make_sfc_climo (no change)
plot_allvars (no change)
plot_allvars_diff (no change)
run_fcst ---> forecast
run_post ---> post
run_prdgen ---> gen_product (I believe @chan-hoo and @gsketefian suggested this in PR #631 )

RRFS tasks (not yet in SRW app)
run_prepstart ---> prepstart (may want to think of a more descriptive name for what these prepstart tasks actually do)
run_prepstart_ensmean ---> prepstart_ensmean
run_prepstart_fromext ---> prepstart_fromext
process_bufr ---> process_bufrobs (there are bufr tasks which refer to bufr postprocessing)
process_lightning (no change)
process_radarref (no change)
run_ref2tten ---> prep_ref2tten (used in operational HRRR) or ref2tten
run_anal ---> analysis or analysis_gsi
run_enkf ---> enkf or analysis_enkf
run_nonvarcldanl ---> analysis_nonvarcld
run_postanal ---> update_lbc_soil (postanalysis is not good because post typically refers to running the UPP)

@BenjaminBlake-NOAA
Copy link
Collaborator

@JeffBeck-NOAA or @gsketefian - do either of you have any thoughts about the task names I've proposed here? I think it makes sense to keep most of the current task names the same, except for the forecast, post, and product generation jobs. Any feedback is much appreciated!

@gsketefian
Copy link
Collaborator

@BenjaminBlake-NOAA Your proposed renaming sounds good to me. For get_extrn_[ics|lbcs], my preference is to rename them to get_extrn_mdl_files_[ics|lbcs]. Thanks for asking!

@JeffBeck-NOAA
Copy link
Collaborator

@BenjaminBlake-NOAA, everything looks good here. I would also vote for the following:

run_anal ---> analysis_gsi
run_enkf ---> analysis_enkf

@BenjaminBlake-NOAA
Copy link
Collaborator

@gsketefian @JeffBeck-NOAA Thank you both for your feedback! Your suggestions sound good to me. I'll open a PR soon for renaming the tasks currently in the SRW app.

@BenjaminBlake-NOAA
Copy link
Collaborator

@gsketefian In my PR I will not modify the names of the verification tasks, since you had said you would update those at some point to remove the run_ prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants