place_sheet runs in parallel: gather-first, one rebuild for every rank count - #518
Conversation
…k count The serial refusal is gone, and not by adding a parallel fork: serial and parallel now share ONE mechanism, so the serial suite exercises the machinery the parallel path runs. Gather-first. The sheet's region - victims, crossed cells, and a margin - is marked as an SF-reconciled function of the mesh (per-vertex h is reduced over the star forest, so no rank's verdict depends on the partition), gathered onto the rank that owns most of it with a shell partitioner, and the serial carve-and-fill runs there as the rank-local step. The gather and its star+1 growth are a mask-driven port of the contact stream's fault-aware redistribution (feature/fault-split-node c869357 / 1d48731, recorded in the coordination ledger), whose label-driven original is measured at np=2..8. The rebuild is the uninterpolated-cells + DMPlexInterpolate pattern (their 3-D splitter's move, same ledger entry): only cell-to-vertex cones are wired by hand, PETSc derives every face, edge and cone orientation, and old faces and edges are recovered by joining their surviving vertex tuples - so labels transfer topologically and no tet orientation is ever hand-encoded. Because the gathered region is rank-interior, nothing the surgery deletes or adds is shared: the star-forest's leaf set is unchanged and reconnect._rebuild_point_sf renumbers it in one exchange. The result is partition-independent by construction - the fill sees the identical cavity whatever the partition was. Two parallel defects found by the np sweep and fixed: - np>=4 HANG: the gather can empty a rank, and cell_diameters raises on one; the rank died while its peers entered the next collective. An empty rank contributes the identity of every reduction, never a raise - the 2-D module's own discipline, now applied at both call sites. The refusal ptest at np=4/5 is the regression test, and it also proves every refusal collective: all ranks raise the identical error, or none does. - Seam faces masquerade as walls: on a distributed mesh a partition-seam face also has local support 1, so "support 1 = wall" misclassifies the seam and protects the wrong vertices. A wall face is support 1 AND unshared, with the vertex mask OR-reconciled. Gates, all global: the labelled facet count equals the input triangle count, domain volume conserved, global Euler number 1 from owned stratum counts, every prior surface's interior-face count re-read off the result, and the info dict asserted identical on every rank. ptest_0854 passes at np=2,3,4,5; the serial suites are unchanged and green on the unified path. Underworld development team with AI support from Claude Code
lmoresi
left a comment
lmoresi
left a comment
There was a problem hiding this comment.
Adversarial review (authoring session).
Where we attacked it: (1) The happy path is not evidence — the np sweep went to 5 ranks and the refusal path is a first-class test, which is what caught the np≥4 hang (gather empties a rank; cell_diameters raises on one; the rank died inside a collective chain and even pytest-timeout could not kill the survivors — diagnosed from the partial dump's petsc garbage-cleanup MPI error). Empty ranks now contribute the identity of every reduction, and the refusal ptest doubles as the collectivity proof: all ranks raise the identical message or none does. (2) Seam-vs-wall: local support 1 misclassifies partition seams as domain walls; the test is support 1 AND unshared, OR-reconciled. (3) Partition-dependence: per-vertex h is SF-reduced before any marking decision, so which vertices are victims is a function of the mesh, never the partition; the gathered cavity is therefore identical from any incoming distribution, and the ptest asserts the info dict identical on every rank. (4) No serial/parallel fork: one rebuild path, so 87 serial tests exercise the machinery the parallel path runs.
Taken, with attribution (ledger rule 2): the shell-partition gather + SF-propagate (fault-split-node c869357/1d487319, mask-driven port) and the uninterp+DMPlexInterpolate rebuild with getFullJoin recovery (their 3-D splitter) — the move that means no tet cone orientation is ever hand-encoded.
Known limits, stated: load concentrates on the surgery rank for the duration of the placement (bounded by the fault region; per-rank fault patches are the recorded escalation); edge_split.cell_diameters still raises on an empty rank — guarded at our call sites, latent for other parallel callers (flagged in the ledger rather than drive-by-fixed, Charter S9); gather-mask margin is +2 h with a loud collective assert if it ever under-reaches.
Recommend squash-merge (solo branch).
Underworld development team with AI support from Claude Code
There was a problem hiding this comment.
Pull request overview
This PR enables underworld3.utilities.place_surface.place_sheet() to run in parallel using a unified “gather-first” mechanism shared with serial execution, ensuring partition-independent results by gathering the sheet’s affected region onto one rank, running the carve/fill there, and then collectively rebuilding the mesh across ranks.
Changes:
- Added SF-reconciled marking + a shell-partitioner gather step so the sheet region becomes rank-interior before surgery.
- Reworked the 3-D rebuild path to use an uninterpolated-cells +
DMPlexInterpolate-style rebuild and added collective global gates (volume, Euler, label integrity). - Added a parallel regression test suite covering deterministic invariants and “collective refusal” behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/parallel/ptest_0854_place_sheet_parallel.py |
Adds parallel tests asserting deterministic contract checks and collective error/refusal behavior across ranks. |
src/underworld3/utilities/place_surface.py |
Implements gather-first parallel path for place_sheet, SF-reconciled marking, seam/wall handling, and collective rebuild + global validation gates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| tmp = chart_values.copy() | ||
| sf.reduceBegin(MPI._typedict[chart_values.dtype.char], tmp, | ||
| chart_values, mpi_op) | ||
| sf.reduceEnd(MPI._typedict[chart_values.dtype.char], tmp, | ||
| chart_values, mpi_op) | ||
| out = chart_values.copy() | ||
| sf.bcastBegin(MPI._typedict[chart_values.dtype.char], chart_values, out, | ||
| MPI.REPLACE) | ||
| sf.bcastEnd(MPI._typedict[chart_values.dtype.char], chart_values, out, | ||
| MPI.REPLACE) |
| [v_uninterp(v) for v in tet]) | ||
| new.symmetrize() | ||
| new.stratify() | ||
| new.interpolate() |
|
We reviewed this against the fault-contact socket (place -> split_fault -> contact -> solve) and pushed fixes for what we found (8ff3659). Verdict: with those fixes, the PR meets the contract and is ready to merge (as a MERGE COMMIT, per the shared-ancestry rule from #510). Finding 1 (correctness, fixed): the sewn mesh had mixed cell handedness. The parallel rewrite dropped the serial path's signed-volume flip; DMPlexInterpolate derives face cones but never normalises cell vertex order, and gmsh's fill tets arrive opposite-handed to the kept cells. Evidence: 4131 vs 443 signed volumes on the sewn mesh; the first Stokes solve (plain, no fault — the isolation control) never converged while every abs()-based volume gate stayed green; the same mesh with cells re-oriented solves in 22 s. Fix: made cells oriented against the kept cells' own sign at wiring, plus a global orientation census gate — abs() volume checks are structurally blind to this defect class. Finding 2 (gates, added): PETSc's DMPlex validity battery as the final pass (maintainer request). Full set in serial — check_geometry is exactly the instrument that catches Finding 1 on day one. In parallel only the partition-safe subset (symmetry, skeleton, point-SF): we measured check_geometry false-positive on every rank of a PLAIN distributed UnstructuredSimplexBox, so it cannot gate distributed results on this stack. Gate failures are allgathered before raising, per this module's own collective-refusal discipline. Finding 3 (contract, our side of the socket): sheet triangulations must have no all-rim triangles — the splitter refuses them (both copies of such a face would carry the same vertex triple), and structured grids produce them at corners. Conditioning by centroid-fanning creates sub-h facets that give the contact a residual floor (measured: rel 1.1e-2 stall persisting under LU, so not solver conditioning); conditioning by flipping the corner-quad diagonals is clean. Recorded on the fault-network side; noted here because sheet providers will hit it. Acceptance on the fixed build: place -> split -> frictionless contact converges in 18 s (faster than the gmsh-embed control, 22 s), no-opening leak 8e-18, sensible slip taper; serial suites 26 green; ptest_0854 green at np=2 and np=4. One honest gap for the ledger: the parallel gates here are topological — a parallel FE acceptance through the splitter waits on the contact stream's pairing-through-redistribution work, which is ours, not this PR's. Underworld development team with AI support from Claude Code |
|
Follow-up from the composed acceptance (place -> split -> contact, and its isolation ladder): there is a SECOND parallel defect, real and reproducible, in the rebuilt mesh's higher-strata consistency. We are handing this one back with the evidence rather than fixing it, since it sits in the rebuild's design. Symptom: plain Stokes (no fault machinery at all) on the placed mesh at np=2 takes 3594 s; the identical mesh in serial solves in 22 s; the plain BASE box at np=2 solves in 13 s. The solve converges eventually — the operator is assembled wrong, not singular. What it is NOT (each eliminated by measurement): not the contact machinery (plain Stokes grinds); not element quality (radius-ratio distribution matches the base mesh); not cell handedness (the orientation census gate passes); not missing/duplicated points (global owned v/e/f/c identical serial vs np=2: 1138/6444/9881/4574); not vertex SF corruption (coordinate drift exactly 0); not the partition (a FRESH DMPlexDistribute of the placed mesh — canonical SF, healthy seam counts — still takes 2212 s); not the solver config (base box np=2 is fast). What the evidence points at: DMPlexCheckFaces FAILS on the rebuilt mesh at np=4 (rank 3: face closure 1462/1456/1418 vs cell face 1456/1462/1418 — same vertex set, swapped order = a cone-orientation mismatch), while the plain distributed base box passes CheckFaces cleanly. We must retract part of our earlier gate scoping: only check_geometry false-positives on healthy distributed meshes; check_faces failing on the REBUILT mesh is a true positive. Inconsistent face/edge cone orientations across the point SF disorder the P2 edge/face DOF numbering during assembly — which produces exactly this signature, and which serial cannot see (no seam) and redistribution cannot heal (it faithfully migrates the local cones). Where to look: the collective uninterpolate -> DMPlexInterpolate rebuild interpolates each rank's chart locally; nothing afterwards reconciles the leaf faces'/edges' cone order against their roots. The 2-D module never met this because its surgery output is consumed rank-locally; the FE assembly across the seam is what demands root/leaf cone agreement. Suggested acceptance for the fix: CheckFaces green on the rebuilt mesh at np=2..5, and plain Stokes on a placed mesh at np=2 within ~2x of the base box (ours: 13 s). Merge posture: this does not block the serial capability (fully validated, 18 s acceptance) or the parallel topological layer (gates green np=2-5). It DOES block parallel FE work on placed meshes. Our recommendation: merge, with this defect recorded as the ledger item gating "parallel placed-mesh FE", or hold for the cone-reconciliation fix — maintainer's call. Underworld development team with AI support from Claude Code |
…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 the last refusal on the 3-D placed surface, per the maintainer ruling that "planned, ready" parallel gaps fossilize. Serial and parallel share one mechanism — no fork for the serial suite to diverge from.
Mechanism
Gather-first. The sheet's region is marked as an SF-reconciled function of the mesh (per-vertex h reduced over the star-forest, so no rank's verdict depends on the partition), gathered onto one rank by a shell partitioner (mask-driven port of the contact stream's fault-aware redistribution,
c8693579/1d487319, recorded in the ledger — the label-driven original is measured at np=2–8), and the serial carve-and-fill runs there as the rank-local step.The rebuild is the uninterpolated-cells +
DMPlexInterpolatepattern (their 3-D splitter's move, same ledger entry): only cell→vertex cones are wired by hand, PETSc derives every face/edge/orientation, and old faces and edges are recovered by joining surviving vertex tuples — labels transfer topologically, no tet orientation hand-encoded, ever. The gathered region is rank-interior, so nothing the surgery touches is shared: the leaf set is unchanged and_rebuild_point_sfrenumbers in one exchange. Partition-independent by construction — the fill sees the identical cavity whatever the incoming partition.Two parallel defects found by the sweep, fixed, regression-tested
cell_diametersraises on one — a rank-local death while peers entered the next collective (pytest-timeout couldn't even kill it). Empty ranks now contribute the identity of every reduction; the refusal ptest at np=4/5 regression-tests it and simultaneously proves every refusal collective (all ranks raise the identical error, or none does).Verification
ptest_0854at np=2, 3, 4, 5 (version-stable gates: facet count ≡ input triangles, volume conserved, global Euler 1 from owned counts, walls preserved as owned sums, info dict identical on every rank).Underworld development team with AI support from Claude Code