-
Notifications
You must be signed in to change notification settings - Fork 32
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
Implement general 2D forest meshes #1047
base: lroberts36/refactor-mesh-constructors
Are you sure you want to change the base?
Implement general 2D forest meshes #1047
Conversation
…6/add-forest-orientation-2
…rdinate transformation
…add-forest-orientation-2
…add-forest-orientation-2
…add-forest-orientation-2
Passes |
Agrees with old |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool. And I think I understood most of what's going on here. But this is pretty complicated and introduces a lot of new concepts to parthenon that are hard to visualize and reason about. I feel pretty strongly that this MR needs documentation associated with it.
std::unordered_map<uint64_t, std::shared_ptr<parthenon::forest::Node>> nodes; | ||
nodes[0] = parthenon::forest::Node::create(0, {0.0, 0.0}); | ||
nodes[1] = parthenon::forest::Node::create(1, {1.0, 0.0}); | ||
nodes[2] = parthenon::forest::Node::create(2, {1.0, 1.0}); | ||
nodes[3] = parthenon::forest::Node::create(3, {0.0, 1.0}); | ||
nodes[4] = parthenon::forest::Node::create(4, {2.0, 0.0}); | ||
nodes[5] = parthenon::forest::Node::create(5, {2.0, 1.0}); | ||
nodes[6] = parthenon::forest::Node::create(6, {0.0, 2.0}); | ||
nodes[7] = parthenon::forest::Node::create(7, {1.0, 2.0}); | ||
nodes[8] = parthenon::forest::Node::create(8, {2.0, 2.0}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how should I interpret this initialization? These are coordinates, but of what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be nice to have some automated machinery for common useful topologies to save users from having to manually define mesh structures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, nodes store a position in the x-y plane. This was convenient for making plots like the refinement plot shown above but the positions are unused otherwise. Probably in the future we don't actually want to associate nodes with positions since they are just there for specifying the topology of the mesh.
nx1 = 8 | ||
x1min = 0.0 | ||
x1max = 1.0 | ||
ix1_bc = outflow | ||
ox1_bc = outflow | ||
|
||
nx2 = 8 | ||
x2min = 0.0 | ||
x2max = 1.0 | ||
ix2_bc = outflow | ||
ox2_bc = outflow | ||
|
||
nx3 = 1 | ||
x3min = -0.5 | ||
x3max = 0.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a forest setting, how are nx1, nx2, nx3 interpreted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are ignored. These should probably be removed from this input deck.
<parthenon/static_refinement0> | ||
level = 1 # refinement level | ||
x1min = 0.0 # refinement region inner boundary, X1-dir | ||
x1max = 0.5 # refinement region outer boundary, X1-dir | ||
x2min = 0.0 # refinement region inner boundary, X2-dir | ||
x2max = 0.5 # refinement region outer boundary, X2-dir | ||
x3min = -1.0 # refinement region inner boundary, X3-dir | ||
x3max = 1.0 # refinement region outer boundary, X3-dir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same Q as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Static refinement of a region is not currently supported for general forests, so this will be ignored. Currently, to statically refine a general forest you just have to initially tell it the the LogicalLocation
s that you want to be refined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this (and above) would probably be good to explain. And the extraneous lines removed from the input deck.
Are you satisfied with the latex document that is linked in the PR description or do you think it needs more detail? I don't really want to write this up in Sphinx, since I don't know how to deal with tikz and equations there. Do we have a way of including latex documentation in the repo? |
I think it works well as a description of your design and implementation. I don't think it suffices as a user-level explanation of how to actually build a forest or use it. Ideally what I would like to see is a link to your latex writeup in the docs folder---and maybe the tex source under version control in the repo---as well as some user-focused references for how to actually build a tree.
There seems to be some way to do this... but a cursory skim doesn't provide a clean answer. It does support latex syntax for equations, but your doc has quite a bit of inline math. |
I think that a lot of the practical interface with forests is going to change in near future PRs. For instance, the simplest way to specify forests would be to have blocks for each face in the parameter input that contain a list of the nodes defining the face and some information about the coordinates of the face (although this would become unwieldy for forests with lots of trees). I also haven't completely figured out how to deal with singular valence points, more general coordinates, and visualization. As a result, I don't think it makes sense to write up detailed documentation of how to use forests in this PR, since it is really focused on under the hood changes (plus the minimum necessary interface to write some tests).
Also tikz needs to work for the diagrams (which I think are key to understanding what is going on). |
Fair enough. Can we at least stick the tex source file in the repo under version control and link to the pdf in the docs?
I think there's some way to just use a tex file but I couldn't figure it out from the docs... And yes tikz is life. |
Yeah, Sphinx is extremely frustrating. Stack Overflow suggests you can't include a latex file directly, but the comment is pretty old. |
ah that's frustrating |
…add-forest-orientation-2
PR Summary
This PR implements the minimum necessary technology for defining meshes based on generally connected forests of octrees. I have left details of coordinates to a future PR, so for now each tree must cover a hyper-rectangular domain defined by$\vec{x}_\text{min}$ and $\Delta \vec{x}$ . Some rough notes about my implementation are in ForestOfOctreesNotes.pdf
The four major things this PR accomplishes:
forest::Node
,forest::Edge
, andforest::Face
for defining the forest mesh topology.forest::Face
contains routines for finding neighbors and then calculating the correct logical coordinate transformations to those neighbors. For now, I have stuck to defining 2D forests but the mechanisms used here should generalize to 3D relatively easily in a future PR.forest::Forest::Make2D
that builds aForest
from aforest::ForestDefinition
.ForestDefinition
contains a set ofFaces
, boundary conditions defined onforest::Edge
s,RegionSize
s associated with each block, and a set of initialLogicalLocation
s to refine.Mesh
constructor as well asParthenonManager::ParthenonInitPackagesAndMesh
that takes aforest::ForestDefinition
and constructs a mesh based on a general 2D forest.Smaller things also included in the PR:
RelativeOrientation
->LogicalLocationCoordinateTransformation
for clarity.MeshBndryFnctn
toMesh::forest::trees
instead ofMesh
and add appropriate registration routines.gid
s and other information through boundaries to ensure that neighbor communication is occurring correctly. The gold file has just been verified by hand.Things this PR does not do (but maybe a future PR should):
MeshBlock
.Some examples of properly nested refinement near singular mesh points (this mesh configuration is tested in the newly added unit test):
Boundaries filled with neighbor gids in a 2x2 forest where the lower left tree coordinates are rotated by 90 degrees relative to the other trees (results of the boundary communication regression test):
PR Checklist