Generate second-order node maps from the linear tables by rochi00 · Pull Request #4554 · libMesh/libmesh

rochi00 · 2026-09-16T16:44:18Z

Put the second-order element side/edge topology and reference-node
coordinates into one constexpr derivation in
fe_reference_element_traits.h: a second-order side row is the linear
corners, then the edge midpoints from the edge table, then the face
centre; higher-order reference nodes are subentity centroids.

The element classes' side_nodes_map/edge_nodes_map are now generated
from that derivation at compile time instead of being 26 hand-typed
literals. Every table is bit-identical to the one it replaces and is
constant-initialized. The maps keep their names, their [side][node]
indexing and their public visibility (MOOSE ray tracing reads them
directly); only their type changes, to a ReferenceElementTable that
indexes like the old arrays.

moosebuild · 2026-09-16T18:37:29Z

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

Coverage

	54e0d5	#4554 2697ed
	Total	Total	+/-	New
Rate	65.99%	66.03%	+0.04%	100.00%
Hits	79562	79711	+149	159
Misses	40996	41006	+10	0

Diff coverage report

Full coverage report

This comment will be updated on new commits.

roystgnr

Didn't quite make it through the whole thing, but this should give you enough to chew on for a first pass. ;-)

I love this in theory, but in practice? Adding 1000 lines of code to replace under 300 is never a good sign for a refactor. I assume some of this is prerequisite for Kokkos work, giving us static lookups for data we previously had to get via virtual functions? Those cases count as genuinely new features and justify some new lines, but I'll bet we can factor this way down.

roystgnr · 2026-09-16T18:13:25Z

+{
+
+template <unsigned int N>
+struct ReferenceElementVector


This looks like an atavism that never got used?

roystgnr · 2026-09-16T18:15:20Z

   * element node numbers.
   */
-  static const unsigned int side_nodes_map[num_sides][nodes_per_side];
+  static const ReferenceElementTable<num_sides, nodes_per_side> side_nodes_map;


These are public members; is there any way we can avoid changing their type and thus breaking backwards compatibility?

roystgnr · 2026-09-16T18:19:10Z

+}
+
+LIBMESH_DEVICE_INLINE constexpr ReferenceElementTable<8, 3>
+pyramid_edge_nodes()


Let's have a less-broad name for this? pyramid_quadratic_edge_nodes() maybe? It's valid for Pyramid13/14/18, but not Pyramid5 and not for many hypothetical future subclasses like a cubic Pyramid30.

Same issue with tet, hex, and prism cases; pyramid is just where I noticed it first.

roystgnr · 2026-09-16T18:22:13Z

+
+
+LIBMESH_DEVICE_INLINE constexpr ReferenceElementTable<5, 4>
+prism6_side_nodes()


Can we move the element-specific cases to element-specific headers? E.g. cell_prism6.h for this and cell_prism.h for the quadratic version?

roystgnr · 2026-09-16T18:48:21Z

+}
+
+LIBMESH_DEVICE_INLINE bool
+requires_side_specific_topology(ElemType parent)


These should probably be static member functions (or static arrays like type_to_n_sides_map in the simpler cases) in Elem::.

roystgnr · 2026-09-16T19:06:11Z

+}
+
+LIBMESH_DEVICE_INLINE constexpr bool
+try_local_edge_node(ElemType parent,


Documentation?

roystgnr · 2026-09-16T19:09:23Z

+}
+
+LIBMESH_DEVICE_INLINE constexpr ElemType
+linear_sibling_or_invalid(ElemType type)


Like "parent", AMR/C called dibs on "sibling" first.

Is this just Elem::first_order_equivalent_type(), but rewritten with a different name? We cannot keep letting AI try to slip that by us! That does not get us from an API with N functions to one with N², it gets us from N to ∞.

roystgnr · 2026-09-16T19:15:53Z

+  }
+}
+
+// Corner k of a linear element's side, from the stored linear tables.


What does "corner" mean here? vertex? But we don't seem to be doing any tests on k here; this will happily return a non-vertex.

This also seems like something that could be a composition rather than a single function.

roystgnr · 2026-09-16T19:17:39Z

+try_local_edge_node(ElemType parent,
+                    unsigned int edge,
+                    unsigned int edge_node,
+                    unsigned int & node)


We have an invalid_uint; could we just return that for the false case rather than a separate bool?

roystgnr · 2026-09-16T19:33:09Z

+}
+
+LIBMESH_DEVICE_INLINE constexpr bool
+try_local_edge_node(ElemType parent,


We can drop the try_ on all these names; this is the same thing as our local_edge_node but taking an input ElemType (which again shouldn't be called "parent") rather than Elem * this, right? We can just overload Elem::local_edge_node with a static, right? I'd prefer to throw an error (ideally there'd be some way to libmesh_assert() in runtime invocations while doing static_assert at compile-time, though I confess I'm not sure how) when we're asking for an invalid node, but if we really do need to call functions like these in cases where it might return false, we can return invalid_uint instead.

Move the side_nodes_map and edge_nodes_map initializers from the .C files into the class definitions as static constexpr members, so the reference-element topology is available at compile time to anything that includes the element header. C++17 makes these inline, so the out-of-line definitions go away; the members' names, types and public visibility are unchanged.

Move its initializer into the class definition so node counts can be composed with other compile-time lookups, e.g. type_to_n_nodes_map[side_type(t, s)] for the number of nodes on a side.

Add Elem::side_type(ElemType, s), Elem::local_side_node(ElemType, s, n) and Elem::local_edge_node(ElemType, e, n), giving the reference-element topology of any fixed-topology type without an instantiated Elem to call the virtual versions on. They read the same static node maps the element classes use, so there is still one copy of each table. Out of range input asserts; types without a static topology error out.

A second-order element's edge_nodes_map is its first-order equivalent's edge_nodes_map with the mid-edge node appended, numbered after the vertices in edge order; its side_nodes_map row is the first-order vertex row, then the mid-edge node of each consecutive vertex pair, then the face node if the side has one. Compute both at compile time from the first-order tables (fe_reference_element_traits.h) instead of writing them out, so the 15 second-order classes store no node tables of their own and a side map can no longer disagree with an edge map. The classes keep their side_nodes_map / edge_nodes_map members, as references to the derived arrays, so every existing use -- indexing, std::begin()/std::end() on a row, sizeof, MOOSE's ray tracing -- compiles unchanged. The only hand-written facts left are the first-order tables and the face-node rule for the seven types that have face nodes, which stay in their element headers.

test_static_topology checks the new Elem::side_type/local_side_node/ local_edge_node(ElemType, ...) overloads against the virtual versions on an actual element of every type. test_higher_order_node_placement checks the one relationship the static_asserts can't: every non-vertex reference node sits at the centroid of the vertices of its edge, face or element, except for EDGE4's trisection nodes. This is the check that flagged the Pyramid reference-point bugs fixed in libMesh#4539.

roystgnr requested changes Sep 16, 2026

View reviewed changes

rochi00 added 6 commits September 16, 2026 13:56

Make Elem::type_to_n_nodes_map constexpr

9c1c2d4

Move its initializer into the class definition so node counts can be composed with other compile-time lookups, e.g. type_to_n_nodes_map[side_type(t, s)] for the number of nodes on a side.

Re-bootstrap

2697ede

rochi00 force-pushed the pr1-topology-traits branch from 5a536de to 2697ede Compare September 17, 2026 16:14



		LIBMESH_DEVICE_INLINE constexpr ReferenceElementTable<5, 4>
		prism6_side_nodes()

Conversation

rochi00 commented Sep 16, 2026

Uh oh!

moosebuild commented Sep 16, 2026 • 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

moosebuild commented Sep 16, 2026 •

edited

Loading