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

A mesh generator that checks if sidesets enclose blocks #20623

Closed

Conversation

snschune
Copy link
Contributor

Reason

Capability to check if set of sidesets encloses set of blocks.

closes #20621

Design

Mesh generator that takes a list of blocks and boundaris. Optionally a new sideset name
can be provided, if not provided, an error is thrown if an outside bpundary of the set of blocks
is not covered by one of the sidesets.

Impact

new object, none expected

@snschune snschune requested a review from andrsd as a code owner March 23, 2022 05:01
@snschune snschune force-pushed the sideset_enclosed_blocks_20621 branch from 5854ed9 to 03c859a Compare March 23, 2022 05:06
@moosebuild
Copy link
Contributor

Job Precheck on 03c859a wanted to post the following:

The following file(s) are changed:

conda/libmesh/meta.yaml

The libmesh conda configuration has changed; ping @idaholab/moose-dev-ops

@snschune snschune force-pushed the sideset_enclosed_blocks_20621 branch from 03c859a to 81b79e3 Compare March 23, 2022 13:43
@moosebuild
Copy link
Contributor

moosebuild commented Mar 23, 2022

Job Documentation on fc30f41 wanted to post the following:

View the site here

This comment will be updated on new commits.

@snschune snschune force-pushed the sideset_enclosed_blocks_20621 branch from 74b3678 to fc30f41 Compare March 23, 2022 22:57
const Elem * neigh = elem->neighbor_ptr(j);

// is this an outside boundary to blocks?
// NOTE: the next line is NOT sufficient for AMR!!
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can I have some help from maybe @lindsayad. I don't think the check I am doing will work if we have hanging nodes. How do I generalize this correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you trying to support the case where you have subdomains that differ on different children of the same parent element? libMesh supports that case, but unfortunately since we don't yet support sidesets that aren't on sides of parent elements, you won't yet be able to support that case here. For now it probably makes sense to just worry about level-0.

I'd worry more about distributed mesh support, personally. For the sides in between ghost elements and remote elements on any particular processor's part of the mesh, you don't have enough information to tell whether you need a new boundary id there or not without communication. The solutions to this are "exit with an error if the mesh is distributed", "throw in a MeshSerializer to temporarily un-distribute it" or "write that communication routine", ranked from least to most by both difficulty and usefulness.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My concern was this case

Screen Shot 2022-03-30 at 12 29 43 PM

Both elements are in the same block. But between elem 2 is refined. The side is checked
with this line of code:

bool is_outer_bnd = !neigh || blk_ids.find(neigh->subdomain_id()) == blk_ids.end();

My fear was that somehow neigh == nullptr in that case. I now think that neigh is not
nullptr but that it is simply not an active element. Is that correct @roystgnr?

I think that is different than what you thought I
wanted. Is that the case you described:
Screen Shot 2022-03-30 at 12 35 23 PM
i.e., a single parent element has children with different blocks. That is too exotic of a case for me to care about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually if I think about it. What subdomain is returned for this case:
Screen Shot 2022-03-30 at 12 38 08 PM
I am in elem 1 and for the indicated side I get the neighbor pointer. Then I get the subdomain id. What is that subdomain id? (B1, B2, B3)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd worry more about distributed mesh support, personally. For the sides in between ghost elements and remote elements on any particular processor's part of the mesh, you don't have enough information to tell whether you need a new boundary id there or not without communication. The solutions to this are "exit with an error if the mesh is distributed", "throw in a MeshSerializer to temporarily un-distribute it" or "write that communication routine", ranked from least to most by both difficulty and usefulness.

I am not sure I understand why that is. I don't think I ever access sides of ghosted elements because my loops are only over active local elems.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the first case, where I'll assume you got four children by refining Elem 2 and you haven't edited any of their subdomain ids since, I think you're safe: Elem 2 still has the same subdomain as all it's children, and Elem 2 is still the neighbor of Elem 1, it's just that elem2->active() would now be false for it.

In the second case? You're not at all safe: for all you know, you might get B4 for the subdomain id! Elem::subdomain_id() just returns the subdomain this element, regardless of whether it's active or not, and regardless of what ids its descendants might have. If, for instance, elem2 is on block 4, and you refine it and set its children to live on blocks 1,2, and 3, then elem2 is still on block 4! You can get its set of active descendants (or its set of active descendants on the side neighboring elem1; that's a natural enough task that we have an API for it) and check each of their subdomain_id() values individually, but we won't do anything to try to keep the parent in sync here; in cases like this where you give different subdomains to different children it's impossible to keep the parent in sync with all of them.

Comment on lines +96 to +100
for (auto & block_id : blk_ids)
{
for (const Elem * elem : as_range(mesh->active_local_subdomain_elements_begin(block_id),
mesh->active_local_subdomain_elements_end(block_id)))
{
Copy link
Member

Choose a reason for hiding this comment

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

I think I would do something like the following, but I would have @roystgnr confirm

Suggested change
for (auto & block_id : blk_ids)
{
for (const Elem * elem : as_range(mesh->active_local_subdomain_elements_begin(block_id),
mesh->active_local_subdomain_elements_end(block_id)))
{
for (const Elem * elem : as_range(mesh->local_level_elements_begin(0),
mesh->local_level_elements_end(0)))
{
if (!blk_ids.count(elem->subdomain_id()))
continue;

I would only bother looping over level 0 because in fact we error if you attempt to add sideset info for a non-level 0 element

void BoundaryInfo::add_side(const Elem * elem,
                            const unsigned short int side,
                            const boundary_id_type id)
{
  libmesh_assert(elem);

  // Only add BCs for level-0 elements.
  libmesh_assert_equal_to (elem->level(), 0);

Copy link
Contributor

Choose a reason for hiding this comment

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

"You can't add sideset info for a non-level-0 element" is more of a missing feature than an API design. We're hoping to fix that in the near future, once @fdkong or I have time to look at libMesh/libmesh#3094 again.

"If you add boundary info to a parent element, that automatically associates all its descendant elements sharing that side with that boundary id", on the other hand, is an API design we'll be leaving unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So say if I hit an element that was refined by AMR and I try to add one of its sides to a sideset, and I run in devel or dbg, then I would hit is assert ( libmesh_assert_equal_to (elem->level(), 0);)?

In that case, the code cannot safely work for AMR and potentially not for distributed mesh either. I know how to check for distributed mesh and error out. For AMR, can I check if the active element is the ultimate parent or something like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... checking if an active element is level-0 (the ultimate parent) isn't a good AMR check (a uniformly refined mesh will have active elements at higher levels), but it might be a good check for your situation here, for now: when you're looping over active elements just error out if you're not at level 0. Even on that hypothetical uniformly refined mesh, you won't currently be allowed to add boundary ids to child sides.

If we didn't have libMesh/libmesh#3094 waiting in the wings I'd say it would be worth a little effort to try to handle refined meshes where child elements do all belong to their parents' subdomains, but instead the best thing for you to do might be to just handle the unrefined mesh case for now but tag yourself on that PR so we don't forget to notify you when handling the refined case too becomes an option.

@GiudGiud GiudGiud self-assigned this May 15, 2022
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity in the last 100 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale PRs that have reached or exceeded 90 days with no activity label Aug 24, 2022
@snschune
Copy link
Contributor Author

keep open

@github-actions github-actions bot removed the stale PRs that have reached or exceeded 90 days with no activity label Aug 30, 2022
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

This pull request has been automatically marked as stale because it has not had recent activity in the last 100 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale PRs that have reached or exceeded 90 days with no activity label Dec 9, 2022
@GiudGiud
Copy link
Contributor

GiudGiud commented Dec 9, 2022

I've submitted this to various intern programs, see if something comes out

@github-actions github-actions bot removed the stale PRs that have reached or exceeded 90 days with no activity label Dec 10, 2022
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity in the last 100 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale PRs that have reached or exceeded 90 days with no activity label Mar 21, 2023
@github-actions github-actions bot closed this Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PRs that have reached or exceeded 90 days with no activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mesh generator that checks/adds sidesets around a set of blocks
6 participants