VariationalMeshSmoother: Support for higher order elements #4218

pbehne · 2025-07-28T21:20:48Z

This pull request extends the VariationalMeshSmoother to handle higher-order elements, as part of the work on issue #4082.

Improved Test Coverage:
- Updated the VariationalMeshSmoother tester to better handle symmetric configurations. Nodes previously unaffected by the DistortHypercube due to lying on planes of symmetry now receive a 5% forced distortion.
- The above forced distortion removed the need to skip distortion checks on nodes with coordinate value 0.5, simplifying the distortion_is logic.
Extended Element Support:
- EDGE3 and HEX20: Added tests verifying smoother functionality on these higher-order elements. Minor adjustments to distortion_is were made to correctly map midpoint nodes to integer values using fe_order.
- TRI6: Introduced a target equilateral quadratic triangle to support smoothing for TRI6 elements, along with corresponding tests.
- HEX27: Implemented logic to determine face neighbors for face-center nodes when no nodal neighbors are available, enabling full support and test coverage for HEX27 elements.

Ref #4082.

Previously, some nodes lay on a plane of symmetry and were not distorted by the DistortHypercube class. A check has been added for this and if a node will not be distorted due to symmetry, a 5% distortion is forced. Because of this, we no longer have to skip checks in distortion_is on nodes with a coordinate value of 0.5.

Smoothing for these higher-order elements worked out-of-the box and only required a small tweak to the `distortion_is` function to map midpoint nodes to integer values. This also required tightening the tolerance in the constraint variants.

This required adding a target equilateral quadratic triangle to the VariationalSmootherSystem for TRI6 elements.

This involved implementing a utility function to return face neighbors when no nodal neighbors are found for a face's central node. Ref libMesh#4082

roystgnr · 2025-07-28T22:43:25Z

src/systems/variational_smoother_constraint.C

+      // It doesn't make sense for a node having no edge neighbors to belong to
+      // multiple elements, right? Otherwise, we will figure out those cases as
+      // they occur.


Won't any face-centered node on the "boundary" between two subdomains belong to two elements?

But you've already instantiated the HEX27 tests!? I'd have expected that to be the iconic example of the problem.

Yeah, so these high order hex tests take a bit to run in dbg mode, so I always ran them in opt and didn't see the assertion hit in dbg until I put this PR up. I don't know what I was thinking when I wrote the assertion. I took it out.

How'd we pass tests in CI? I could have sworn the 32-bit recipe was running with unit tests enabled and in dbg mode.

src/systems/variational_smoother_system.C

moosebuild · 2025-07-29T04:07:24Z

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

Coverage

	d1acc0	#4218 ead09f
	Total	Total	+/-	New
Rate	64.45%	64.54%	+0.08%	97.37%
Hits	75713	75828	+115	74
Misses	41759	41671	-88	2

Diff coverage report

Full coverage report

This comment will be updated on new commits.

roystgnr

This looks fine to me just reading the code, but before we merge:

I want to give @jwpeterson a day to look on it or pass on looking on it

I want to kick those Fetch failures and let CI finish

I want to figure out why exactly we weren't tripping that bogus assert before! Even if the library code is right, maybe there's something screwed up in the test coverage?

pbehne · 2025-07-29T14:52:50Z

I want to figure out why exactly we weren't tripping that bogus assert before! Even if the library code is right, maybe there's something screwed up in the test coverage?

Is this something you want to check, or want me to look into it?

roystgnr · 2025-07-29T20:28:10Z

Is this something you want to check, or want me to look into it?

Between errands this morning and tomorrow I'm running stupidly behind, so ... wait a minute. Emphasis on "stupidly". That commit was hitting that assertion failure left and right! E.g. https://civet.inl.gov/job/3073780/

I could swear I saw it pass but maybe that was only after the assertion-removal commit.

Okay, we'll merge tomorrow morning unless John screams.

jwpeterson · 2025-07-30T14:31:03Z

src/systems/variational_smoother_system.C

@@ -195,7 +195,7 @@ void VariationalSmootherSystem::prepare_for_smoothing()
                  // Scale the nodal positions to conserve area
                  auto node_ptr = std::make_unique<Node>(
                      equilateral_points[node_id] * side_length, node_id);
-                  target_elem->set_node(node_id) = node_ptr.get();
+                  target_elem->set_node(node_id, node_ptr.get());


I think I'll make a separate PR with just this fix as well, it shouldn't conflict with #4218 and it will give us a working --disable-deprecated build before #4218 just in case that is useful for debugging for some reason...

…_tri6 Conflicts: src/systems/variational_smoother_system.C

jwpeterson · 2025-07-30T15:44:13Z

I fixed a conflict with devel (that I introduced, sorry!) and updated the branch. I also haven't reviewed this yet, but I'll do so now.

jwpeterson · 2025-07-30T16:21:57Z

src/systems/variational_smoother_constraint.C

@@ -521,6 +521,47 @@ void VariationalSmootherConstraint::constrain_node_to_line(const Node & node,
    }
 }

+void VariationalSmootherConstraint::find_nodal_or_face_neighbors(
+    const MeshBase &mesh, const Node &node,


Sorry to continue harping on this, but we put spaces both before and after the & and *, so here it would be:

const MeshBase & mesh, const Node & node,

In addition we usually do a hanging indent when there are multiple lines of parameters rather than indenting a fixed four spaces.

I think the clang-format checked in to the repo handles this automatically (?), but I've never used it.

jwpeterson · 2025-07-30T16:24:06Z

src/systems/variational_smoother_constraint.C

+  if (!neighbors.size())
+    {
+      // Grab the element containing node
+      const auto *elem = nodes_to_elem_map.at(node.id()).front();


Always use libmesh_map_find() instead of map::at(). The former gives a nicely formatted error message in the exception when the key is not found, the latter just throws a generic std::exception.

jwpeterson · 2025-07-30T16:29:09Z

src/systems/variational_smoother_constraint.C

+          for (const auto &local_node_id : nodes_on_side)
+            if (elem->node_id(local_node_id) == node.id())
+            {
+              found_node = true;
+              break;
+            }


I'd refactor this into a std::find_if(nodes_on_side.begin(), nodes_on_side.end(), [&](auto local_node_id){...}); call rather than hand-coding the linear search.

jwpeterson · 2025-07-30T16:33:07Z

src/systems/variational_smoother_system.C

+                  auto node_ptr = std::make_unique<Node>(
+                      equilateral_points[node_id] * side_length, node_id);


Node::build() can be used here, and I'd emplace_back() the new Node directly into owned_nodes, catching the reference returned by that to then pass to target_elem->set_node(...). This just saves the step of defining the node_ptr temporary variable.

jwpeterson · 2025-07-30T16:35:31Z

src/systems/variational_smoother_system.C

+        }// if find == end()
+
+      // Reference volume computation
+      Real elem_integrated_det_J(0.);


Minor comment but I don't think we use this type of initialization for Reals elsewhere (we typically use boring Real foo = 0.;) so it looks a bit out of place IMO.

jwpeterson · 2025-07-30T16:37:03Z

tests/mesh/mesh_smoother_test.C

@@ -101,12 +103,12 @@ private:
                    modulation *= (pj - 0.5) * (pj - 0.5) * 4.; // quadratic bump centered at 0.5
                  }
              }
-            output(i) = p(i) + (std::pow(xi, 3) - xi) * modulation;
+            const auto delta = (std::pow(xi, 3) - xi) * modulation;


Please use libMesh::Utility::pow<N> for all integer powers. I know this isn't a new line of code in this PR, but I forgot to mention it on the earlier one. It is just a template function that compiles down to pure multiplication/no function calls at runtime.

jwpeterson · 2025-07-30T16:42:22Z

tests/mesh/mesh_smoother_test.C

+    // Get mesh dimension, determine whether type is triangular
+    const auto * ref_elem = &(ReferenceElem::get(type));
+    const auto dim = ref_elem->dim();
+    const bool type_is_tri = dynamic_cast<const Tri*>(ref_elem);


Not necessary for this PR, but this feels so clunky that we should probably add a helper function that does this by looking only at an ElemType enum. Not sure what the best place would be, but maybe a helper function like enum_to_string() would probably make sense. It would need to be updated as new Tri types are added, but we have a few things like that already so I wouldn't mind one more.

Created #4220 for this.

…libMesh#4218.

pbehne added 6 commits July 28, 2025 15:04

Added tests for EDGE3, HEX20 elements

261a0fa

Smoothing for these higher-order elements worked out-of-the box and only required a small tweak to the `distortion_is` function to map midpoint nodes to integer values. This also required tightening the tolerance in the constraint variants.

Added support and tests for TRI6 elements

39fcac2

This required adding a target equilateral quadratic triangle to the VariationalSmootherSystem for TRI6 elements.

Added support and tests for HEX27 elements.

b354456

This involved implementing a utility function to return face neighbors when no nodal neighbors are found for a face's central node. Ref libMesh#4082

Delete trailing whitespace

039f15c

Removed assertion I wrote when I was stupid.

a00f377

roystgnr reviewed Jul 28, 2025

View reviewed changes

roystgnr requested changes Jul 28, 2025

View reviewed changes

src/systems/variational_smoother_system.C Outdated Show resolved Hide resolved

src/systems/variational_smoother_system.C Outdated Show resolved Hide resolved

pbehne added 2 commits July 29, 2025 07:50

Replaced dynamic cast with compile-time conditional.

7a718fb

Consolidated code by using Elem::build.

84ec3c0

roystgnr approved these changes Jul 29, 2025

View reviewed changes

Made ref_area const.

f74447f

pbehne mentioned this pull request Jul 30, 2025

Added sliding boundary nodes to VariationalMeshSmoother #4216

Merged

Use non-deprecated set_node.

e6bc76c

jwpeterson reviewed Jul 30, 2025

View reviewed changes

jwpeterson mentioned this pull request Jul 30, 2025

Fix deprecated Elem::set_node() call #4219

Merged

Merge remote-tracking branch 'origin/devel' into variational_smoother…

7345f23

…_tri6 Conflicts: src/systems/variational_smoother_system.C

jwpeterson requested changes Jul 30, 2025

View reviewed changes

pbehne added 2 commits July 30, 2025 11:35

Addressed John's review.

8d5a268

Changed instance of std::pow to Utility::pow.

ead09fc

jwpeterson approved these changes Jul 30, 2025

View reviewed changes

roystgnr approved these changes Jul 31, 2025

View reviewed changes

roystgnr merged commit 231f9e6 into libMesh:devel Jul 31, 2025
20 checks passed

pbehne added a commit to pbehne/libmesh that referenced this pull request Jul 31, 2025

Refactored element type check to use string comparison as suggested in …

4d5ad98

…libMesh#4218.

pbehne mentioned this pull request Jul 31, 2025

VariationalMeshSmoother: Refactored element type check to use string comparison #4220

Merged

pbehne deleted the variational_smoother_tri6 branch July 31, 2025 22:07

		auto node_ptr = std::make_unique<Node>(
		equilateral_points[node_id] * side_length, node_id);

VariationalMeshSmoother: Support for higher order elements #4218

VariationalMeshSmoother: Support for higher order elements #4218

Uh oh!

Conversation

pbehne commented Jul 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

moosebuild commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage

Uh oh!

roystgnr left a comment

Choose a reason for hiding this comment

Uh oh!

pbehne commented Jul 29, 2025

Uh oh!

roystgnr commented Jul 29, 2025

Uh oh!

jwpeterson Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwpeterson commented Jul 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

moosebuild commented Jul 29, 2025 •

edited

Loading

jwpeterson Jul 30, 2025 •

edited

Loading