libmesh
libmesh copied to clipboard
Allow boundary be associated with child elements
Use case: moving boundary with AMR. Boundary needs to be defined on child elements
@dewenyushu Do you want to try if this works for you?
@dewenyushu Do you want to try if this works for you?
Cool! Let me try with my use case and get back to you
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 ()
?
@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?
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
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
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).
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.
Finally, we made everything work for our use case. It is time to check why I broke the whole world.
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 |
Warnings
- New new line coverage rate 58.06% is less than the suggested 90.0%
This comment will be updated on new commits.
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.
@roystgnr Could you please take a review on this?
Job Coverage on 0a41e91 : invalidated by @fdkong
I am curious about what elements are in BoundaryInfo
, top elements or active elements or both?
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.