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.

Sign up for GitHub

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

Jump to bottom

Feature/remove shell workflow #380

Closed
wants to merge 8 commits into from
Closed

Feature/remove shell workflow #380

wants to merge 8 commits into from

Conversation

danielabdi-noaa
Copy link
Collaborator

@danielabdi-noaa danielabdi-noaa commented May 6, 2022 •
edited
Loading

DESCRIPTION OF CHANGES:

This PR removes the shell workflow.

TESTS CONDUCTED:

I have run the tests successfully on HERA and ORION and JET. The successful tests are exactly the same a recent shell workflow generation run done in #744

Out of 56 tests

  • community_ensemble_008mems
  • community_ensemble_2mems
  • deactivate_tasks
  • grid_CONUS_25km_GFDLgrid_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_CONUS_3km_GFDLgrid_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_AK_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_AK_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_CONUS_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
  • grid_RRFS_CONUS_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • 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
  • 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_2017_gfdlmp
  • grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2
  • grid_RRFS_CONUS_25km_ics_NAM_lbcs_NAM_suite_HRRR
  • grid_RRFS_CONUS_25km_ics_NAM_lbcs_NAM_suite_RRFS_v1beta
  • grid_RRFS_CONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
  • grid_RRFS_CONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15_thompson_mynn_lam3km
  • grid_RRFS_CONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_CONUScompact_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_CONUScompact_13km_ics_HRRR_lbcs_RAP_suite_HRRR
  • grid_RRFS_CONUScompact_13km_ics_HRRR_lbcs_RAP_suite_RRFS_v1alpha
  • grid_RRFS_CONUScompact_13km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
  • grid_RRFS_CONUScompact_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_HRRR
  • grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_RRFS_v1beta
  • grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
  • grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1alpha
  • grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
  • grid_RRFS_CONUScompact_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_CONUScompact_3km_ics_HRRR_lbcs_RAP_suite_GFS_v15p2
  • grid_RRFS_CONUScompact_3km_ics_HRRR_lbcs_RAP_suite_HRRR
  • grid_RRFS_CONUScompact_3km_ics_HRRR_lbcs_RAP_suite_RRFS_v1alpha
  • grid_RRFS_CONUScompact_3km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
  • grid_RRFS_NA_13km_ics_FV3GFS_lbcs_FV3GFS_suite_RRFS_v1beta
  • grid_RRFS_SUBCONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_RRFS_SUBCONUS_3km_ics_HRRR_lbcs_RAP_suite_GFS_v15p2
  • grid_SUBCONUS_Ind_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
  • grid_SUBCONUS_Ind_3km_ics_HRRR_lbcs_RAP_suite_HRRR
  • grid_SUBCONUS_Ind_3km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
  • inline_post
  • MET_ensemble_verification
  • MET_verification
  • nco_ensemble
  • nco_grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
  • custom_ESGgrid
  • custom_GFDLgrid
  • custom_GFDLgrid__GFDLgrid_USE_GFDLgrid_RES_IN_FILENAMES_eq_FALSE
  • custom_GFDLgrid__GFDLgrid_USE_GFDLgrid_RES_IN_FILENAMES_eq_TRUE
  • pregen_grid_orog_sfc_climo
  • specify_DOT_OR_USCORE
  • specify_DT_ATMOS_LAYOUT_XY_BLOCKSIZE
  • specify_RESTART_INTERVAL
  • specify_template_filenames

HERA
0 failures

ORION
1 failure

nco_grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR (run_fcst segfault)

JET
5 failures (a lot of improvement since last time)

MET_ensemble_verification (run_gridstatvx_)
MET_verification (run_gridstatvx)
grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_HRRR (make_lbcs)
nco_ensemble (run_fcst)
pregen_grid_orog_sfc_climo (run_fcst)

DEPENDENCIES:

None

DOCUMENTATION:

To be completed ...

ISSUE (optional):

None

CONTRIBUTORS (optional):

@christinaholtNOAA @venitahagerty

@gsketefian
Bug fixes (grid size + suppress screen output from module load) (ufs-…
187f16c 
…community#756)

## DESCRIPTION OF CHANGES: 
1) Adjust y-direction size of write-component grid of `SUBCONUS_Ind_3km` predefined grid from 195 to 197 (this was just an oversight in PR ufs-community#725 ).
2) Redirect output of module load in launch script (`launch_FV3LAM_wflow.sh`) to `/dev/null` to avoid unwanted screen output (which was introduced in PR #[238](ufs-community/ufs-srweather-app#238) in ufs-srweather-app and is about how to load the `regional_workflow` environment and is not relevant in this context).

## TESTS CONDUCTED: 
1) Plotted the `SUBCONUS_Ind_3km` grid to ensure it has correct size (it does).
2) Manually ran `launch_FV3LAM_wflow.sh` from the command line to verify that screen output is suppressed (it is).
@danielabdi-noaa
Copy link
Collaborator Author

@venitahagerty Christina suggested that we collaborate on this PR, in which I replace the shell workflow with python workflow generation code. In the process some shell scripts are removed. I have tried to make sure something used by the job scripts is not removed, but I may have missed something. Please let me know if you have any suggestions.

@christinaholtNOAA The python unittest that I included in this PR seem to run fine here, so I guess the reason why it was not running on ufs-community repo is because it is a fork of fork of it, instead of a direct fork.

@venitahagerty
Copy link

@danielabdi-noaa Do you activate the conda environment regional_workflow before running generate_FV3LAM_wflow? The error I'm getting is that python modules jinja2, yaml, and f90nml are not available.

@danielabdi-noaa
Copy link
Collaborator Author

@danielabdi-noaa Do you activate the conda environment regional_workflow before running generate_FV3LAM_wflow? The error I'm getting is that python modules jinja2, yaml, and f90nml are not available.

Yes, that is correct. Those packages are only available under the regional_workflow environment.

@venitahagerty
Copy link

@danielabdi-noaa I'm not seeing the command to activate regional_workflow in run_WE2E_tests.sh. In which shell is that environment activated?

@danielabdi-noaa
Copy link
Collaborator Author

danielabdi-noaa commented May 6, 2022 •
edited
Loading

@danielabdi-noaa I'm not seeing the command to activate regional_workflow in run_WE2E_tests.sh. In which shell is that environment activated?

@venitahagerty When you load modules as follows the last step does the conda activation.
For example on Hera from the ufs-srweather-app directory:

   source etc/lmod-setup.sh  # for bash login
   module use modulefiles
   module load build_hera_intel
   module load wflow_hera # Here it will print a message
   conda activate regional_workflow

Then you can run the regression test.

@venitahagerty
Copy link

@danielabdi-noaa Sometimes the short range weather app will be run in an automated way. It would be useful if run_WE2E_tests.sh did the activation of the conda environment regional_workflow. Do you have any objections to adding that activation to that shell?

@danielabdi-noaa
Copy link
Collaborator Author

danielabdi-noaa commented May 6, 2022 •
edited
Loading

@danielabdi-noaa Sometimes the short range weather app will be run in an automated way. It would be useful if run_WE2E_tests.sh did the activation of the conda environment regional_workflow. Do you have any objections to adding that activation to that shell?

@venitahagerty I think it is a good idea! The job launch script in regional_workflow/ush has code to load necessary modules, activate conda etc, so I guess we can use something like that to modify run_WE2E_tests.sh as well. @christinaholtNOAA What do you think about this?

@danielabdi-noaa
Copy link
Collaborator Author

danielabdi-noaa commented May 7, 2022 •
edited
Loading

@venitahagerty Unfortunately activation of conda or other libraries needed for getting yaml,jinja etc is not uniform across platofrms. On cheyenne for example, once needs to activate ncarlib in a different way. Even for conda, the way it is activated on odin is different from other platforms, so it maybe best to leave it for the user. One potential solution is to print the exact commands needed to activate the needed environtment, when loading wflow_* modules, so that I can parse that and execute it in run_WE2E_tests.sh script. That will require modifying ufs-srweather-app so will take time. So maybe we can do the manual steps for now? Thanks

JeffBeck-NOAA and others added 3 commits May 9, 2022 10:44
@JeffBeck-NOAA
Update default SPP ISEED array in config_defaults.sh to use unique va…
00e2155 
…lues (ufs-community#759)

* Modify RRFS North America 3- and 13-km domain configuration and WE2E test.

* Modify default ISEED values for SPP

* Fix grid in WE2E test
@chan-hoo
Update workflow python scripts (ufs-community#760)
89a0c7f 
* update python scripts
@chan-hoo
Change output file name of run_post to meet NCO standards (ufs-commun…
1ec6851 
…ity#758)

* change output file name

* change variable name

* update python script

* remove duplicates

* add a check for empty variables

* move variable to common area

* clean up unnecessary comments

* update scripts

* remove duplicate

* update python scripts

* fix user-staged dir path issue in python script
@danielabdi-noaa
Copy link
Collaborator Author

I am going to turn this pull request from draft to ready since it looks like it is working reasonably well on three systems I tested so far.

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

@venitahagerty venitahagerty Awaiting requested review from venitahagerty

@JeffBeck-NOAA JeffBeck-NOAA Awaiting requested review from JeffBeck-NOAA JeffBeck-NOAA will be requested when the pull request is marked ready for review JeffBeck-NOAA is a code owner

@christinaholtNOAA christinaholtNOAA Awaiting requested review from christinaholtNOAA christinaholtNOAA will be requested when the pull request is marked ready for review christinaholtNOAA is a code owner

@christopherwharrop-noaa christopherwharrop-noaa Awaiting requested review from christopherwharrop-noaa christopherwharrop-noaa will be requested when the pull request is marked ready for review christopherwharrop-noaa is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@danielabdi-noaa @venitahagerty @gsketefian @JeffBeck-NOAA @chan-hoo