Add selectable 3D mortar triangulation options#32820
Add selectable 3D mortar triangulation options#32820Andreas-Lepak wants to merge 9 commits intoidaholab:nextfrom
Conversation
- add triangulation selection for 3D mortar contact - add triangulate_triangles control for triangular polygons - add regression coverage for centroid, ear_clipping, and delaunay paths refs idaholab#32816
377c4cf to
eb665c3
Compare
0d2a7bf to
950e2d4
Compare
|
Job Documentation, step Docs: sync website on 60b8714 wanted to post the following: View the site here This comment will be updated on new commits. |
950e2d4 to
a8bb78d
Compare
- 3D mortar action/debug tests: RunApp → CheckFiles
(check_files is only valid on the CheckFiles tester; RunApp rejected
it, causing a fatal parser error and a non-zero exit that failed every
test-running CI job)
- continuity-3d-non-conforming: restore Outputs/file_base cli_args on
the hex8 and penalty-hex8 tests so the emitted Exodus output matches
the existing gold (continuity_{penalty_,}sphere_hex8_out.e)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- MortarSegmentHelper::triangulatePoly: for the default centroid mode
(without triangulate_triangles) short-circuit to the pre-PR algorithm
before the new canonicalization/area-filtering path. The new path
used an area-weighted polygon centroid and skipped degenerate
triangles, which shifted integration weights for existing 3D mortar
baselines (e.g. test/tests/mortar/3d-periodic.3d_periodic_mortar and
modules/contact/test/tests/3d-mortar-contact.frictionless-mortar-3d).
Reverting the default to the arithmetic-mean centroid fan preserves
those baselines while keeping the opt-in modes (vertex, ear_clipping,
delaunay, centroid+triangulate_triangles) on the new path.
- 3d-mortar-contact tests: change Executioner/end_time=0 to
end_time=0.5 dtmin=0.5 for the action-ear-clipping-debug-mesh and
action-delaunay-debug-mesh cases. With end_time=0 the simulation
never takes a step, which trips restep testing ("Time 0 was never
retried..."). The centroid variants already use end_time=0.5; this
makes ear_clipping and delaunay consistent and exercises a real
solve through each triangulation backend.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Job Precheck, step Clang format on 772a270 wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
The previous commit's centroid-mode short-circuit had a 2-line "if" condition that fits on a single line; clang-format flagged it in Precheck. Collapsing it unblocks the rest of the CI matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Job Coverage, step Generate coverage on 60b8714 wanted to post the following: Framework coverage
Modules coverageContact
Full coverage reportsReports
This comment will be updated on new commits. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
lindsayad
left a comment
There was a problem hiding this comment.
The majority of the code added is in the framework. We should add framework level testing (in test) for that code. And something a little more stringent than CheckFiles would be good. If all of a sudden our triangulation changed dramatically, it would be good to know. At the same time, especially when relying on upstream libraries, we don't want to be overly stringent ... e.g. if the Delaunay triangulation changes slightly due to an algorithmic tweak change in triangle or poly2tri then we don't want to fail tests. So I don't know the exact answer for the right test choice
| /// Map from undisplaced AMG key to the selected mortar segment triangulation strategy | ||
| std::unordered_map<MortarKey, std::string> _mortar_segment_triangulation_map; | ||
|
|
||
| /// Map from displaced AMG key to the selected mortar segment triangulation strategy | ||
| std::unordered_map<MortarKey, std::string> _displaced_mortar_segment_triangulation_map; | ||
|
|
||
| /// Map from undisplaced AMG key to whether already-triangular polygons are further subdivided | ||
| std::unordered_map<MortarKey, bool> _triangulate_triangles_map; | ||
|
|
||
| /// Map from displaced AMG key to whether already-triangular polygons are further subdivided | ||
| std::unordered_map<MortarKey, bool> _displaced_triangulate_triangles_map; |
There was a problem hiding this comment.
You're just propagating the pattern but I think we should make something like a MortarInterfaceConfig struct and then just have two maps, one for undisplaced and one for displaced, instead of 10
| { | ||
| struct ParsedMortarTriangulationOptions | ||
| { | ||
| MortarSegmentTriangulationMode mode = MortarSegmentTriangulationMode::centroid; |
There was a problem hiding this comment.
We typically capitalize enumerator options, at least the first letter
There was a problem hiding this comment.
first letter is now capitalized
| struct ParsedMortarTriangulationOptions | ||
| { | ||
| MortarSegmentTriangulationMode mode = MortarSegmentTriangulationMode::centroid; | ||
| std::string canonical_name = "centroid"; |
There was a problem hiding this comment.
Can we define an enum to string conversion instead of dragging around a conceptual duplicate?
| Real | ||
| orient2dHelper(const Point & a, const Point & b, const Point & c) |
There was a problem hiding this comment.
add a code comment on what this is doing please
There was a problem hiding this comment.
comment added
| std::array<unsigned int, 2> | ||
| canonicalEdgeHelper(const unsigned int a, const unsigned int b) |
There was a problem hiding this comment.
comment added
| // Get orientation of secondary poly | ||
| const Point e1 = secondary_nodes[0] - secondary_nodes[1]; | ||
| const Point e2 = secondary_nodes[2] - secondary_nodes[1]; | ||
| const Real orient = e2.cross(e1) * _normal; | ||
|
|
||
| // u and v define the tangent plane of the element (at center) | ||
| // Note we embed orientation into our transformation to make 2D poly always | ||
| // positively oriented |
There was a problem hiding this comment.
why are we deleting all the code comments?
There was a problem hiding this comment.
sorry, they are back again
| else if (poly_nodes.size() == 3) | ||
|
|
||
| // Legacy centroid path: when the default triangulation (centroid) is selected | ||
| // and triangle re-tessellation is not requested, reproduce the pre-PR |
There was a problem hiding this comment.
another developer a year from now won't have any idea what pre-PR means. This is a common problem with agents. I don't know why they always insert context like this into code. Let's remove the reference to "pre-PR". I think it's enough to just call it legacy. We are also sadly accumulating legacy/old-default options in the mortar system. This keeps minimal changes on PRs but makes for bad user defaults. I don't think you should do anything about this in this PR, but I think we absolutely should in the future. This is going to become more and more relevant as we have vendors trying to use mortar contact in production
There was a problem hiding this comment.
sorry, should have paid more attention to the comments that claude left. And I completely agree about updating the legacy defaults for the mortar contact, we should address this at some point!
| MooseEnum triangulation( | ||
| #if defined(LIBMESH_HAVE_TRIANGLE) || defined(LIBMESH_HAVE_POLY2TRI) | ||
| "vertex centroid ear_clipping delaunay", | ||
| #else | ||
| "vertex centroid ear_clipping", | ||
| #endif | ||
| "centroid"); | ||
| triangulation.addDocumentation( | ||
| "vertex", | ||
| "Triangulate clipped 3D mortar polygons by forming a fan from an existing polygon vertex."); | ||
| triangulation.addDocumentation( | ||
| "centroid", | ||
| "Triangulate clipped 3D mortar polygons by forming a fan from a polygon centroid."); | ||
| triangulation.addDocumentation( | ||
| "ear_clipping", "Triangulate clipped 3D mortar polygons with an ear-clipping algorithm."); | ||
| #if defined(LIBMESH_HAVE_TRIANGLE) || defined(LIBMESH_HAVE_POLY2TRI) | ||
| triangulation.addDocumentation( | ||
| "delaunay", | ||
| "Triangulate clipped 3D mortar polygons using libMesh's constrained-Delaunay PSLG " | ||
| "triangulation backend while preserving polygon boundary edges."); | ||
| #endif | ||
| params.addParam<MooseEnum>( | ||
| "triangulation", | ||
| triangulation, | ||
| "Strategy used to triangulate clipped 3D mortar polygons into mortar segments. The default " | ||
| "is 'centroid' to preserve the legacy 3D mortar segmentation behavior."); | ||
| params.addParam<bool>( | ||
| "triangulate_triangles", | ||
| false, | ||
| "Whether a clipped 3D mortar polygon that is already a triangle should still be subdivided " | ||
| "during triangulation. When enabled, already-triangular polygons are subdivided with the " | ||
| "centroid-based path because the vertex-fan, ear-clipping, and Delaunay backends cannot " | ||
| "refine a triangle any further on their own."); |
There was a problem hiding this comment.
I think we have some parameter transfer functionality. Alternatively you could make a static member upstream that returns InputParameters that we consume in multiple places. This makes it so that we if we add or remove enumerators in the future, we only have to do it in one spot
There was a problem hiding this comment.
Added a static MortarConsumerInterface::triangulationParams() that returns an InputParameters containing only the triangulation enum + triangulate_triangles bool. MortarConsumerInterface::validParams() and ContactAction::validParams() both consume it via params += MortarConsumerInterface::triangulationParams();. The duplicated 33-line block in ContactAction.C is gone, so future enum additions / docstring tweaks propagate to both consumers automatically.
| requirement = 'The system shall allow centroid-based 3D mortar triangulation through the ' | ||
| 'contact action and explicitly subdivide already-triangular clipped polygons ' | ||
| 'when triangulate_triangles is enabled.' | ||
| max_parallel = 1 |
There was a problem hiding this comment.
This mirrors the pre-existing frictionless-mortar-3d-debug-mesh test. I believe it is to avoid issues from multiple ranks writing to the same mortar_segment_mesh.e debug mesh file.
There was a problem hiding this comment.
There's opportunity in these new tests to make use of detail to reduce the amount of duplication in the requirements
There was a problem hiding this comment.
The four new action-based debug-mesh tests now sit under a single [frictionless-mortar-3d-action-triangulation] parent with one shared requirement and a detail per child
- Capitalize MortarSegmentTriangulationMode enumerators (Delaunay/Centroid/EarClipping/Vertex) per MOOSE convention. - Collapse the 10 per-interface maps in MortarInterfaceWarehouse into a MortarInterfaceConfig struct keyed by two unordered_maps (undisplaced / displaced). The canonical_name string duplicate is gone; the enum is the single source of truth. - Restore in-function comments in MortarSegmentHelper.C, add comments to orient2dHelper and canonicalEdgeHelper, replace "pre-PR" wording with "legacy". - Drop the offset parameter from triangulatePoly; it now works in local indices and getMortarSegments applies the offset once when shifting tri_map into the global node numbering. - DRY the triangulation parameter definitions via a static MortarConsumerInterface::triangulationParams() factory consumed by both MortarConsumerInterface::validParams() and ContactAction::validParams(). - Group new 3D mortar contact triangulation tests under shared requirement blocks using the detail field; add a comment explaining why max_parallel = 1 is needed for tests that exercise the mortar debug-mesh exodus output. - Add framework-level coverage of the new triangulation modes in test/tests/mortar/continuity-3d-non-conforming with per-mode gold files. Tolerances are tight enough to catch genuine algorithmic regressions while tolerating numerical drift in upstream libmesh PSLG triangulation backends. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Picks up the inline comments that were missed in the previous review-response commit: - Add a code comment to makeCCWTriangleHelper explaining what CCW means and why the triangulation paths assume CCW input. - Drop the redundant std::sort calls on the node and element ranges produced by libMesh's serial ReplicatedMesh; they already iterate in id order. - Use the libMesh Node directly instead of constructing a fresh Point with explicit (x, y, 0) coordinates - Node IS-a Point and the triangulator operates on a 2D mesh, so z = 0 already. - Switch the non-TRI3 element check from mooseError to mooseAssert, since this is an internal invariant of the delaunay backend rather than a user-facing error. - Use index_range(local_triangle) instead of make_range(3u) so the loop range follows the array size. - Use libmesh_map_find instead of std::unordered_map::at when looking up node ids, matching the rest of the MOOSE codebase. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
MortarSegmentHelper.C uses libmesh_map_find but never included libmesh/utility.h directly. The unity build pulls it in transitively through neighboring TUs in framework/src/constraints/, but the non-unity build does not, breaking compilation of this TU.
|
Job Test, step Results summary on 60b8714 wanted to post the following: Framework test summaryCompared against 4e53b3b in job civet.inl.gov/job/3786076. Added tests
Modules test summaryCompared against 4e53b3b in job civet.inl.gov/job/3786076. Added tests
|
|
Job HPC on 60b8714 : invalidated by @Andreas-Lepak |
refs #32816
Reason
The 3D mortar path needs explicit user control over how clipped overlap polygons are triangulated,
and it also needs a way to control whether polygons that are already triangles should be split any
further.
Design
This change adds a
triangulationparameter to 3D mortar contact and threads it through thedirect mortar-object path and the
Contact/mortaraction path. The supported user-facing valuesare:
vertexcentroidear_clippingdelaunayThis change also adds a
triangulate_trianglesboolean parameter, defaulting tofalse.The implementation updates the mortar triangulation helper so that:
vertexuses the vertex-fan path,centroiduses the centroid split path,ear_clippinguses the ear-clipping path,delaunayuses the constrained Delaunay path when available,triangulate_triangles=false,triangulate_triangles=true.Regression coverage was added in the 3D mortar contact tests for both the action path and the
direct mortar-object path:
In addition to the focused MOOSE regressions, the four triangulation modes were exercised on 3D
HEX8, HEX27, and TET4 contact configurations during local verification.
Impact
This change adds new optional 3D mortar input parameters:
triangulationtriangulate_trianglesExisting inputs do not need to change. The default
triangulate_triangles=falsepreserves the default behavior of leaving already-triangular polygons untouched.