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

Added diagnostic check for watertight nodesets #29068

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

DanielYankura
Copy link
Contributor

-Loops through all elements and sides
-If side is external it checks if all its nodes are assigned to a nodeset

closes #28822

Reason

Adds new diagnostic tool for users

Design

If feature it turned on, it loops through each element, then the element's sides. If the sides are external it checks each node to ensure it's been assigned to a nodeset. If not, the node id is printed for reference.

Impact

Adds watertight nodeset check to Mesh Diagnostics

params.addParam<MooseEnum>(
"check_for_watertight_nodesets",
chk_option,
"whether to check for external nodes that are not assigned to any nodeset");
params.addParam<MooseEnum>(
Copy link
Contributor

Choose a reason for hiding this comment

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

let s add a parameter for the nodesets that are supposed to englobe the entire mesh. when this parameter is in use, the nodes are only checked against those "blessed" nodesets

same for sidsets
actually now that I see that this is not an option for sidesets, let s keep it for future work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'll open a new issue for adding that parameter to the sideset and nodeset checks.

mooseError("The nodeset check only works for 2D and 3D meshes");
auto & boundary_info = mesh->get_boundary_info();
boundary_info.build_side_list();
const auto nodeset_map = boundary_info.get_nodeset_map();
Copy link
Contributor

Choose a reason for hiding this comment

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

where is the _boundary_node_id being initialized in libmesh? is the previous call doing it?

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 believe that gets initialized when the mesh is first read in or made?

Copy link
Contributor

Choose a reason for hiding this comment

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

let s be sure we dont need to "prepare_for_use" for this diagnostics to work

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 I looked into it. The node ids get added to _boundary_node_id after execute_mesh_generators is called in ActionWarehouse.C. From what I can tell the mesh is always initialized before this test could ever be called.

Copy link
Contributor

Choose a reason for hiding this comment

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

after execute_mesh_generators

wouldnt that be too late then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it's the execute_mesh_generators task that initializes _boundary_node_id. After initialization mesh diagnostics runs. So that task runs both I believe, but it should initialize first.

Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

looks good


for (const auto elem : mesh->active_element_ptr_range())
{
for (auto i : elem->side_index_range())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (auto i : elem->side_index_range())
for (const auto i : elem->side_index_range())

// Check if side is external
if (elem->neighbor_ptr(i) == nullptr)
{
// Side is externa, now check nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Side is externa, now check nodes
// Side is external, now check nodes

if (num_nodes_without_nodeset < _num_outputs)
{
message = "Node " + std::to_string(node->id()) +
" is external, but has not been assigned to a nodeset";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
" is external, but has not been assigned to a nodeset";
" is on an external boundary of the mesh, but has not been assigned to a nodeset";

cli_args = '--mesh-only'
mesh_mode = 'replicated'
expect_out = 'Number of external nodes that have not been assigned to a nodeset: 1'
detail = '2D meshed square missing a nodeset on one side'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
detail = '2D meshed square missing a nodeset on one side'
detail = '2D meshed square with one external node that does not belong to a nodeset'

cli_args = '--mesh-only'
mesh_mode = 'replicated'
expect_out = 'Number of external nodes that have not been assigned to a nodeset: 1'
detail = '3D meshed cube missing a nodeset on one side'
Copy link
Contributor

Choose a reason for hiding this comment

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

same

@moosebuild
Copy link
Contributor

moosebuild commented Nov 13, 2024

Job Documentation, step Docs: sync website on 6004474 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Nov 13, 2024

Job Coverage, step Generate coverage on 6004474 wanted to post the following:

Framework coverage

61ed70 #29068 600447
Total Total +/- New
Rate 85.13% 85.13% +0.00% 97.56%
Hits 107285 107322 +37 40
Misses 18743 18743 - 1

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud GiudGiud self-assigned this Nov 13, 2024
@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on 0c3d1cd wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/29068/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 7377410027ba9dbf48902235a0437fdf96485e29

{
// This node does not have a nodeset!!!
num_nodes_without_nodeset++;
checked_nodes.insert(*node);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is deprecated in libmesh (this copy-construction I think?)

you should store the ids. that s enough to distinguish nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I was trying to figure out why it was failing that test. I think I have the same issue in #28701 so I'll fix it in that one as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didnt remember that one was still open. Do you need anything else from me for that? Are we waiting on Roy's review?

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 think I'm just waiting on Roy's review. Though if this one get's merged first I might have to make some changes to the other before it can be merged.

-Loops through all elements and sides
-If side is external it checks if all its nodes are assigned to a nodeset

closes idaholab#28822
-Fixed spelling mistake
-made iterator const
-improved error messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add diagnostic check for water-tight nodesets
3 participants