Add customizable global viz task #383
Conversation
|
Example images produced by this PR: 
|
|
@andrewdnolan and @xylar let me know what you think of a task like this one. I know there's a lot more that could be done to generalize this so I see this PR more as a first step that we could build on. |
cbegeman
force-pushed
the
add-viz-task
branch
from
54d964a to
b22254a
Compare
|
@cbegeman, sorry for the delay on this. I'll take a look tomorrow but at a glance it looks really exciting! |
xylar
left a comment
xylar
left a comment
There was a problem hiding this comment.
This looks really handy! A few small comments for now.
| def determine_time_variable(ds): | ||
| prefix = '' | ||
| time_variable = None | ||
| if 'timeSeriesStatsMonthly' in ds.keys(): |
There was a problem hiding this comment.
I don't think timeSeriesStatsMonthly will be a data variable. Should this be xtime_startMonthly?
| cell_indices = np.where( | ||
| (ds_mesh.maxLevelCell > 0) | ||
| & (lat_cell >= min_latitude) | ||
| & (lat_cell <= max_latitude) | ||
| & (lon_cell >= min_longitude) | ||
| & (lon_cell <= max_longitude) | ||
| ) |
There was a problem hiding this comment.
This culling process is a little bit risky because cells with centers outside the range may nevertheless have vertices in the range and the boundary may look ragged with this approach.
Also, many projections don't have straight edges in latitude/longitude space so this may drop cells that should appear within the project bounds. Might it be better to have mosaic try to do this kind of optimization and not try to handle it on the Polaris side?
There was a problem hiding this comment.
Or I could just check lonVertex/latVertex instead of cell centers?
There was a problem hiding this comment.
I think this is okay for cell-centered fields, though I might need to give it a bit more thought.
What we definitely need to do is iterate over the list of variables and make sure they are cell fields. (Only if this culling does subset the mesh, otherwise no restriction on them having to be cell-fields).
@xylar and I have talked about adding a more complete regional masking functionality (which would replicate the culler), which accepts a cell mask but would allow plotting cell, edge, or vertex fields. That's definitely something I can put at the top of the mosiac development priorities.
andrewdnolan
left a comment
andrewdnolan
left a comment
There was a problem hiding this comment.
This looks great! Thanks for putting it together @cbegeman.
Most of my comments mainly have to do with mosaic limitations. Hopefully I can start tackling some of these limitations soon!
| cell_indices = np.where( | ||
| (ds_mesh.maxLevelCell > 0) | ||
| & (lat_cell >= min_latitude) | ||
| & (lat_cell <= max_latitude) | ||
| & (lon_cell >= min_longitude) | ||
| & (lon_cell <= max_longitude) | ||
| ) |
There was a problem hiding this comment.
I think this is okay for cell-centered fields, though I might need to give it a bit more thought.
What we definitely need to do is iterate over the list of variables and make sure they are cell fields. (Only if this culling does subset the mesh, otherwise no restriction on them having to be cell-fields).
@xylar and I have talked about adding a more complete regional masking functionality (which would replicate the culler), which accepts a cell mask but would allow plotting cell, edge, or vertex fields. That's definitely something I can put at the top of the mosiac development priorities.
| if z_index != 0: | ||
| filename_suffix = f'_z{z_index}' | ||
|
|
||
| if self.config.has_option(section_name, 'colormap_name'): |
There was a problem hiding this comment.
Since colormap only accepts a string, all variables requested will be plotted with the same colormap (if the colormap is specified). Same goes for vmin / vmax.
I think this is fine for now, especially given the viz_dict you've defined. Might be something to tackle later, cause it could cause some unexpected behavior.
There was a problem hiding this comment.
Yeah, I wasn't really sure how much to build out the customization options here. I feel like commented lists aren't great because the user will eventually lose track of the correspondence between variable and options. I also wasn't sure how much we wanted this task to be a stepping stone to MPAS-A. Probably easier to have a conversation some time about philosophically where to go with this.
|
@xylar Thank you! I'll merge after adding some docs. |
3 participants
This PR adds a customizable visualization task for data on MPAS meshes. It has two steps,
viz_horiz_fieldandviz_transect.viz_transectis not run by default but technically either steps is optional. This task does not have a climatology function and thus doesn't produce analogous plots to MPAS-Analysis. It makes use ofplot_global_mpas_field(which in turn uses mosaic) andplot_transectfrommpas_tools.ocean.viz.transect.Checklist
api.md) has any new or modified class, method and/or functions listedTestingcomment in the PR documents testing used to verify the changes