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

Port obs-monitor to wcoss2 #34

Merged

Conversation

EdwardSafford-NOAA
Copy link
Collaborator

This release allows obs-monitor to run on wcoss2. The strategy is to split the model level yaml plot file into lots of pieces and submitted as a single serial job with one node per piece (plot job). Satellite plots (rad and ozn) are broken down by instrument, for iasi and cris down to instrument/plot_type. The minimization and conventional plots are submitted as two plot jobs without breaking them down further.

There are some caveats:

  • Surface pressure plots are not working on either wcoss2 or hera. This is a problem in eva (mon_data_space.py), and issue #194 has been opened to address it.
  • Logos and tight layout options have been temporarily commented out of the yaml plot tempates. These features fail on wcoss2 and seem to be issues in emcpy. Issue #142 has been opened to address the problem.
  • Some of the plots in parm/gfs/gfs_plots.yaml have been commented out owing to what seems to be file corruption issues in my test data suite (probably hera stmp space issues).

Testing has been conducted on wcoss2 and hera, and all plots work on both machines within the stated caveats.

Save off work & try on hera
Save work in progress.
Add runWcoss.sh script, lump all plots into single job.
Add some Hera items.
Merge branch 'feature/wcoss2-32' of https://github.com/EdwardSafford-NOAA/obs-monitor into feature/wcoss2-32
Fix pycodestyle issues.
Bit more cleanup.
Update J-job.
Clean up.
More cleanup.
@CoryMartin-NOAA
Copy link
Contributor

@EdwardSafford-NOAA looks like you've got a conflict that needs resolved

@@ -460,13 +460,13 @@ graphics:
figure:
layout: [4,1]
figure size: [20,18]
tight layout:
# tight layout:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevindougherty-noaa can we figure out why tight layout and plot logo aren't working on wcoss?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EdwardSafford-NOAA What is the error that occurs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Traceback (most recent call last):
File "/lfs/h2/emc/da/noscrub/edward.safford/eva/opt/bin/eva", line 8, in
sys.exit(main())
File "/lfs/h2/emc/da/noscrub/edward.safford/eva/opt/eva/eva_driver.py", line 120, in main
eva(config_file)
File "/lfs/h2/emc/da/noscrub/edward.safford/eva/opt/eva/eva_driver.py", line 85, in eva
figure_driver(eva_dict, data_collections, timing, logger)
File "/lfs/h2/emc/da/noscrub/edward.safford/eva/opt/eva/plotting/batch/base/plot_tools/figure_driver.py", line 150, in figure_driver
make_figure(handler, figure_conf, plots_conf,
File "/lfs/h2/emc/da/noscrub/edward.safford/eva/opt/eva/plotting/batch/base/plot_tools/figure_driver.py", line 256, in make_figure
fig.save_figure(output_file, **saveargs)
File "/lfs/h2/emc/da/noscrub/edward.safford/eva/opt/emcpy/plots/create_plots.py", line 232, in save_figure
self.fig.savefig(pathfile, **kwargs)
File "/apps/prod/ve/intel/19.1.3.304/python/3.10.4/evs/1.0/lib/python3.10/site-packages/matplotlib/figure.py", line 3343, in savefig
self.canvas.print_figure(fname, **kwargs)
File "/apps/prod/ve/intel/19.1.3.304/python/3.10.4/evs/1.0/lib/python3.10/site-packages/matplotlib/backend_bases.py", line 2366, in print_figure
result = print_method(
File "/apps/prod/ve/intel/19.1.3.304/python/3.10.4/evs/1.0/lib/python3.10/site-packages/matplotlib/backend_bases.py", line 2232, in
print_method = functools.wraps(meth)(lambda *args, **kwargs: meth(
TypeError: FigureCanvasAgg.print_png() got an unexpected keyword argument 'tight layout'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same issue is at play with the logos, it reports an unexpected keyword argument.

@EdwardSafford-NOAA
Copy link
Collaborator Author

Not sure how I managed to create those conflicts. Neither was in my branches on hera or wcoss2. Me and git. :( Resolved now.

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments, mostly looks great!

parm/gfs/gfs_plots.yaml.orig Outdated Show resolved Hide resolved
for yaml in ${DATA}/obs*.yaml; do
echo "${ctr} $yaml"
echo "${ctr} ${APRUN_PY} ${USHobsmon}/plotObsMon.py -i ${yaml} -p ${PDATE}" >> $cmdfile
for yaml in ${DATA}/*.yaml; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry this might not be "safe" enough and perhaps we want a prefix on the YAML or something to ensure it isn't trying to run things it shouldn't?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. I'll see what I can do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resulting yaml files can include:

  • sat_[satname]_[instrument].yaml
  • sat_[satname][instrument][ctr].yaml (sat/instruments with lots of channels)
  • minimization.yaml
  • observations.yaml

What if I precede the file names with OM_ and then do
for yaml in ${DATA}/OM_*yaml; do
Would that be safe enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I think that's good enough, and we can cross a future bridge if we need to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitPlotYaml.py now uses a prefix of OM_PLOT_ to uniquely identify the resulting yaml files, and exobsmon_plot.sh has been modified to search for OM_PLOT*yaml.

ush/runWcoss.sh Outdated
@@ -0,0 +1,25 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about having one run script but it "sources" a machine specific file to do all of the module loads/paths/set APRUN, etc.?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that could work. The command file generated on wcoss2 would source the resource file on the first line then the plot commands would follow. Note that there's nothing needed for hera -- the command file is just run directly with no extra fuss.

Copy link
Collaborator Author

@EdwardSafford-NOAA EdwardSafford-NOAA Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've replaced runWcoss.sh with setup_wcoss2.sh, which is now sourced in the first line of the $cmdfile when run on wcoss2. $cmdfile is then directly run as the batch job on both hera and wcoss2.

Rm gfs_plots.yaml.orig from branch.  It slipped in by mistake.
Add prefix to split yaml files, clean up J-job setup.
Replace runWcoss.sh with a setup script that get's sourced by the
cmdfile.
@CoryMartin-NOAA
Copy link
Contributor

Looks good @EdwardSafford-NOAA merge when ready! (if you didn't before you now have the power to do so)

@EdwardSafford-NOAA EdwardSafford-NOAA merged commit 4a414a4 into NOAA-EMC:develop Jun 26, 2024
2 checks passed
@EdwardSafford-NOAA EdwardSafford-NOAA deleted the feature/wcoss2-32 branch June 27, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants