Move fixed-file mapping variables out of `config_defaults.yaml`. by danielabdi-noaa · Pull Request #421 · ufs-community/ufs-srweather-app

danielabdi-noaa · 2022-10-17T21:28:28Z

DESCRIPTION OF CHANGES:

This PR mainly addresses issue #367. Fixed files mapping arrays are something that are probably never changed by the user so they don't belog there

The fixed file mapping arrays are removed out of default config file and put in a separate file (fixed_files_mapping.yaml)
Removed set_extrn_mdl_params.py as it barely does anything
Removed LMOD_PATH out of config_defaults because it is not used
LBC_SPEC_FCST_HRS moved out of config_defaults
WORKFLOW_ID moved out of config_defaults

Other mis-placed variables in config_defaults will be moved as I find them

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

TESTS CONDUCTED:

DEPENDENCIES:

None

DOCUMENTATION:

None

ISSUE:

Move fixed-file mapping variables out of config_defaults.yaml #367

CHECKLIST

My code follows the style guidelines in the Contributor's Guide
I have performed a self-review of my own code using the Code Reviewer's Guide
I have commented my code, particularly in hard-to-understand areas
My changes need updates to the documentation. I have made corresponding changes to the documentation
My changes do not require updates to the documentation (explain).
My changes generate no new warnings
New and existing tests pass with my changes
Any dependent changes have been merged and published

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

CONTRIBUTORS (optional):

@gspetro-NOAA

venitahagerty · 2022-10-17T23:50:32Z

Machine: hera
Compiler: intel
Job: WE
Repo location: /scratch1/BMC/zrtrr/rrfs_ci/autoci/pr/1089943228/20221017233518/ufs-srweather-app
Build was Successful
Rocoto jobs started
Long term tracking will be done on 9 experiments
If test failed, please make changes and add the following label back:
ci-hera-intel-WE
Experiment Succeeded on hera: nco_grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
Experiment Succeeded on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
Experiment Succeeded on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
Experiment Succeeded on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_RRFS_v1beta
Experiment Succeeded on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_HRRR
All experiments completed

venitahagerty · 2022-10-18T00:10:32Z

Machine: jet
Compiler: intel
Job: WE
Repo location: /lfs1/BMC/nrtrr/rrfs_ci/autoci/pr/1089943228/20221017233519/ufs-srweather-app
Build was Successful
Rocoto jobs started
Long term tracking will be done on 9 experiments
If test failed, please make changes and add the following label back:
ci-jet-intel-WE
Experiment Succeeded on jet: nco_grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
Experiment Succeeded on jet: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
Experiment Succeeded on jet: grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2
Experiment Succeeded on jet: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
Experiment Succeeded on jet: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
Experiment Succeeded on jet: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR
Experiment Succeeded on jet: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
Experiment Succeeded on jet: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_RRFS_v1beta
Experiment Succeeded on jet: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_HRRR

Also, move other variables that are unlikely/never changed by user.

MichaelLueken

@danielabdi-noaa Nice work! I have some comments, but everything looks good.

MichaelLueken · 2022-10-25T14:30:49Z

+                f'''
+                The specified configuration file does not exist:
+                "{config_file}"'''


For consistency, would it be better to convert the single quotes to double quotes, then convert the double quotes around {config_file} to single quotes?

Note that I did not make these changes by hand, but run a python code formatter on all python files usingblack --safe. I do this from time to time but we should probably apply this on the client/server side and force a coding/formatting standard. Black has a preference for double quotes (single or triple) over single quotes which is why you are seeing those changes.

Thanks for the information about the black python code formatter. From StackOverflow, using Git hooks, it looks like it would be possible to run black on the python scripts before committing changes. Is this python code formatter available on RDHPCS machines, or is this something that you run on your own machine before creating the PR? If it is available on RDHPCS, then it should be fairly easy to set up a Git hook to run before committing (create a .git/hooks/pre-commit hook to use the formatter on the python files).

Do we want to use black, or is there another python code formatter that might be better? Do we want to enforce what black recommends, or create our own standard?

I think this would be a good topic for the SRW meeting this week.

@MichaelLueken I think that is a good idea! black is not available on RDHPCS machines as far as I know but we can use flake8 or something similar.

MichaelLueken

Thanks, @danielabdi-noaa, for addressing my concerns. I approve of this update.

christinaholtNOAA

LGTM!

danielabdi-noaa changed the title ~~Rearrange fixed-file mapping variables location.~~ Move fixed-file mapping variables out of config_defaults.yaml. Oct 17, 2022

danielabdi-noaa added ci-hera-intel-WE Kicks off automated workflow test on hera with intel ci-jet-intel-WE Kicks off automated workflow test on jet with intel labels Oct 17, 2022

venitahagerty removed ci-hera-intel-WE Kicks off automated workflow test on hera with intel ci-jet-intel-WE Kicks off automated workflow test on jet with intel labels Oct 17, 2022

danielabdi-noaa force-pushed the feature/fix_rearrange branch from b99a305 to 58614d8 Compare October 18, 2022 16:00

danielabdi-noaa added 2 commits October 21, 2022 22:52

Move fixed-file mapping variables out of config_defaults.

2a6affd

Also, move other variables that are unlikely/never changed by user.

Minor bug fix.

13d5512

danielabdi-noaa force-pushed the feature/fix_rearrange branch from 58614d8 to 13d5512 Compare October 21, 2022 22:52

Use python script to check python version and packages.

e64be4c

danielabdi-noaa force-pushed the feature/fix_rearrange branch from 525dc29 to e64be4c Compare October 22, 2022 15:45

danielabdi-noaa added 2 commits October 22, 2022 16:37

Minor fixes.

c52e1f5

Format python files with black.

4c02e37

danielabdi-noaa force-pushed the feature/fix_rearrange branch from ebd2555 to 4c02e37 Compare October 22, 2022 17:32

Add pipefail in strict error checking mode.

5abf530

danielabdi-noaa marked this pull request as ready for review October 24, 2022 19:18

danielabdi-noaa requested review from BenjaminBlake-NOAA, JeffBeck-NOAA, RatkoVasic-NOAA, chan-hoo, christinaholtNOAA, christopherwharrop-noaa, gsketefian, jkbk2004, mark-a-potts, mkavulich, panll and ywangwof as code owners October 24, 2022 19:18

danielabdi-noaa requested review from EdwardSnyder-NOAA, MichaelLueken, dmwright526, gspetro-NOAA, natalie-perlin and willmayfield as code owners October 24, 2022 19:18

MichaelLueken reviewed Oct 25, 2022

View reviewed changes

Remove debugging comments from UFS_plot

093b146

danielabdi-noaa force-pushed the feature/fix_rearrange branch from 80b2cc0 to 093b146 Compare October 25, 2022 15:29

danielabdi-noaa added 2 commits October 25, 2022 19:46

More formatting changes.

2ed2733

Removed unused python utilities.

12c41c6

danielabdi-noaa force-pushed the feature/fix_rearrange branch from d78f94e to 12c41c6 Compare October 25, 2022 20:13

MichaelLueken approved these changes Oct 25, 2022

View reviewed changes

MichaelLueken added the run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests label Oct 25, 2022

christinaholtNOAA approved these changes Oct 26, 2022

View reviewed changes

danielabdi-noaa merged commit 6668873 into ufs-community:develop Oct 26, 2022

Conversation

danielabdi-noaa commented Oct 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DESCRIPTION OF CHANGES:

Type of change

TESTS CONDUCTED:

DEPENDENCIES:

DOCUMENTATION:

ISSUE:

CHECKLIST

LABELS (optional):

CONTRIBUTORS (optional):

Uh oh!

venitahagerty commented Oct 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

venitahagerty commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaelLueken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MichaelLueken Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

danielabdi-noaa Oct 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MichaelLueken Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

danielabdi-noaa Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaelLueken left a comment

Choose a reason for hiding this comment

Uh oh!

christinaholtNOAA left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danielabdi-noaa commented Oct 17, 2022 •

edited

Loading

venitahagerty commented Oct 17, 2022 •

edited

Loading

venitahagerty commented Oct 18, 2022 •

edited

Loading

danielabdi-noaa Oct 25, 2022 •

edited

Loading