Pyvista script to create convection box cookbook documentation#6993
Pyvista script to create convection box cookbook documentation#6993JarettBakerDunn wants to merge 2 commits into
Conversation
6b5ae86 to
9028a15
Compare
alarshi
left a comment
There was a problem hiding this comment.
@JarettBakerDunn : Thank you for working on this! I really liked the updated figures you generated.
I left some comments to improve the readability of the code, but otherwise it looks good to me. In my experience using PyVista, the camera settings have been annoying and a result of trial and error. I am not happy that you (or anyone else) have to choose different hard-coded values for different cookbooks, but I don't have an alternative either.
@naliboff: I think you had this idea, so I am curious to know your thoughts on the overall structure for regenerating the plots.
| 2- The provided YAML configuration file <environment.yml> can be used to create the <py_aspect> environment and install all the packages listed above: | ||
| >> conda env create -f environment.yml | ||
| 2- The provided YAML configuration file <env-py_aspect.yml> can be used to create the <py_aspect> environment and install all the packages listed above: | ||
| >> conda env create -f env-py_aspect.yml |
There was a problem hiding this comment.
Ah, thank you for catching this! Could you make this change into a separate PR since it is unrelated to documentation change you are working on?
There was a problem hiding this comment.
Did you write this file because the existing environment file, env-py_aspect.yml, took a long time to install all the packages? It looks like your packages are already part of the available environment file, but that file also includes several other packages that you might not need, or perhaps the versions included differ from what you require?
There was a problem hiding this comment.
Now that I'm looking at it again there are actually some problems with env-py_aspect.yml, one is that it looks like it includes the build strings (the hexadecimal codes after the second equals sign) and the specific version numbers, which prevents me from actually using environment file. After removing all of these, the environment file is usable but I wasn't sure I should make these edits to an existing file.
Also, yes, it is taking a very very long time to install all of these packages and the resulting environment will probably be very big, which is another reason I wanted to make another environment file which was smaller and focused on using pyvista with the docs.
| There is another YAML configuration file <env-py_aspect_docs.yml> which has fewer dependencies and is intended to be used when using | ||
| pyvista to generate figures for ASPECT documentation. This YAML file will create the <py_aspect_docs> environment. The environment | ||
| is created and installed in the same way as <env-py_aspect.yml>. | ||
|
|
There was a problem hiding this comment.
If we decide to keep this file, I would suggest that we move it into $ASPECT_SOURCE_DIR/doc/ folder
| use_3d_text=False,use_2d=True, # These parameters make it easier to plot 2D data | ||
| axes_ranges=[0,1,0,1,0,0], # We displaced mesh_actor earlier, so we need to reset the axes ranges | ||
| grid='front',ticks='outside',location='outer', | ||
| xtitle='X Axis',ytitle='Y Axis'# Model bounds are in meters, kilometers make the labels more readable |
There was a problem hiding this comment.
The comment might be leftover from a previous script, I don't think it applies here.
| # shows the axes labels on the actual mesh (show_axes shows a widget in 3d space) | ||
| plotter.show_bounds( | ||
| use_3d_text=False,use_2d=True, # These parameters make it easier to plot 2D data | ||
| axes_ranges=[0,1,0,1,0,0], # We displaced mesh_actor earlier, so we need to reset the axes ranges |
There was a problem hiding this comment.
Could you use the variable plot_spatial_bounds here instead, since you have already defined it above?
| bounds_array = np.array(plot_spatial_bounds) | ||
| xmag = float(abs(bounds_array[1] - bounds_array[0])) | ||
| ymag = float(abs(bounds_array[3] - bounds_array[2])) | ||
| aspect_ratio = ymag / xmag |
There was a problem hiding this comment.
| aspect_ratio = ymag / xmag |
Since you are redefining it in the line below.
| ) | ||
|
|
||
| # Calculate Camera Position from Bounds | ||
| bounds_array = np.array(plot_spatial_bounds) |
There was a problem hiding this comment.
Hmmm, is there a reason why you converted it to array? I think you can do the subsequent operations using the original list, plot_spatial_bounds. Please correct me if I am wrong.
| xmid = xmag / 2 + bounds_array[0] # X midpoint | ||
| ymid = ymag / 2 + bounds_array[2] # Y midpoint |
There was a problem hiding this comment.
I know this is computing the exact same thing, but could you replace the definitions with
xmid = (plot_spatial_bounds[1] - plot_spatial_bounds[0]) /2 and similarly for ymid?
It will be easier to read that you are computing the midpoint along x and y direction. You could also remove the in-line comments.
| mesh = pv.read("../../output-convection-box/solution/solution-00000.pvtu") | ||
| plot_spatial_bounds = [0, 1, 0, 1, 0, 0] | ||
| mesh = mesh.clip_box(bounds=plot_spatial_bounds, invert=False) | ||
| arrows = mesh.glyph(scale="velocity", factor=0.01,orient="velocity") |
There was a problem hiding this comment.
| arrows = mesh.glyph(scale="velocity", factor=0.01,orient="velocity") | |
| # calculate median velocity for arrow scaling | |
| vel = mesh["velocity"] | |
| vmag = np.linalg.norm(vel, axis=1) | |
| median_v = float(np.median(vmag[vmag > 0])) if np.any(vmag > 0) else 1.0 # median of nonzero velocities, or 1.0 if all velocities are zero | |
| arrows = mesh.glyph(scale="velocity", factor=0.025 / median_v,orient="velocity") |
scale based on median velocity so that factor doesn't have to be tweaked for each use case
ad2123b to
0d9cc98
Compare
0d9cc98 to
e6a9349
Compare
e6a9349 to
aa1c8a0
Compare
This adds a python script to plot the figures in the documentation for the convection box cookbook. This pull request is related to #6826, which I closed in order to preserve the discussion while also not including the crustal deformation scripts and images.
I also added another YAML file to build a conda environment that provides everything needed to run the script.
I had some trouble with the existing YAML environment file, which was not able to find a lot of the required packages.
Aside from that, the py_aspect environment contains a lot of modules which aren't needed to plot the figures, so the py_aspect_docs is a bit smaller.
I am not sure if adding a new conda environment file is the best solution here, please let me know.