-
Notifications
You must be signed in to change notification settings - Fork 53
WIP: Update MOC mask files and task to handle IndoPacific, Pacific and Indian regions #246
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
Conversation
|
@milenaveneziani, mask files will need to be renamed on several machines to support this change. I want to be careful about this because I don't want to interfere with a-prime. But I think moving to standard names should be a high priority. Perhaps we can move to the new names but make links to the old names where needed, and remove the links when a-prime has moved to the same standard names as MPAS-Analysis? |
|
I've been working on this a bit more and I think it makes sense to put this change on hold for a day or 2 while I try to:
|
|
@xylar: I agree that we can put this on hold for a few days. To reply to your specific points above:
keep in mind that, if we solve 1, we basically finalize the workflow to create oceanMOCBasin files, which will include region masks for the Atlantic, the IndoPacific and the related southern transects (those are the 2 key regions we want MOC for, besides the global region). Also, good news is @vanroekel succeeded in running a short run with MOC AM on (using the old Atlantic mask/transect), so if we get the new mask/transect files, we are done. I will put all the various steps to get to there on the same Confluence page where we discussed ncclimo climos.
yes, this needs to be done in 2 steps. We need to 1) update the postprocessed MOC to add the IndoPacific region, and 2) add a new python script that takes in the MOC AM output fields and produces the same plots as in 1). I was thinking to take this on when we are ready. |
|
It's very encouraging that @vanroekel seems to have got the AM working. I have done quite a number of tests today and, while I am not able to produce a good Atlantic MOC for any grid but EC60to30, I also have not found any problem with the southern boundary transect. For example, I checked that: is always positive except at edges adjacent to land (where fluxes are zero) and it is: I have previously checked that all edges in the transect are south of (or toward land from) the region they're supposed to bound, so I don't think that can be a problem. I tried switching from Any other ideas? |
|
Thanks @xylar for looking into this today. |
|
@milenaveneziani, I looked at both QU240 and EC60to30, with very similar results. I don't have data to work with at higher resolution that EC60to30. If you want to point me to result from a high-res run on NERSC or IC, I'm happy to take a look. Otherwise, it'll have to wait until I get an OLCF account... |
|
@milenaveneziani, actually there's a high res mesh at NERSC, and that's all I would need for this test. I'll take a look. |
|
@milenaveneziani, I checked the oRRS16to6v3 grid and the edge sign seems to be exactly backwards: |
|
Great finding! |
|
We seem to have a solution for the southern boundary: I think I will add some of my recent MOC clean up to this PR, rather than having several of MOC-related PRs. |
| 'regionNames') | ||
|
|
||
| # Load basin region related variables and save them to dictionary | ||
| mpasMeshName = config.get('input', 'mpasMeshName') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@milenaveneziani, please note that I made some significant changes starting here. Please let me know if these changes make sense to you or if you have concerns.
| mpasMeshName = config.get('input', 'mpasMeshName') | ||
| regionMaskDirectory = config.get('regions', 'regionMaskDirectory') | ||
|
|
||
| regionMaskFile = '{}/{}_MOCBasinsAndTransectMasks.nc'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is where the region mask file gets named automatically. We can change the name if you prefer something else.
| 'does not exist') | ||
| iRegion = 0 | ||
| self.dictRegion = {} | ||
| dsMask = xr.open_dataset(regionMaskFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I open the mask/transect file only once outside the loop over regions. I think this makes more sense.
|
|
||
| found = False | ||
| iRegion = 0 | ||
| for mocRegion in dsMask.regionNames.values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous code for determining the region index iRegion was incorrect. It just used the index into the list of regions to be plotted in the config file. I have fixed that here by looping over regions in the file and finding the one matching the requested region (with _MOC added to the region name in the file). Let me know if this makes sense to you.
We probably could have subset the data set using dsMask.sel(nRegion='{}_MOC'.format(region)) but I wasn't sure that would work or if it would be clear to developers...
| # sectionName, dictClimo, | ||
| # dictTseries) | ||
| else: | ||
| self._load_mesh() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes more sense to load the mesh variables only once and to store the results for later use. I guess the only reason not to do that would be if there are memory issues, but these aren't big variables compared to others we load.
| variableList = ['timeMonthly_avg_normalVelocity', | ||
| 'timeMonthly_avg_vertVelocityTop'] | ||
| 'timeMonthly_avg_vertVelocityTop', | ||
| 'timeMonthly_avg_layerThickness'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add layerThickness so we can compute layerThicknessEdge from it later on. This way, we're not relying on refLayerThickness.
| velArea = verticalVel * self.areaCell[:, np.newaxis] | ||
|
|
||
| layerThicknessCell = annualClimatology.avgLayerThcikness.values | ||
| layerThicknessEdge = \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I compute layer thickness at edges by averaging layer thickness from adjacent cells (exactly as done in MPAS-Ocean).
| @@ -0,0 +1,53 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a script for generating the MOC mask files. Please let me know:
- If you're okay with where this is stored. There would also be scripts in
preprocess/observationsfor generating observations. - If you're good with how the script works (e.g. just taking grid name and assuming the mask creator, mesh file, boundary extractor, etc. are linked locally). Alternatively, one could supply the path to the MPAS-Tools and geometric_features repos, and the script would handle the details. I'm not sure that's really easier...
|
@milenaveneziani, let me know what you think of these changes overall. I would like to merge them in combination with re-generating the MOC mask/transect files on NERSC. It might also be a good time to think about storing the mask and observation files in a common repo somewhere so they can more easily be copied across machines... |
881ea18 to
e63a7af
Compare
e63a7af to
1fda6e6
Compare
About this, the idea has always been to transfer mask and observation files to the ACME repo. We can also keep supporting our current locations: |
This would make the masks an observations available only to E3SM developers, correct? This may be necessary, since the grids are considered proprietary right now. But it isn't useful for MPAS-Analysis users outside of ACME. So it would be worth also having instructions for how general users can set up these masks for whatever mesh they are using. Currently, this is also problematic because the MPAS-Tools repository is not public. |
|
you are right. I think in the past we mentioned github as an option, although we can easily reach maximum capacity. |
|
I concur @milenaveneziani, as far as I know we never reached consensus. |
|
I believe I have finally fixed the southern boundary extractor for MOC regions, see MPAS-Dev/MPAS-Tools#181 I'm working on testing at high resolution but my EC60to30 and QU240 results (both with and without land ice) are promising so far. |
|
I have updated this PR to handle the transects created by this tool and to plot the IndoPacific region as well by default and the Pacific and Indian regions optionally. |
|
This branch is so stale that I think it would be best to start over when the time comes. I'm closing it. |


With this merge, the MOC analysis task supports mask files generated from the MOC basins defined in geometric_features, not just the Atalntic.
The name of the mask file is constructed automatically from the mesh name as
{mesh}_MOCBasinsAndTransectMasks.nc. The expected region names are{region}_MOC, rather than just{region}.A preprocessing script for generating the mask files is added to the repo.