Optimize generate_box_mesh
#458
Conversation
majosm
force-pushed
the
optimize-box-mesh-gen
branch
from
a1e9116 to
15f51ca
Compare
majosm
force-pushed
the
optimize-box-mesh-gen
branch
2 times, most recently
from
ee58bb5 to
5bfbe52
Compare
|
@MTCam was still seeing some slowness after the box mesh changes, which turned out to be due to inefficiencies in the periodic boundary processing (specifically, the vertex matching). My measurements above are for a nonperiodic mesh, so the issue isn't present there. I modified the vertex matching code to use a hybrid of the two existing algorithms; it does spatial partitioning until the number of vertices to compare is sufficiently small, then it switches to a direct pairwise distance calculation. Before: After: |
|
Also, I just remembered today that #352 exists, which would eliminate the last remaining non-numpy loop here. I think I'll try to resurrect that in a separate PR rather than dragging this one out any further. |
alexfikl
left a comment
alexfikl
left a comment
There was a problem hiding this comment.
Took a look over the mesh/generation.py changes and it mostly looks good to me! I left some "thinking out loud" comments to maybe reduce some memory allocations, but otherwise just small nitpicks.
Very impressive performance improvements though! 😁
meshmode/mesh/processing.py
Outdated
| # Compute an approximate median vertex coordinate | ||
| hist, bin_edges = np.histogram( | ||
| both_vertices[idim_largest, :], | ||
| both_vertices.shape[1]) | ||
| nvertices_up_to_bin = np.cumsum(hist) | ||
| median_bin = np.where(nvertices_up_to_bin > both_vertices.shape[1]/2)[0][0] | ||
| median_coord = bin_edges[median_bin] |
There was a problem hiding this comment.
Yeah, turns out np.median performs about the same as the np.histogram approximation. I switched it over.
meshmode/mesh/processing.py
Outdated
|
|
||
| # {{{ vertex matching | ||
|
|
||
| def _rec_match( |
There was a problem hiding this comment.
There is meshmode.mesh.find_point_permutation, which does the same thing, minus all the algorithmic bells and whistles. I feel that this could just become that routine. The interface is a bit simpler, but I think that's probably a plus. I think the "indices" handling could probably be done separately.
Coincidentally, find_point_permutation is also untested, so we should fix that.
majosm
force-pushed
the
optimize-box-mesh-gen
branch
2 times, most recently
from
7f01e4e to
b16df22
Compare
|
Maybe squash into two commits? |
* optimize generate_box_mesh * optimize boundary processing loop * fix type annotation * parameterize dtypes * simplify definition of a0/a1 for 1D * use np.column_stack() instead of np.array([...]).T * avoid potentially creating temporary arrays in vertex assignment * *_crit -> *_face
…ations (inducer#458, part 2) * optimize vertex matching in periodicity processing * use np.median instead of approximating with np.histogram turns out to be just as fast * combine _match_vertices and mesh.tools.find_point_permutation into find_point_to_point_mapping * add tests for find_point_to_point_mapping * use find_point_to_point_mapping instead of find_point_permutation
majosm
force-pushed
the
optimize-box-mesh-gen
branch
from
b16df22 to
9f9016f
Compare
|
Rebased and mirgecom CI passed, should be good to go @inducer. |
* optimize generate_box_mesh * optimize boundary processing loop * fix type annotation * parameterize dtypes * simplify definition of a0/a1 for 1D * use np.column_stack() instead of np.array([...]).T * avoid potentially creating temporary arrays in vertex assignment * *_crit -> *_face
|
Awesome, thanks! In it goes. |
3 participants
Reimplements
generate_box_meshusing numpy, and rearranges some of the boundary processing code to make it faster.Before (shock1d):
After:
Fixes #456.
To-do:
find_point_permutationrefactorfind_point_permutationrefactor