-
Notifications
You must be signed in to change notification settings - Fork 168
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
Consistency check of use of modules and variables in ecf job scripts #629
Consistency check of use of modules and variables in ecf job scripts #629
Conversation
jenkfgdas_sfc.ecf jgfs_wave_prdgen_bulls.ecf jgdas_wave_postsbs.ecf The following scripts changed to remove module load cfp: jenkfgdas_select_obs.ecf jenkfgdas_fcst.ecf jgfs_atmos_awips_master.ecf jgfs_atmos_wafs_grib2.ecf jgfs_atmos_awips_g2_master.ecf jgfs_atmos_fbwind.ecf jgfs_atmos_gempak_ncdc_upapgif.ecf jgfs_atmos_npoess_pgrb2_0p5deg.ecf jgfs_atmos_pgrb2_spec_gempak.ecf jgfs_wave_prdgen_bulls.ecf jgfs_forecast.ecf jgdas_forecast.ecf jgdas_atmos_gempak_meta_ncdc.ecf The following scripts changed to remove module load grib_util: jenkfgdas_fcst.ecf jgfs_atmos_postsnd.ecf jgfs_atmos_wafs_gcip.ecf jgfs_atmos_gempak.ecf jgfs_atmos_gempak_ncdc_upapgif.ecf jgfs_atmos_gempak_meta.ecf jgfs_atmos_npoess_pgrb2_0p5deg.ecf jgfs_atmos_pgrb2_spec_gempak.ecf jgfs_wave_prep.ecf jgfs_wave_gempak.ecf jgfs_wave_prdgen_bulls.ecf jgfs_forecast.ecf jgdas_forecast.ecf jgdas_atmos_gempak.ecf jgdas_atmos_gempak_meta_ncdc.ecf jgdas_wave_prep.ecf The following scripts changed to remove module load libjpeg: jenkfgdas_fcst.ecf jgfs_atmos_postsnd.ecf jgfs_atmos_wafs_gcip.ecf jgfs_atmos_gempak.ecf jgfs_atmos_gempak_ncdc_upapgif.ecf jgfs_atmos_gempak_meta.ecf jgfs_atmos_npoess_pgrb2_0p5deg.ecf jgfs_atmos_pgrb2_spec_gempak.ecf jgfs_wave_gempak.ecf jgfs_wave_prdgen_bulls.ecf jgfs_forecast.ecf jgdas_forecast.ecf jgdas_atmos_gempak.ecf jgdas_atmos_gempak_meta_ncdc.ecf The following scripts changed to remove module load gempak jgfs_wave_prdgen_bulls.ecf The following scripts changed to remove module load hdf5 jgfs_wave_prdgen_bulls.ecf The following scripts changed to remove module load netcdf jgfs_wave_prdgen_bulls.ecf The following scripts changed to remove module load bufr jgfs_wave_prdgen_bulls.ecf The following scripts changed to remove module load esmf jgfs_forecast.ecf jgdas_forecast.ecf Merge NCO resource change: jgfs_atmos_analysis.ecf jgfs_wave_post_bndpnt.ecf jgfs_wave_postpnt.ecf jgdas_atmos_analysis.ecf jgdas_wave_postpnt.ecf GitHub issue NOAA-EMC#587
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @lgannoaa for this upgrade. I have a few comments and questions:
- I see
export USE_CFP=YES
set in many scripts that do not load thecfp
module. This was raised in the issue Check the modules loaded in ecf scripts against the job to ensure that they are used. Do this for all the jobs #587. Ifcfp
is not loaded, is thisexport USE_CFP=YES
still appropriate? If not, then we should remove it. There are quite a few scripts that set this variable. - Please do not include resource updates in this PR. They will be included in a PR of its own once NCO gives a green light that those are the resources they are using. This PR should only address issues raised in Check the modules loaded in ecf scripts against the job to ensure that they are used. Do this for all the jobs #587.
- We will have to ask NCO if it is acceptable to them to include these updates at this stage in the acceptance testing. It is only after a confirmation from them, can we accept these in
feature/ops-wcoss2
. If they disagree, then we will include them intodevelop
at the appropriate time. Nevertheless, this is a desirable changeset.
I have not done a careful review but some things I noticed out right:
- ecf/scripts/enkfgdas/forecast/jenkfgdas_fcst.ecf loads the
esmf
module whereas ecf/scripts/gdas/jgdas_forecast.ecf and ecf/scripts/gfs/jgfs_forecast.ecf don't.
I am not aware of the ensemble forecasts having any different requirements (re. esmf) than the GDAS or GFS forecast. Would you kindly care to explain why is ESMF module required in the ensemble forecast and where it is used (or vice-versa i.e. why the GDAS and GFS forecasts do not need it).
And a book-keeping comment.
The title of the PR is the first few words from the commit message.
Please edit that to reflect what the PR is actually doing. Might I suggest something like "Addresses Issue #587" or "Consistency check of use of modules and variables in ecf job scripts"
A check on job/ush/script from HOMEgfs, I found the following reference to USE_CFP: gldas_forcing.sh exgdas_atmos_chgres_forenkf.sh exgdas_atmos_gldas.sh exgdas_enkf_update.sh exglobal_atmos_analysis.sh exglobal_diag.sh Therefore, remove the export USE_CFP=YES from the following ecflow scripts: jenkfgdas_ecen.ecf jenkfgdas_sfc.ecf jenkfgdas_select_obs.ecf jenkfgdas_post_master.ecf jenkfgdas_fcst.ecf jgdas_forecast.ecf jgdas_atmos_analysis_calc.ecf jgdas_atmos_emcsfc_sfc_prep.ecf jgdas_atmos_tropcy_qc_reloc.ecf jgdas_atmos_gempak_meta_ncdc.ecf jgdas_atmos_post_manager.ecf jgfs_atmos_awips_master.ecf jgfs_atmos_wafs_master.ecf jgfs_atmos_wafs_blending_0p25.ecf jgfs_atmos_wafs_blending.ecf jgfs_atmos_wafs_grib2.ecf jgfs_atmos_wafs_grib2_0p25.ecf jgfs_atmos_awips_g2_master.ecf jgfs_atmos_fbwind.ecf jgfs_atmos_analysis_calc.ecf jgfs_atmos_emcsfc_sfc_prep.ecf jgfs_atmos_tropcy_qc_reloc.ecf jgfs_atmos_gempak.ecf jgfs_atmos_gempak_ncdc_upapgif.ecf jgfs_atmos_npoess_pgrb2_0p5deg.ecf jgfs_atmos_pgrb2_spec_gempak.ecf jgfs_atmos_post_manager.ecf jgfs_forecast.ecf 2. Undo change related to resource updates 3. Remove esmf from efcs. Reference to GitHub NOAA-EMC#587
A check on job/ush/script from HOMEgfs, I found the following reference to USE_CFP: Therefore, remove the export USE_CFP=YES from the following ecflow scripts:
|
@lgannoaa Commit messages and PRs should be single-subject and focus more on a general description of the what, focusing on any interface changes (new variables, argument changes, etc.), along with a "why". We can see a diff of the files you modified for the technical details of what you did, particularly when it is straightforward like this. The also should assume the reader only knows what you put in the message and not anything in the issue or that you were asked to do. For instance, this PR would be something like:
See any of these PRs of mine that were merged recently for additional examples: #526, #570, #581, #612 |
@WalterKolczynski-NOAA my understanding from @arunchawla-NOAA is: I looked back on several closed PR submitted by other co-worker as of 2022/2/3. I did not see that you put similar comment. Going forward, please stop putting this kind of comment on my PR. |
@WeiWei-NCO |
jgdas_wave_prep.ecf jgfs_wave_prep.ecf
@aerorahul , thanks for reopen this PR. After contact Wei Wei, he suggested one more change in wave prep job to remove unused library. This action was done and tested. |
Tested in para. Look good. Thanks! |
* Add support for writing restart file on the write grid comp
Description
The following scripts changed to remove module load wgrib2:
jenkfgdas_sfc.ecf
jgfs_wave_prdgen_bulls.ecf
jgdas_wave_postsbs.ecf
The following scripts changed to remove module load cfp:
jenkfgdas_select_obs.ecf
jenkfgdas_fcst.ecf
jgfs_atmos_awips_master.ecf
jgfs_atmos_wafs_grib2.ecf
jgfs_atmos_awips_g2_master.ecf
jgfs_atmos_fbwind.ecf
jgfs_atmos_gempak_ncdc_upapgif.ecf
jgfs_atmos_npoess_pgrb2_0p5deg.ecf
jgfs_atmos_pgrb2_spec_gempak.ecf
jgfs_wave_prdgen_bulls.ecf
jgfs_forecast.ecf
jgdas_forecast.ecf
jgdas_atmos_gempak_meta_ncdc.ecf
The following scripts changed to remove module load grib_util:
jenkfgdas_fcst.ecf
jgfs_atmos_postsnd.ecf
jgfs_atmos_wafs_gcip.ecf
jgfs_atmos_gempak.ecf
jgfs_atmos_gempak_ncdc_upapgif.ecf
jgfs_atmos_gempak_meta.ecf
jgfs_atmos_npoess_pgrb2_0p5deg.ecf
jgfs_atmos_pgrb2_spec_gempak.ecf
jgfs_wave_prep.ecf
jgfs_wave_gempak.ecf
jgfs_wave_prdgen_bulls.ecf
jgfs_forecast.ecf
jgdas_forecast.ecf
jgdas_atmos_gempak.ecf
jgdas_atmos_gempak_meta_ncdc.ecf
jgdas_wave_prep.ecf
The following scripts changed to remove module load libjpeg:
jenkfgdas_fcst.ecf
jgfs_atmos_postsnd.ecf
jgfs_atmos_wafs_gcip.ecf
jgfs_atmos_gempak.ecf
jgfs_atmos_gempak_ncdc_upapgif.ecf
jgfs_atmos_gempak_meta.ecf
jgfs_atmos_npoess_pgrb2_0p5deg.ecf
jgfs_atmos_pgrb2_spec_gempak.ecf
jgfs_wave_gempak.ecf
jgfs_wave_prdgen_bulls.ecf
jgfs_forecast.ecf
jgdas_forecast.ecf
jgdas_atmos_gempak.ecf
jgdas_atmos_gempak_meta_ncdc.ecf
The following scripts changed to remove module load gempak
jgfs_wave_prdgen_bulls.ecf
The following scripts changed to remove module load hdf5
jgfs_wave_prdgen_bulls.ecf
The following scripts changed to remove module load netcdf
jgfs_wave_prdgen_bulls.ecf
The following scripts changed to remove module load bufr
jgfs_wave_prdgen_bulls.ecf
The following scripts changed to remove module load esmf
jgfs_forecast.ecf
jgdas_forecast.ecf
Merge NCO resource change:
jgfs_atmos_analysis.ecf
jgfs_wave_post_bndpnt.ecf
jgfs_wave_postpnt.ecf
jgdas_atmos_analysis.ecf
jgdas_wave_postpnt.ecf
GitHub issue #587
Type of change
Change ecflow script to remove unused module and test with up-to-date job card resource
How Has This Been Tested?
It has been tested.
Tested COM:
/lfs/h2/emc/ptmp/Lin.Gan/ecfops/com/para/gfs/v16.2
Tested log:
/lfs/h2/emc/ptmp/Lin.Gan/ecfops/com/output/prod/today
Checklist