Add global moc postprocess #93

pwolfram · 2017-01-31T20:27:21Z

This merge migrates postprocess-computed meridional overturning circulation calculations.

Previously numpy commands were not valid to specify config file settings. Now, numpy commands can be used as the values in config key-value pairs. This is beneficial because long arrays are now easily specified, e.g., np.arange(0,1,100).

Takes existing moc.py workflow and integrates it into MPAS-Analysis.

pwolfram · 2017-01-31T20:28:17Z

This follows directly from @mark-petersen's moc.py script. Note, the config file used to test following the tutorial is at https://gist.github.com/dad27a6991e17866e6ef4f3e21263230. This presumes that #92 to allow numpy usage in config files has already been fulfilled.

pwolfram · 2017-01-31T20:43:29Z

Note, per @xylar's earlier request the hard-coded file names will likely need to be fixed during the review unless @mark-petersen would prefer this script be handled differently.

xylar · 2017-01-31T23:12:43Z

@pwolfram, this seems like a nice start. But currently it seems too much like a direct translation of @mark-petersen's script and doesn't take enough advantage of the analysis infrastructure.

I'm only part way through looking over the code so I'll try to give some more comments over the next few days as I have time.

pwolfram · 2017-01-31T23:18:18Z

@xylar, I attempted to make this as faithful to the moc.py script as possible to respect @mark-petersen's code but am happy to make edits to meld this more into the MPAS-Analysis framework.

xylar · 2017-01-31T23:22:42Z

@pwolfram, I think when we bring scripts into MPAS-Analysis, it's more important to be "faithful" to the design concept of this repo than that of the original script. I appreciate what you did as kind of a teaching tool but an important part of translating scripts to this repo is showing how files can be found without having to use paths in the config file, how times can be selected without the need for time indices, etc.

pwolfram · 2017-01-31T23:28:34Z

@xylar, agreed. I need to update this when I get a chance.

milenaveneziani · 2017-02-01T20:31:17Z

@pwolfram: thanks for doing this. I agree with @xylar that we should integrate this fully within MPAS-Analysis.
In my mind, this was the plan I envisioned for adding MOC (and this is reflected in the MPAS-Analysis roadmap on Confluence):

Add script for postprocessed MOC, but for the global calculation only. Also add the timeseries part to this script (max MOC at 26.5N, where we can eventually compare with the RAPID observational array, when it makes sense to do so);
Solve issue of extracting a southern boundary out of our MOCbasin regions (Add an extractor for the southern boundary of MOCs MPAS-Tools#147)
Generate mask files for MOCbasins and MOCsouthernBoundary for our main MPAS meshes
Add regional MOC computation to postprocessed MOC script
Test MOC analysis member in ACME, both global and regional calculation
Add script to plot MOC from the AM. Add option to verify whether MOC AM was turned on: if not, use postprocessed MOC script instead. Exit with a warning that no MOC can be calculated if no fields necessary to compute postprocessed MOC was saved.

So, for this PR, we would just need to address 1).

How does this sound?

xylar

Oops, same issue here that my comments didn't get posted. Sorry about that.

xylar · 2017-01-31T23:01:40Z

config.analysis

+[moc_postprocess]
+filename  = ../mpaso.hist.am.timeSeriesStats.yr100-119.avg.nc
+filenamemesh = ../mpaso.rst.0002-01-01_00000.nc
+filenamemask = ../EC60to30v3_SingleRegionAtlanticWTransportTransects_masks.nc


The first two of these filenames need to be read in from the streams file like normal. In place of the third file name, I would strongly prefer that an automated process be devised for generating the mask file. Presumably this would be achieved by running scripts from the GeometricFeatures repository, and maybe also adding some geometry to that repo. I'm definitely opposed to adding any new steps where the user has to generate a resolution-specific file before calling analysis. This is precisely the kind of thing that is already a big hindrance, especially in seaice_modelvsobs.

I would comment out the Atlantic mask file for now, because we want to be plotting only the global MOC for this PR (see my initial comment and point 1)).

xylar · 2017-01-31T23:07:01Z

config.analysis

+filenamemask = ../EC60to30v3_SingleRegionAtlanticWTransportTransects_masks.nc
+timeIndex = 0
+horVelName = time_avg_normalVelocity
+vertVelName = time_avg_vertVelocityTop


Shouldn't these two be handled through the variableMap instead of being config options?

xylar · 2017-01-31T23:07:40Z

config.analysis

+filename  = ../mpaso.hist.am.timeSeriesStats.yr100-119.avg.nc
+filenamemesh = ../mpaso.rst.0002-01-01_00000.nc
+filenamemask = ../EC60to30v3_SingleRegionAtlanticWTransportTransects_masks.nc
+timeIndex = 0


Should this be a time rather than a time index?

milenaveneziani · 2017-02-02T18:23:57Z

config.analysis

 [seaice_modelvsobs]
 generate = 1

+[moc_postprocess]


We probably do not need a specific section for mocPostprocess (it could just be 'MOC'), but I am OK with leaving this section name for now until we'll have the moc AM stuff in.

milenaveneziani · 2017-02-02T18:25:55Z

config.analysis

 compare_with_obs = True
 # Number of points over which to compute moving average (e.g., for monthly
 # output, N_movavg=12 corresponds to a 12-month moving average window)
 N_movavg = 12


I would remove the moc_timeseries section, and add everything that is in there under moc_postprocess. This is because we will want to add the MOC time series to the current script that does the MOC vertical section.

xylar · 2017-02-02T18:57:54Z

@pwolfram, you will have to decide if you want this merge before #86 (which still needs some work but is getting closer). Obviously there will be a non-negligible amount of merging work needed to handle make make this module compatible with the new config layout.

milenaveneziani · 2017-02-02T19:19:21Z

Also, @pwolfram, I think it is appropriate to add the timeseries plot to this script before merging, if you are OK with the idea.
I could look into it more and suggest some related changes, if it helps.

pwolfram · 2017-02-02T19:48:54Z

@xylar and @milenaveneziani, I think it is best to wait on this until #86 and add the timeseries plot too. I don't think there is a rush on this PR.

milenaveneziani · 2017-03-14T20:23:35Z

This PR has been superseded by #139 (where the comments added here have been addressed).

pwolfram added 3 commits January 31, 2017 13:16

Specify and apply numpy commands in config files

7b6d3ad

Previously numpy commands were not valid to specify config file settings. Now, numpy commands can be used as the values in config key-value pairs. This is beneficial because long arrays are now easily specified, e.g., np.arange(0,1,100).

Adds meridional overturning circulation diagnostic

9fca699

Takes existing moc.py workflow and integrates it into MPAS-Analysis.

BFB Cleanup: adds docstring and PEP8 compliance

6c3fbad

pwolfram added the enhancement label Jan 31, 2017

pwolfram requested a review from mark-petersen January 31, 2017 20:27

pwolfram mentioned this pull request Feb 2, 2017

Specify and apply numpy commands in config files #92

Merged

xylar requested changes Feb 2, 2017

View reviewed changes

milenaveneziani reviewed Feb 2, 2017

View reviewed changes

milenaveneziani changed the title ~~Add moc postprocess~~ Add global moc postprocess Feb 2, 2017

milenaveneziani added the in progress label Feb 27, 2017

milenaveneziani force-pushed the develop branch from 42c8112 to 9a554a0 Compare March 9, 2017 16:06

milenaveneziani closed this Mar 14, 2017

pwolfram deleted the add_moc_postprocess branch March 24, 2017 17:23

Add global moc postprocess #93

Add global moc postprocess #93

Uh oh!

Conversation

pwolfram commented Jan 31, 2017

Uh oh!

pwolfram commented Jan 31, 2017

Uh oh!

pwolfram commented Jan 31, 2017

Uh oh!

xylar commented Jan 31, 2017

Uh oh!

pwolfram commented Jan 31, 2017

Uh oh!

xylar commented Jan 31, 2017

Uh oh!

pwolfram commented Jan 31, 2017

Uh oh!

milenaveneziani commented Feb 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xylar left a comment

Choose a reason for hiding this comment

Uh oh!

xylar Jan 31, 2017

Choose a reason for hiding this comment

Uh oh!

milenaveneziani Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

xylar Jan 31, 2017

Choose a reason for hiding this comment

Uh oh!

xylar Jan 31, 2017

Choose a reason for hiding this comment

Uh oh!

milenaveneziani Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

milenaveneziani Feb 2, 2017

Choose a reason for hiding this comment

Uh oh!

xylar commented Feb 2, 2017

Uh oh!

milenaveneziani commented Feb 2, 2017

Uh oh!

pwolfram commented Feb 2, 2017

Uh oh!

milenaveneziani commented Mar 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

milenaveneziani commented Feb 1, 2017 •

edited

Loading