[develop] Add plotting scripts to the workflow.

[danielabdi-noaa]

DESCRIPTION OF CHANGES:

This PR addresses issue #481 . The plotting scripts under ush/Python are incorporated to the workflow.

Detailed set of changes:

• A new task task_plot_allvars is added to config_defaults.yaml.

task_plot_allvars:
  PLOT_ALLVARS_TN: "plot_allvars"
  NNODES_PLOT_ALLVARS: 1
  PPN_PLOT_ALLVARS: 24
  WTIME_PLOT_ALLVARS: 00:20:00
  MAXTRIES_PLOT_ALLVARS: 1
  # Reference experiment's COMOUT directry
  # This should be a template to compare multiple cycle dates and hours
  COMOUT_REF: ""
  # Plot fcts start and increment, and end (by default the forecast length)
  PLOT_FCST_START: 0
  PLOT_FCST_INC: 3
  PLOT_FCST_END: ""

• Associated j-job and a python ex-script are added. I believe it is Ok to have a python ex-script for NCO operations.
• plot_allvars and plot_allvars_diff are done within the same task. The user would need to provide the baseline experiment directory EXPTDIR_REF if diff plots are needed.
• Shape file loacations are added for each platform with the name FIXshp
• The ush/Python folder is removed. It makes life easier because job submitting scripts do not need to be modified
• Modify the plotting scripts:
  • Make them work with NCO mode
  • add logging capability with different levels
  • non-positional arguments
  • silence deprecation and other warnings. Now clean INFO messages printed.
  • Bug fix with the diff plotter regarding number of ticks/labels mismatch, which was causing the workflow to fail
  • ensemble runs in NCO mode don't work at the moment because file names have .mem1.
• New WE2E test case added to test plotting scripts

grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional_plot

Plots

Here is the result for 10m wind at f001 forecast hour. I compared to the same experiment so there should not be any diff, which is why third plot is blank.

Here is a better one with two different CCPP physics suites, namely, FV3_GFS_2017_gfdlmp and FV3_GFS_2017_gfdlmp_regional

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:

I've run the WE2E test case added in this PR on Hera, Jet and Orion

• hera.intel
• orion.intel
• cheyenne.intel
• cheyenne.gnu
• gaea.intel
• jet.intel
• wcoss2.intel
• NOAA Cloud (indicate which platform)
• Jenkins
• fundamental test suite
• comprehensive tests (specify which if a subset was used)

DEPENDENCIES:

None

DOCUMENTATION:

Needs documentation update. I think the entire Graphics section can be removed, which is what motivated this work.
https://srw-ug.readthedocs.io/en/release-public-v2.1.0/Graphics.html

ISSUE:

• Plotting scripts should be part of the workflow #481

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:

• Work In Progress
• bug
• enhancement
• documentation
• release
• high priority
• run_ci
• run_we2e_fundamental_tests
• run_we2e_comprehensive_tests
• Needs Cheyenne test
• Needs Jet test
• Needs Hera test
• Needs Orion test
• help wanted

CONTRIBUTORS (optional):

[MichaelLueken]

These changes look good to me! I ran the new test on both Hera and Cheyenne and the test successfully completed on both machines. Approving this work.

[MichaelLueken]

Good morning, @RatkoVasic-NOAA! Just wanted to check and see if having an ex-script in python is against NCO guidelines. Thanks!

Good morning, @BenjaminBlake-NOAA and @dmwright526! Given the nature of these changes, and the fact that you are both co-creators of the python plotting scripts, I would like for you to make sure that you are okay with these changes as well. Thanks!

Good morning, @chan-hoo and/or @MatthewPyle-NOAA! Would it be possible to run the new plotting WE2E test on WCOSS2? That way, we can address any issues there before this work is merged into develop. Thanks!

[chan-hoo]

@MichaelLueken, I am sorry for the late response (Something is wrong. I didn't receive an email from GitHub ...). I'll test it on wcoss2.

[MichaelLueken]

@chan-hoo Thank you very much for testing this on WCOSS2!

[chan-hoo]

@danielabdi-noaa, unfortunately, the 'pygrib' package is not available on wcoss2. NCO denied it due to the dependency on eccodes.

[BenjaminBlake-NOAA]

@danielabdi-noaa @MichaelLueken As @chan-hoo noted, pygrib is not available on WCOSS2. Its replacement is grib2io, which will require a fair amount of script changes. Does it make sense to create another version of each plotting script for WCOSS2?

[chan-hoo]

@danielabdi-noaa, is it possible to replace 'pygrib' with 'grib2io' for all the platforms?

[danielabdi-noaa]

@chan-hoo @BenjaminBlake-NOAA Thanks for testing on wcoss2. Too bad pygrib is not available there, but it is good to know there is work to do on WCOSS2. The scope of this PR is to have the plotting scripts incorporated to the workflow, and they are run optionally so should not be a problem to have a system that they don't work on. Besides the pygrib/grib2io issue, it may also be a good idea to merge the two scripts since there seems to be a fair amount of duplication of code. So maybe those can be targets for a future PR? If @chan-hoo or @BenjaminBlake-NOAA feel that can be done in this PR relatively easily, let me know so I can add you as a collaborator. I am not well versed in the scripts to make the modifications

[BenjaminBlake-NOAA]

@danielabdi-noaa Eventually we will definitely want the plotting scripts to work on WCOSS2, but I think for now I agree with you that making the changes to get the scripts working there is outside the scope of this PR. @chan-hoo maybe once this PR is merged we can open a new PR to get the scripts working on WCOSS2?

I do want to take a closer look at these changes since I was one of the original creators of the plotting scripts, so I can test the changes on Hera. I might not get to it until tomorrow or Friday though - hopefully that is ok!

[chan-hoo]

@danielabdi-noaa, I agree with you, too. I'll work with @BenjaminBlake-NOAA and open a new PR in the future.

[MichaelLueken]

@chan-hoo @BenjaminBlake-NOAA If possible, I think it would be best to not include separate scripts for WCOSS2. The regional_workflow conda environment on Tier-1 platforms includes pygrib. We will need to coordinate with @natalie-perlin to replace this with grib2io.

[chan-hoo]

@MichaelLueken , yes, I agree. We will search for a good option to use one script for all the platforms. If @natalie-perlin adds grib2io to all the other platforms, it will make our work much easier. @natalie-perlin, please let me know what you think.

[MichaelLueken]

While the required number of approvals have been met, @BenjaminBlake-NOAA hasn't had the opportunity to review these changes yet. Adding the DO_NOT_MERGE label until @BenjaminBlake-NOAA has reviewed these changes and have approved.

[BenjaminBlake-NOAA]

Thanks @MichaelLueken ! I will add my approval once I get a chance to review the changes.

[BenjaminBlake-NOAA]

@danielabdi-noaa I was able to run a successful test on Hera and everything looks great, thanks for all your work in putting this together!

One question I have is regarding the config.yaml file. When I copied config.community.yaml and generated the workflow initially, the plot_allvars task was not included in my FV3LAM_wflow.xml file. Would it be possible to add this line to config.community.yaml (and maybe also config.nco.yaml)? Then if users want to run the plotting job they just need to set it to true.

RUN_TASK_PLOT_ALLVARS: false

[danielabdi-noaa]

@BenjaminBlake-NOAA Thanks for taking the time to review and run the tests on Hera! I've now incorporated the changes you requested and run the community test case, which just finished with plots produced every 3hr fcst upto 12hrs.

[MichaelLueken]

The DO_NOT_MERGE label has been removed following approval from @BenjaminBlake-NOAA.

@danielabdi-noaa Are there any further changes that you would like to make to this PR, or is this work now ready to be merged?

[danielabdi-noaa]

@MichaelLueken It is ready to be merged, thanks!