Allow boundary be associated with child elements

[fdkong]

Use case: moving boundary with AMR. Boundary needs to be defined on child elements

[fdkong]

@dewenyushu Do you want to try if this works for you?

[dewenyushu]

@dewenyushu Do you want to try if this works for you?

Cool! Let me try with my use case and get back to you

[dewenyushu]

The code change makes it possible to refine and coarsen with moving elements (no error anymore, yay)!

However, (maybe not related to this PR though), it does not seem to make sense to coarsen when a child element has different subdomain ID as its parent element, does it? Shall we add a check on this while doing coarsening, possibly in MeshRefinement::refine_and_coarsen_elements ()?

[jwpeterson]

@fdkong Have you looked into why this fails all the tests? It seems to be related to periodic boundary neighbors:

bcs/dmg_periodic.check: *** ERROR ***
bcs/dmg_periodic.check: Periodic boundary neighbor not found
bcs/dmg_periodic.check:

Overall this seems like a fairly big conceptual change IMO, we have always assumed that no child elements are stored in the BoundaryInfo and that they inherit their Boundary id from the parent. Can you or @dewenyushu give me some more information on what the use case is here?

[fdkong]

However, (maybe not related to this PR though), it does not seem to make sense to coarsen when a child element has different subdomain ID as its parent element, does it? Shall we add a check on this while doing coarsening, possibly in MeshRefinement::refine_and_coarsen_elements ()?

For "normal" simulation, parent and children stay on the same subdomain.

For use case, it is so different from other cases. Let us rethink (brainstorm) a bit more on our use case before we push libmesh in that direction.

It could be a rabbit hole ..... Fixed one issue, created ten new issues, hahahaha

[fdkong]

Have you looked into why this fails all the tests? It seems to be related to periodic boundary neighbors:

Not yet, at this point, we are still experimenting whether or not the idea works for our use case

[roystgnr]

Can you or @dewenyushu give me some more information on what the use case is here?

Apologies; I discussed this with Fande over Slack (or at least I belittled his suggestions until we hit on something that was workable) (or at least something that should have been workable - not sure what's causing the test failure here) but didn't think to bring you into the loop already.

They're doing some transient domain stuff - additive manufacturing, IIRC? - and want to be able to approximate a changing domain boundary via adaptive refinement, the same way we can with changing subdomain coverage. But whereas we can give every descendant element a unique subdomain id, right now descendants have to share the boundary id of their top-level ancestor on its sides, and can't have any internal boundary sides that aren't on its sides.

I thought this "runtime-auto-set boolean to determine whether we need to search for non-top-parents too" idea was a clever way to do that without adding much cost (there's a bunch of other if tests under the hood in a multimap search, adding only one more in the common case seemed justifiable; I'd imagined iterating through parents rather than this searched_elem_vec strategy).

[roystgnr]

We need changes in BoundaryInfo::raw_boundary_ids, and also for the nullptr-neighbor case in BoundaryInfo::boundary_ids, no? Not sure how missing those could break an existing test, but I can't picture us passing new tests (which this PR also needs) without fixes there.

[fdkong]

Finally, we made everything work for our use case. It is time to check why I broke the whole world.

[moosebuild]

Job Coverage on 4be946d wanted to post the following:

Coverage

	581d66	#3094 4be946
	Total	Total	+/-	New
Rate	55.71%	55.71%	-0.00%	58.06%
Hits	44387	44398	+11	18
Misses	35288	35297	+9	13

Diff coverage report

Full coverage report

Warnings

• New new line coverage rate 58.06% is less than the suggested 90.0%

This comment will be updated on new commits.

[jwpeterson]

This PR has been marked "do not merge" since we are no longer accepting PRs into the master branch. All new PRs should be made on the devel branch instead. Once this PR's target branch has been updated to devel, the "do not merge" label will be removed.

[fdkong]

@roystgnr Could you please take a review on this?

[moosebuild]

Job Coverage on 0a41e91 : invalidated by @fdkong

[roystgnr]

Does _children_on_boundary get set by add_edge yet?

[fdkong]

No.

[fdkong]

BTW, what is difference between add_edge and add_side?

[roystgnr]

In 2D nobody uses add_edge. In 3D, add_edge associates a boundary id with just one 1D edge of a polyhedron, not a whole 2D side.

[roystgnr]

Likewise in add_shellface? Not saying you need to add support for child boundary edges or child boundary shellfaces, but if you don't add it then we can probably leave the asserts for them as strict as they were before.

[fdkong]

Good point

[roystgnr]

It's not enough to just test elem and elem->top_parent(), is it? Suppose we add a boundary side to a child element, then refine that element. The grandchildren elements on that side should still have that boundary id, shouldn't they? We'll want to loop upward parent by parent in the _chidren_on_boundary case.

[fdkong]

For my use case, it is enough. Top parent is assigned a boundary when reading in a mesh. Once the boundary is moving, boundaries will be on active elements. We literally delete everything else.

Should we allow users to leave boundary everywhere? I can make that change

[fdkong]

I feel like if the current element has a boundary, and then parents should not have the same boundary ID. Otherwise which one we should choose for users?

So do we need to do some checks?

[roystgnr]

One person's "Works for me" is another person's "Booby trap". Refining an element gives you children who have the same boundary ids on the sides they share with their parent; for that to suddenly fail is not a bug libMesh currently has and we're not about to introduce it now.

I feel like if the current element has a boundary, and then parents should not have the same boundary ID. Otherwise which one we should choose for users?

If they're all the same then there's no choice; we return that id. If they differ, then we return true if we match any of them (in the has_boundary_id test case) or we add them all to the vector we're filling (in the boundary_ids test case). Say element p has boundary id b1 on a side. We refine it, and on the shared side child c now also correctly has boundary id b1. What should happen when we add b2 to that side of c? That shouldn't imply removing b1. When querying for a complete set of boundary ids we'll need to remember to query p (and on up the tree) even if we find one or more ids assigned to lower levels.

So do we need to do some checks?

I think the one place we're missing here where it gets a little interesting is coarsening.

In general, when we coarsen an element, the boundary data attached to the now-vanishing children needs to disappear, not to become dangling pointers.

When we try to coarsen the mesh down to a parent element which has had the same boundary id added to that side of all its children. If the id hasn't been explicitly added to the parent, it won't be on the parent. But if that side was entirely on the boundary before coarsening it should be on entirely on the boundary after! In that case we should be adding the id to the parent as we remove it from the children.

And the only place that gets really interesting: what happens when some of the children have an id and others do not? I could see an argument for "the parent then gets the id from any of its children" or for "the parent doesn't get the id" or for "the parent gets the id if a majority (rounding up? down?) of its children have it", and that's all fine because when we're coarsening we expect our data to be getting coarser, but whatever we do we ought to document the hell out of it and add a few unit tests.

[roystgnr]

Well, wait, I could see one more interesting question: if we do want to enforce "a parent side gets ids that were given to one/most of the children sides it contains" (we may) or "gets ids that were given to all of the children sides" (we do?), do we want to add that id to the parent preemptively, when it's added to (enough/all of?) the children, before coarsening? That might be useful for users to be able to query directly without ever having to manually set ids on non-active elements. But then on the other hand if we do that then we need to do the same for removal of ids, for consistency, and that very consistency would a source of inflexibility for users who might prefer a different behavior than whatever we choose.

[dewenyushu]

Hi @roystgnr , thanks for your review! I am trying to take over the PR from @fdkong so that this effort does not go down in the drain. However I am super confused... What is the agreement that you guys arrive at? - Revert the changes here and add a documentation about not returning descendants' boundary ids from this function? Thanks :)

[roystgnr]

IIRC the code functionalities we absolutely need to add are "search all ancestors when filling out children's boundary ids" and "transfer an id up to an ancestor when coarsening away children that all have that id on that ancestor's side". The bit we need to decide upon is what to do when coarsening away children only some of whom have an id on a particular side, and for that I had three suggestions, but I'd even be content with the laziest of the three, "the id gets lost there", so long as it's documented well.

[dewenyushu]

Thanks @roystgnr ! Please correct me if I am wrong:

search all ancestors when filling out children's boundary ids

so this is to avoid the case of not returning the element side if any of its "intermediate" ancestors have the boundary ID while its top parent does not?

transfer an id up to an ancestor when coarsening away children that all have that id on that ancestor's side

guess this should be done in the function BoundaryInfo::add_sides, not in this function?

what to do when coarsening away children only some of whom have an id on a particular side, and for that I had three suggestions, but I'd even be content with the laziest of the three, "the id gets lost there", so long as it's documented well.

not very sure where and how to keep track of (or throw an exception/warning..?) about children elements of different IDs being coarsened away... would you mind pointing me to the function where this should be added?

[roystgnr]

to avoid the case of not returning the element side if any of its "intermediate" ancestors have the boundary ID while its top parent does not?

Exactly.

guess this should be done in the function BoundaryInfo::add_sides, not in this function?

Hmm... actually, I'd say it's not safe to do it until the actual coarsening happens. If we try to do it any earlier, then something as simple as adding a boundary id and removing it again could see the removal fail, because that addition was the last child sharing a parent side and we "helpfully" added the id to the parent. Or ... we could also propagate removals up to ancestors? That would make sense too: when the last child sharing a side has an id added there, we add it to the parent side; when any child sharing a side has an id removed we remove it from the parent (and so recursively from any other ancestors that have it)? That gets really tricky, though. The desired behavior is easy enough to describe, but the implementation details...

not very sure where and how to keep track of (or throw an exception/warning..?) about children elements of different IDs being coarsened away

We don't throw warnings or exceptions about losing information when coarsening; that's kind of implied by the concept of coarsening. It just needs to be in the BoundaryInfo documentation somewhere.

would you mind pointing me to the function where this should be added?

MeshRefinement::coarsen_elements() could handle transferring boundary ids to newly-reactivated coarse parents, maybe?

[dewenyushu]

Cool! Really appreciate your suggestions here! Let me look into this a bit more and I may (will) bother you again with some questions about the implementation details.

[YaqiWang]

I am curious about what elements are in BoundaryInfo, top elements or active elements or both?

[roystgnr]

In libMesh master only top elements can be in BoundaryInfo, and other elements inherit their top-parents' info.

After this PR we'll still have elements inheriting ancestors' info, but that will be overrideable with the ability to add child elements to BoundaryInfo too.