Change tag/face mapping argument of _compute_facial_adjacency_from_vertices#352
Open
majosm wants to merge 10 commits intoinducer:mainfrom
Open
Change tag/face mapping argument of _compute_facial_adjacency_from_vertices#352majosm wants to merge 10 commits intoinducer:mainfrom
_compute_facial_adjacency_from_vertices#352majosm wants to merge 10 commits intoinducer:mainfrom
Conversation
…rtices change the structure to something that can be constructed more efficiently
Co-authored-by: Alex Fikl <alexfikl@gmail.com>
Co-authored-by: Alex Fikl <alexfikl@gmail.com>
majosm
force-pushed
the
optimize-tag-face-mapping
branch
from
8c3430a to
db96dca
Compare
majosm
force-pushed
the
optimize-tag-face-mapping
branch
from
db96dca to
65f7f01
Compare
Collaborator
Author
On my laptop, for a mesh with 1.2M elements:
|
inducer
reviewed
Oct 17, 2022
Owner
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
Some comments from an initial scroll.
| faces=np.concatenate([ids.faces for ids in face_ids_list])) | ||
|
|
||
|
|
||
| def _find_matching_index_pairs_merged(indices): |
Owner
There was a problem hiding this comment.
Describe shape of indices (and describe the axis along which the return value indexes).
| return np.stack((order[match_indices], order[match_indices+1])) | ||
|
|
||
|
|
||
| def _find_matching_index_pairs(left_indices, right_indices): |
Owner
There was a problem hiding this comment.
Describe shape of left_indices and right_indices (and describe the axis along which indiex_pairs indexes).
meshmode/mesh/__init__.py
Outdated
Comment on lines
1426
to
1427
| :class:`numpy.ndarray` of shape ``(2, nfaces)`` containing | ||
| the element and face indices of each tagged face in the group. |
Owner
There was a problem hiding this comment.
I'm guessing you mean element indices in tag_to_group_faces[0] and face indices in tag_to_group_faces[1]? If so, say so.
| """ | ||
| Return an array of dimension ``(2, nmatches)`` containing pairs of indices into | ||
| *indices* representing entries that are the same. | ||
| """ |
Owner
There was a problem hiding this comment.
Document the interface consequences of lexsort being stable that are being used below.
| """ | ||
| index_pairs = _find_matching_index_pairs_merged( | ||
| np.concatenate((left_indices, right_indices), axis=1)) | ||
| index_pairs[1, :] -= left_indices.shape[1] |
Owner
There was a problem hiding this comment.
Might it be worthwhile to assert that these are in range (i.e. not drawn into negative)?
meshmode/mesh/__init__.py
Outdated
| face_index_pairs = _find_matching_index_pairs( | ||
| tagged_elements_and_faces.T, | ||
| np.stack((bdry_elements, bdry_element_faces))) | ||
| face_indices = face_index_pairs[1, :] |
Owner
There was a problem hiding this comment.
Suggested change
| face_indices = face_index_pairs[1, :] | |
| bdry_face_indices = face_index_pairs[1, :] |
meshmode/mesh/__init__.py
Outdated
| is_tagged = np.full(len(bdry_elements), False) | ||
|
|
||
| for tag, tagged_elements_and_faces in tag_to_group_faces[igrp].items(): | ||
| face_index_pairs = _find_matching_index_pairs( |
Owner
There was a problem hiding this comment.
Suggested change
| face_index_pairs = _find_matching_index_pairs( | |
| vol_bdry_index_pairs = _find_matching_index_pairs( |
inducer
reviewed
Oct 17, 2022
| Return an array containing pairs of indices into *indices* representing entries | ||
| that are the same. | ||
|
|
||
| Given an array *indices* of shape ``(N, nindices)`` containing integer-valued |
Owner
There was a problem hiding this comment.
Suggested change
| Given an array *indices* of shape ``(N, nindices)`` containing integer-valued | |
| Given an array *indices* of shape ``(N, ntuples)`` containing integer-valued |
Comment on lines
+1358
to
+1359
| Returned matches are ordered such that the second element of the pair occurs | ||
| after the first in *indices*. |
Owner
There was a problem hiding this comment.
assert (result[0] < result[1]).all()?
(Also improve phrasing here.)
| # Indices into left_indices are the same as indices into all_indices, but need | ||
| # a conversion for right_indices | ||
| index_pairs[1, :] -= left_indices.shape[1] | ||
| assert index_pairs[1, :] >= 0 # Sanity check |
Owner
There was a problem hiding this comment.
Suggested change
| assert index_pairs[1, :] >= 0 # Sanity check | |
| assert (index_pairs[1, :] >= 0).all() # Sanity check |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Now accepts a list of dicts (one per mesh group), where each dict maps a tag to a numpy array of element/face indices that belong to that tag. This removes the need to use Python loops over elements when generating and using the tag/face mappings.