Skip to content
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

Overhaul facial adjacency + boundary tagging #252

Merged
merged 24 commits into from
Sep 22, 2021

Conversation

majosm
Copy link
Collaborator

@majosm majosm commented Jul 27, 2021

This PR:

  1. Restructures Mesh's facial adjacency structure to allow multiple adjacency groups for each pair of mesh element groups. In other words, List[Dict[FacialAdjacencyGroup]] becomes List[List[FacialAdjacencyGroup]].
  2. Splits FacialAdjacencyGroup into InteriorAdjacencyGroup and BoundaryAdjacencyGroup subclasses (with InterPartitionAdjacencyGroup inheriting from BoundaryAdjacencyGroup).
  3. Splits boundary adjacency for different boundary tags into different groups.
  4. Splits inter-partition adjacency for different ranks into different groups.
  5. Removes boundary_tags/boundary_tag_bit from Mesh.
  6. Adds affine transformations to InteriorAdjacencyGroup/InterPartitionAdjacencyGroup.

This will almost certainly need to be split into multiple PRs, but I wanted to throw this up here first for CI + discussion.

(Probably incomprehensible unless read commit-by-commit.)

not fully implemented for partition_mesh; need to be able to create
multiple nonlocal adjacency groups with distinct transformations
changed from a list of maps to a list of lists to allow multiple
adjacency groups per mesh group pair
splits single InterPartitionAdjacencyGroup into one per remote
neighbor group for each local mesh group
@majosm
Copy link
Collaborator Author

majosm commented Jul 27, 2021

(Looks like I need to do a little more work on the BTAG_NONE/BTAG_ALL/BTAG_REALLY_ALL handling. Stay tuned. 🙂)

@majosm majosm force-pushed the facial-adj-overhaul branch 5 times, most recently from 77a0fb9 to f2b6ac9 Compare July 29, 2021 21:36
@majosm majosm marked this pull request as ready for review August 3, 2021 17:14
@majosm majosm requested a review from inducer August 3, 2021 17:15
@majosm
Copy link
Collaborator Author

majosm commented Aug 3, 2021

Just looking for a review of the high-level overview in the OP to check if this seems reasonable (though feel free to take a glance through the code if you're feeling particularly adventurous. 🙂) After that I'll split this up into smaller pieces.

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! Here's an initial review up to (but not including) 5cda667 ("remove unnecessary intermediate steps in partition_mesh")

meshmode/mesh/processing.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/processing.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
@majosm majosm force-pushed the facial-adj-overhaul branch 2 times, most recently from fdf3231 to 5732798 Compare August 31, 2021 19:20
@majosm majosm force-pushed the facial-adj-overhaul branch 4 times, most recently from 0f75406 to bdfa920 Compare September 16, 2021 21:40
@majosm
Copy link
Collaborator Author

majosm commented Sep 17, 2021

I think this is ready for another look @inducer.

meshmode/mesh/processing.py Outdated Show resolved Hide resolved
meshmode/mesh/processing.py Show resolved Hide resolved
meshmode/mesh/processing.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Show resolved Hide resolved
meshmode/mesh/__init__.py Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/tools.py Show resolved Hide resolved
Copy link
Collaborator

@thomasgibson thomasgibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments/touch-ups in places after reading through the changes. Thank you for spending the time doing all this!

meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Show resolved Hide resolved
meshmode/mesh/__init__.py Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Outdated Show resolved Hide resolved
meshmode/mesh/__init__.py Show resolved Hide resolved
meshmode/mesh/__init__.py Show resolved Hide resolved
meshmode/interop/firedrake/mesh.py Show resolved Hide resolved
meshmode/mesh/processing.py Show resolved Hide resolved
meshmode/mesh/processing.py Show resolved Hide resolved
majosm and others added 2 commits September 22, 2021 16:19
Co-authored-by: Thomas H. Gibson <gibsonthomas1120@hotmail.com>
Co-authored-by: Thomas H. Gibson <gibsonthomas1120@hotmail.com>
@inducer
Copy link
Owner

inducer commented Sep 22, 2021

Thanks for working on this, and thanks @thomasgibson for taking a look!

@inducer inducer enabled auto-merge (rebase) September 22, 2021 21:47
@inducer inducer merged commit 67e146c into inducer:main Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants