MPAS-Analysis icon indicating copy to clipboard operation
MPAS-Analysis copied to clipboard

Add Greenland shelf regions to ocean analysis tasks

Open cbegeman opened this issue 3 months ago • 11 comments

Adds a default config section for Greenland regional TS Diagrams, hovmoller plots, and regional profiles. All new plots are off by default.

The option to designate 'all' regions in a regionGroup was added to the regional ocean profiles task.

cbegeman avatar May 10 '24 21:05 cbegeman

@alexolinhager Do you want to look over the default config options here and see if we should adjust any of the settings, particularly as it pertains to T, S, and rho?

cbegeman avatar May 10 '24 22:05 cbegeman

To be merged after https://github.com/MPAS-Dev/geometric_features/pull/199

cbegeman avatar May 10 '24 22:05 cbegeman

Example results are located here: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.cbegeman/analysis/20231105.v3b01.piControl.chrysalis/gris_regions_0001-0050_climo_0001-0050/ocean/index.html

cbegeman avatar May 10 '24 22:05 cbegeman

This PR has been extended to include a few more analysis types shown here: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.cbegeman/analysis/20231105.v3b01.piControl.chrysalis/gris_regions_0001-0050_climo_0001-0050_profiles/ocean/index.html.

cbegeman avatar May 15 '24 02:05 cbegeman

@cbegeman, the version of geometric_features with these regions (1.4.0) should now be available. Could you update the constraint on geometric_features here (it occurs in 2 places I think)?

What would you like me to focus on in my review? Code? Testing? both?

xylar avatar May 16 '24 17:05 xylar

@xylar Does this commit look like what you had in mind https://github.com/MPAS-Dev/MPAS-Analysis/pull/1004/commits/f2618435d5ec3c40fbf14a9f0fb53c9230a73494? I haven't had a chance to re-test yet so if you have a chance to test that would be great.

cbegeman avatar May 16 '24 21:05 cbegeman

@cbegeman, could you rebase off of develop? I think this is probably bringing in merge commits from main.

xylar avatar May 16 '24 21:05 xylar

And, yes, the last commit looks perfect, thanks.

xylar avatar May 16 '24 21:05 xylar

@cbegeman, could you rebase off of develop? I think this is probably bringing in merge commits from main.

At the time at which you commented, I think I had already rebased off develop. Can you take a look when you have a chance and let me know if it still looks wrong to you?

cbegeman avatar May 16 '24 23:05 cbegeman

I ran some tests an things ran great for a QUwISC240 run but failed on a QU480 mesh:

Plotting Potential Temperature time series vs. depth...
  Load ocean data...
  Read in depth...
analysis task hovmollerOceanRegions_plotHovmoller_potentialTemperature_ISMIP6_Greenland_Central_West_Shelf failed during run
Traceback (most recent call last):
  File "/gpfs/fs1/home/ac.xylar/mpas_work/MPAS-Analysis/add-greenland-regions/mpas_analysis/shared/analysis_task.py", line 322, in run
    self.run_task()
  File "/gpfs/fs1/home/ac.xylar/mpas_work/MPAS-Analysis/add-greenland-regions/mpas_analysis/ocean/plot_hovmoller_subtask.py", line 382, in run_task
    fig, _, suptitle = plot_vertical_section_comparison(
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/gpfs/fs1/home/ac.xylar/mpas_work/MPAS-Analysis/add-greenland-regions/mpas_analysis/shared/plot/vertical_section.py", line 407, in plot_vertical_section_comparison
    _, ax = plot_vertical_section(
            ^^^^^^^^^^^^^^^^^^^^^^
  File "/gpfs/fs1/home/ac.xylar/mpas_work/MPAS-Analysis/add-greenland-regions/mpas_analysis/shared/plot/vertical_section.py", line 885, in plot_vertical_section
    maskedTriangulation, unmaskedTriangulation = _get_triangulation(
                                                 ^^^^^^^^^^^^^^^^^^^
  File "/gpfs/fs1/home/ac.xylar/mpas_work/MPAS-Analysis/add-greenland-regions/mpas_analysis/shared/plot/vertical_section.py", line 1128, in _get_triangulation
    triMask = np.zeros((nx, ny, 2), bool)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: negative dimensions are not allowed

I suspect the issue is just that some of the regions are completely empty and we need a check for that and some way to handle it. I'm happy to figure this out and fix it, I just don't have time until maybe Tuesday or so.

xylar avatar May 17 '24 04:05 xylar

@xylar Thanks for the suggestions and testing! Glad to hear you support adding Greenland regions to polar_regions; I wasn't sure how many additional plots were acceptable.

Seems like we would want to address the empty regions issue in a separate PR but let's put this one on hold until we're sure that's the reason why QU480 fails.

cbegeman avatar May 17 '24 14:05 cbegeman

@xylar I think this is ready to merge when we figure out how to handle the empty-mask case. Let me know if you need help with that.

cbegeman avatar May 22 '24 22:05 cbegeman

@cbegeman, sorry, lost track of that one! I'll work on it today.

xylar avatar May 23 '24 06:05 xylar