Fix #520: seam cone reconciliation + the recovered orientation fix — parallel FE on placed meshes works - #521
Conversation
…ut reconciled (#520) Interpolating first and installing a star-forest afterwards leaves every rank's faces and edges with whatever cone order its local interpolation chose — nothing reconciles a leaf's cone against its root's, and DMPlexCheckFaces fails at the seam (issue #520's oracle). With the vertex SF attached to the uninterpolated plex, DMPlexInterpolate runs its distributed path: it creates faces and edges, orients the interface cones consistently across ranks, and extends the star-forest to the new points itself. The after-the-fact _rebuild_point_sf call was the defect and is gone; the interpolated SF is mirrored onto the coordinate DM (the parallel-checkpoint property reconnect._install_point_sf documents). Measured: CheckFaces green on the placed mesh at np=2..5 (previously failing at every rank count with same-vertex-set/swapped-order cones). Underworld development team with AI support from Claude Code
… gates Review findings from the fault-contact acceptance (place -> split -> contact -> solve), with fixes: 1. The parallel rewrite dropped the serial path's signed-volume flip. DMPlexInterpolate derives face cones from cell-vertex cones but never normalises the cells' own handedness, and the fill's tets arrive in gmsh's convention — opposite to the plex closure convention the kept cells carry. The sewn mesh had 4131 cells one way and 443 (the fill's) the other; every abs()-based volume gate stayed green while the first Stokes solve on the mesh ground forever on an indefinite operator (plain Stokes, no fault — the isolation control). Made cells are now oriented against the kept cells' own sign before wiring, and an orientation gate (global positive/negative/degenerate census) makes this class of defect loud. 2. PETSc's DMPlex validity battery wired in as the final pass (maintainer ruling): full set in serial; in parallel the partition-safe subset (symmetry, skeleton, point-SF), because check_geometry false-positives on every rank of a PLAIN distributed UnstructuredSimplexBox on this stack. Gate failures are allgathered before anyone raises, per this module's own collective-refusal discipline. Acceptance on the fixed build: place -> split_fault -> frictionless contact solve converges in 18 s (faster than the gmsh-embed control's 22 s), no-opening leak 8e-18, sensible slip; serial suites 26 green, ptest_0854 green at np=2 and np=4. Note the sheet-triangulation contract discovered en route (fault-split side): no sheet triangle may have all three vertices on the rim — condition structured grids by flipping corner-quad diagonals, not by inserting centroid points (sub-h fan facets give the contact a residual floor). Underworld development team with AI support from Claude Code
…xactness tests The recovered review gates move into _validity_and_orientation_gates(), called by place_sheet AND place_thin_volume (the embed shares the sewing skeleton, so it shares the defect classes). check_faces is promoted from serial-only to every rank count: it is issue #520's oracle, the plain distributed box passes it cleanly, and with the vertex SF attached before the interpolate the rebuilt mesh passes it too — so the gate that found the defect now stands guard in every parallel test that places anything. check_geometry stays serial-only (measured false-positives on healthy distributed meshes on this stack). New tests: P2 Poisson must reproduce a quadratic on the placed-sheet mesh (test_0854) and through the embedded zone (test_0855) to solver precision — the end-to-end assembly oracle that was missing when the mixed-handedness defect shipped: a mesh can pass conformity, Euler, exact volume and every abs()-based gate while assembling a wrong operator; a contained exact solution cannot be faked. Measured on the fixed build: max error 1e-11 (sheet) and 9e-16 (volume), serial and np=2,4; plain Stokes on the placed mesh at np=2 is 12.1 s against the base box's 7.6 s — issue #520's acceptance was 2x, the defect measured 3594 s. Underworld development team with AI support from Claude Code
|
Adversarial review (self, posted per project practice). The verification ladder, and what each rung eliminated: The probe was wrong before the mesh was. Our first Stokes probe silently swallowed BC failures (try/except around add_dirichlet_bc) and timed unconverged solves; a second grep-filtered its own diagnostics away. Both violations of our own probe hygiene, both caught by their numbers making no sense. The probe that finally cut through was the exactness oracle: P2 contains quadratics, so a correct assembly RETURNS them — no baselines, no timing, no tolerance debate. Attribution was measured, not assumed. The serial wrongness reproduced bit-identically on landed eb8d2d3 with the working tree stashed — so it predates this branch and is not our SF change. The fresh-plex control (createFromCellList from the same cells+coords: identical error) placed the defect in the DATA, and the signed census then produced the review's own numbers, 4131/443 — which is what sent us looking for the orphaned commit rather than a new mechanism. The recovered fix is theirs, verbatim. 8ff3659 cherry-picked with authorship preserved; our additions are the refactor into a shared gate (both primitives), the check_faces promotion to all rank counts, and the FE oracles. Credit and the process lesson go together: a review-fix pushed to a side branch after a squash-merge has no path into the trunk — the ledger now says review fixes land as PRs, not branch pushes. Residual risks, stated: (1) DMPlexInterpolate's distributed path is now load-bearing for cone reconciliation — we verify its output (check_faces, every call, every rank count) rather than trusting it. (2) The FE oracle uses diffusivity 1 everywhere; a coefficient-jump oracle across the zone skin would additionally pin the label geometry — deferred, the labels are join-recovered and count-gated. (3) The parallel FE proof in CI rides on the in-call gates plus serial exactness tests; a full parallel solve test was weighed and dropped to keep the suites lean — the gate that found this defect now fires inside every parallel placement test. Underworld development team with AI support from Claude Code |
There was a problem hiding this comment.
Pull request overview
Fixes the parallel rebuild path for sewn/placed meshes by ensuring PETSc can reconcile interface cone orientations across the PointSF during interpolation, and adds an end-to-end finite-element oracle to catch wrong-operator assembly (including the recovered mixed-handedness fix).
Changes:
- Attach the uninterpolated mesh’s vertex PointSF before
DMPlexInterpolate, letting PETSc build consistently oriented seam faces/edges in parallel and extending the SF during interpolation. - Restore/centralize validity + orientation gates (PETSc
DMPlexCheck*battery via options + signed-volume handedness census) and apply them to bothplace_sheetandplace_thin_volume. - Add P2 Poisson exactness regression tests for
place_sheetandplace_thin_volumemeshes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/underworld3/utilities/place_surface.py |
Reorders SF/interpolation, restores cell orientation normalization, adds shared validity/orientation gates for sewn meshes. |
tests/test_0854_place_sheet.py |
Adds a P2 Poisson quadratic-reproduction oracle on placed-sheet meshes to detect wrong-operator assembly. |
tests/test_0855_place_thin_volume.py |
Adds the same FE oracle on embedded thin-volume meshes (shared sewing skeleton). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| the interface cones come out consistently ordered across ranks — the | ||
| issue #520 fix. | ||
|
|
||
| ``made_cells`` entries are mixed: a non-negative value is an OLD vertex | ||
| point; ``-(k+1)`` is row ``k`` of ``placed``. Returns the interpolated |
| raise RuntimeError( | ||
| "place_sheet internal: a shared vertex was deleted by the " | ||
| "surgery; the gather mask under-reached.") |
…n test The interface-cone fix (#521, closing #520) verifies independently on this branch: CheckFaces green at np=2 on the rebuilt mesh, placed-mesh plain Stokes 24 s (was 3594 s), and — the acceptance that motivated the whole ladder — the COMPOSED chain place_sheet -> split_fault -> frictionless contact at np=2 converges in 18 s with the serial answer (peak slip 0.1461 vs 0.1462) and machine-zero no-opening leak. ptest_0852 locks that chain as a regression gate: the solve IS the gate, because both #518-era defects passed every topological check. Underworld development team with AI support from Claude Code
Closes #520 — and en route recovers an orphaned review fix without which even SERIAL FE on placed meshes was wrong.
What #520's investigation actually found
Two stacked defects, one of them older and worse than the issue knew:
1. The #518 review's finding-1 fix never landed. The contact session pushed
8ff3659b(orient fill cells to the kept convention + PETSc validity gates) to a review side branch; #518 was squash-merged without it and the branch deleted — the fix was orphaned. Consequence: every placed-sheet mesh since #517 had mixed cell handedness (measured again: 4131 cells one way, 443 the other — the review's exact numbers), and FE assembly on such a mesh is wrong in serial: a P2 Poisson with a LINEAR exact solution came back with errors of 27 under exact LU, violating the maximum principle. Nobody saw it because the only FE consumer (the fault-contact stream) re-rebuilds the plex through its own splitter, and #520's serial table row only recorded time.8ff3659bis cherry-picked here verbatim, authorship preserved.2. The #520 defect proper: interface cones unreconciled across the point SF. The rebuild interpolated each rank's chart with NO star-forest attached and installed one afterwards — so nothing ever reconciled a leaf face/edge cone against its root. The fix is ordering:
_attach_uninterp_vertex_sfgives the uninterpolated plex its vertex SF (one broadcast of the new numbering over the old SF — the leaf set is unchanged under gather-first), andDMPlexInterpolatethen runs its distributed path — creating faces/edges, orienting interface cones consistently, and extending the SF itself. The after-the-fact_rebuild_point_sfcall was the defect and is gone; the interpolated SF is mirrored onto the coordinate DM (the parallel-checkpoint property).Acceptance (issue #520's criteria)
_validity_and_orientation_gates(), shared byplace_sheetandplace_thin_volume, withcheck_facespromoted from serial-only to every rank count (the plain distributed box passes it cleanly; it is exactly this issue's oracle).check_geometrystays serial-only (measured false-positives on healthy distributed meshes).The new oracle class
P2 Poisson must reproduce x²+y²+z² to solver precision on the sewn meshes — added to test_0854 and test_0855. This is the end-to-end assembly check that was missing when the handedness defect shipped: conformity, Euler, exact volume and every abs()-based gate are all structurally blind to a wrong operator; a contained exact solution cannot be faked. Serial suites 38 green; ptest_0854+0855 green at np=2..5 with the check_faces gate firing inside every placement call.
Underworld development team with AI support from Claude Code