Allow boundary be associated with child elements #3470
Conversation
Use case: moving boundary with AMR. Boundary needs to be defined on child elements
The motivaiton is that: "automatic" way might not work for cases. For example, if the flag is on on a subset of processor cores, and then we do reparitioning, and then we might hit trouble because the flag is off on the other processors I try to avoid having a global reduction to have everyone on the same page. In fact, we know when we want to allow children on boundary in MOOSE. In this case, we should set the flag to make the code more robust
|
Job Coverage on b6b8849 wanted to post the following: Coverage
Warnings
This comment will be updated on new commits. |
||||||||||||||||||||||||||
dewenyushu
force-pushed
the
boundary_on_child_elements
branch
4 times, most recently
from
2668cf2 to
b930170
Compare
| this->remove_side(parent, side, bnd_id); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Here is the function to transfer boundary to the parent before coarsening
| // Make sure we transfer the element's boundary id(s) | ||
| // up to its parent when necessary before coarsening. | ||
| // This can be adding or removing the corresonding boundary info. | ||
| _mesh.get_boundary_info().transfer_boundary_ids_to_parent(elem); |
There was a problem hiding this comment.
Here is when I transfer the boundary info to the parent
|
I tried numerous times adding a unit test for this code change and it would not work.. Not sure if this is something related to the code change or related to the unit test (this is the first cpp unit test I am writing so..) The change works for my MOOSE-based application and tests so I am not sure how to debug more... @roystgnr Would you mind taking a quick look and letting me know where I made mistakes that is obvious to you? Thanks!! |
…ing, if the child element has boundary info associated with itself
dewenyushu
force-pushed
the
boundary_on_child_elements
branch
from
5c28e1b to
18a8ea1
Compare
That's good! The best unit tests are the ones that prevent regressions of bugs that have already happened. I can replicate this; looking at it today. |
|
Oh, or maybe I should have refreshed the page first. Looks like CI thinks you already fixed it. Reviewing today, then. |
Great! Really appreciate it!! - I am not familiar with unit tests so please bare with me if there are some stupid mistakes |
roystgnr
left a comment
roystgnr
left a comment
There was a problem hiding this comment.
We're getting closer, at least?
| for (auto it = searched_elem_vec.begin(); it != searched_elem_vec.end(); ++it) | ||
| { | ||
| const Elem * searched_elem = *it; |
There was a problem hiding this comment.
This could be cleaned up as above.
| searched_elem = elem->top_parent(); | ||
| searched_elem_vec.push_back(elem->top_parent()); | ||
| else if (!_children_on_boundary) | ||
| searched_elem_vec.push_back(elem); |
There was a problem hiding this comment.
I'm not even sure what's going on here. We do need to check intermediate parents in the _children_on_boundary case, not just the queried element and its top_parent.
| // If yes, we make sure that the corresponding parent side is added to the boundary. | ||
| // Otherwise, we remove the parent side from the boundary. | ||
| std::vector<const Elem *> family_on_side; | ||
| elem->family_tree_by_side(family_on_side, side); |
There was a problem hiding this comment.
This doesn't check siblings of elem, it checks descendants.
There was a problem hiding this comment.
And you shouldn't need a full family_tree method here, either, because we really do only care about siblings; if an element's really about to be coarsened then any descendants or niblings it has should be subactive and we can ignore them.
There was a problem hiding this comment.
So you can just loop over the parent's children. Note that you also have to make sure to check which children are on side; if we coarsen a 4-pack of Quad4 then we only care about preserving the ids shared by each 2 of them on each side, never all 4 at once.
| { | ||
| LOG_UNIT_TEST; | ||
|
|
||
| std::unique_ptr<Mesh> mesh; |
There was a problem hiding this comment.
Just put the Mesh on the stack here?
But that's a quibble; this looks like an excellent first unit test. Not yet sufficient, given the failure cases above, but a good start.
Co-authored-by: roystgnr <[email protected]>
|
close this one due to #3477 |
4 participants
Taking over #3094