Uh oh!
There was an error while loading. Please reload this page.
Free surface in 3D: spherical shell support, dimension-general surface machinery, and the 2D compatibility-floor fix - #469
Conversation
lmoresi
commented
Jul 29, 2026
Adversarial review (pre-review, per house practice)Ordered by how much each should worry the reviewer. 1. The quotient-space convergence measure is doing load-bearing work. The 2D floor fix's third leg declares convergence while a physically-real (small) incompatibility remains in the pressure-gauge component. This is standard for projected singular systems, and the component is minimized by the directed flux strip — but nothing REPORTS its size per solve. A user whose deformed geometry develops a large gauge-mode residual would see "converged" with no hint. Worth a follow-up: log the projected component's magnitude when it exceeds, say, rtol·reference. 2. The P1-projected σ_nn recovery is validated by physics, not by an exact oracle. The fold-down is exact as a load transfer (interpolation transpose), but its recovery ACCURACY on curved 3D traces is only evidenced by the Y20 benchmark's behaviour (right direction, resolution-convergent bias matching the 2D figure). No manufactured-traction test exists for the 3D path the way the SolCx topography test pins 2D. The delegated benchmarking should include one. 3. 3D parallel is untested. All 3D evidence is serial. The surface-mean reduction and the lex-key gather are collective-by-construction, but the empty-boundary-rank path (a rank owning no piece of the surface) has never actually executed — annulus partitions at np2/np4 always give every rank boundary. A spherical shell at np4+ is the first genuine test, and it is explicitly in the delegated list. 4. The benchmark's fitted rate is not resolution-converged (8.2 at res 0.2, 6.1 at res 0.15, half-space reference 12.5): the O(1) shell correction claim is qualitative. The floor's clean 16%→2.2% convergence is the strong quantitative result; the rate needs the delegated study before any analytic comparison is asserted. 5. Smaller items. The 3D lex-key gather assumes the surface deforms only along the normal (ordering built once — same assumption as the 2D ring, inherited, documented). The What is well-covered: the 2D regression surface (every prior test green throughout, serial and np2, including the re-derived quadrature invariant); the three-cause floor fix each verified by measurement rather than assertion; capability pinned by test_1072 at 17s. Underworld development team with AI support from Claude Code |
…ension-general) _ring_weights becomes the P1 lumped boundary mass accumulated per OWNED facet from the plex (edge length/2 in 2D; triangle area/3 in 3D - the same code is the 3D area-weighted gauge), on live coordinates. Seam weights are assembled by summing per-rank partial contributions (_ring_gather gains op="sum"; ghost facets excluded via the point SF so each facet counts exactly once globally). The quadrature test's invariant changes to the meaningful one: the gauge total equals the FE boundary measure of the deformed POLYGON (BdIntegral), not the ideal-circle arc length. Honest finding from the acceptance rerun: the deformed-ring compatibility floor (rel ~2e-3, 100% pressure-row) SURVIVES this fix at unchanged levels - the demean weighting was not the dominant term. Remaining suspect: the nodal-normal (analytic radial) vs facet-normal mismatch on the deformed polygon - the constraint fixes u.rhat at nodes while the discrete flux functional integrates u.n_facet, and the mismatch involves the free tangential velocity, so no nodal demean of the datum can zero it. Recorded in the acceptance README; formulation question for review. Tests: test_1070 serial (6+1) + np2 (7) + quadrature np4; surface metrics at strong level throughout. Underworld development team with AI support from Claude Code
… quotient convergence — deformed-ring floor resolved The compatibility floor (strong-datum consistent solves stalling honestly at rel ~2e-3 on the deformed ring, residual 100% in the pressure rows) had TWO stacked causes beyond the demean weights, resolved here: 1. The datum's DIRECTED mean: the divergence rows enforce the flux through the deformed FACET normals while the constraint fixes u.n along the nodal (analytic) normal. The datum is now shifted by Phi/S with Phi = BdIntegral(u_n_target * (n_node . Gamma)) and S = BdIntegral(n_node . Gamma) — the same FE surface integral the residual uses (maintainer ruling: strip the mean volume-flux term, computed fully consistently), so the constrained field carries exactly zero discrete net flux. Strong-constraint path only; collective, rank-identical shift. 2. The outer residual is now measured in the pressure-gauge QUOTIENT space when the constant-pressure nullspace is active: the inner KSP projects the constant mode from every increment (the gauge of an enclosed domain), so the outer loop cannot reduce that component and must not measure it — on a deformed faceted boundary it is not exactly zero (free tangential DOFs carry flux through the node-vs-facet normal mismatch) and an unprojected norm floors against the constant offset, stalling the line search. Mirrors PETSc's own projected residual for singular systems. The sigma_nn reaction stash stays unprojected. Measured: power-law annulus FS acceptance now CONVERGES at every step (3, 3, then 1 Newton increment per warm-started step, no warnings) with surface metrics at strong-datum level (err 1.1e-2 ~ isoviscous 7.9e-3, penalty 0.49; net flux 7.4e-4). Tests: test_1018 18/18; test_1070+1071 serial; test_1070+1066 np2 (9). Underworld development team with AI support from Claude Code
…rface machinery + P1-projected sigma_nn recovery The FreeSurface manager now runs on a 3D spherical shell end to end (first smoke: two full solve/advance steps, physically sensible h_inf). The blanket dim!=2 guard is replaced by per-piece capability checks; the two genuinely 2D-only features (tangential transport, ring Taubin filter) are refused explicitly in 3D. - Surface mean/demean: reduction over OWNED-facet partial trace-mass weights (each facet counted exactly once globally, two scalar allreduces) - no gather, no ordering, dimension-general. The ordered ring survives only for the 2D-only features; its gather/dedup machinery generalises to 3D via lexicographic rounded-coordinate keys (deterministic order + exact same-node grouping; the 2D s-ordering and seam handling are unchanged). - Geometry helpers route through _radial (CYLINDRICAL2D or SPHERICAL): radial height/deform direction on shells, last-axis on boxes - already dimension-general otherwise. - sigma_nn on a 3D P2 trace: new boundary_flux mass="p1" - P1-PROJECTED recovery: fold each edge-midpoint load onto its edge vertices (exact interpolation transpose, load-conserving), de-smear with the sound P1 lumped triangle mass, read midpoints back as the P1 interpolant. Chosen over mass="consistent" because the P2 vertex-integral checkerboard (#404 hold) sits exactly at the vertices the P1 h_inf field consumes. The manager translates its "lumped" default to "p1" on 3D meshes; 2D maps "p1" to the sound lumped line mass. - The interior carrier reads the deform displacement by DIRECT nodal row map (built once; falls back to point evaluation only if the carrier nodes do not coincide with the mesh coordinate nodes) - removes the on-node point-evaluation exposure (#432 class) and a per-step global location. Tests: 2D battery green (test_1018 + 1070 + 1071 serial = 25; 1070 + 1066 np2 = 9). 3D shell smoke in ~/+Simulations/FreeSurface/spherical_fs/. Underworld development team with AI support from Claude Code
…e 3D free surface test_1072: end-to-end shell loop (finite mean-free h_inf under the trace-mass gauge, surface moves toward equilibrium, bounded) + explicit refusal of the 2D-only features. The quantitative benchmarking (analytic Y_lm shell rate, bias convergence, low-Ra spherical convection, 3D parallel) is deliberately delegated to the review pass - evidence and scripts in ~/+Simulations/FreeSurface/spherical_fs/. Underworld development team with AI support from Claude Code
9e820d1 to
fc4e3c3Compare…servability, trace-mass conservation assert, docstring currency F1: _surface_node_coords guards the surface stratum IS before getIndices() (the house NULL-handle-IS guard every other call site carries) — a rank owning no surface facets segfaulted the whole job at np>=2 under partition asymmetry. Verified: np=2 --with-mpi plume suite 7 passed. F2: the pressure-gauge quotient projection now surfaces what it removes — solver._rotated_pressure_gauge_residual holds the projected component's norm, and a converged solve warns loudly when that component dominates the converged residual (an incompatible wall-normal datum: net boundary flux an enclosed incompressible domain cannot absorb — previously a silent mass-conservation violation). F4: _surface_weights_local asserts the coordinate-key matching conserves the accumulated trace mass exactly — the 0.0 fallback could otherwise drop weights silently and corrupt the datum gauge. F5: docstrings — mass="p1" documented at both recovery entry points; arc-length wording updated to the FE trace-mass gauge it now is. Underworld development team with AI support from Claude Code
lmoresi
commented
Aug 4, 2026
Adversarial review — head fc4e3c3 (post-rebase), isolated worktree, own buildThe PR body deferred benchmarking to the review pass, so this review verified the five headline claims empirically as well as attacking the code. All five hold; two MAJOR defects found, both fixed in response commit 26eec2d. Findings
Attacks that failed (independently measured)
The rebase itself (4 commits replayed over the #458/#465/#471/#456 wave) was also under review: only the CHANGELOG needed manual resolution, and the semantic composition inside |
Uh oh!
There was an error while loading. Please reload this page.
…de coordinates, and the consistent line mass (#459) (#537) * 2D boundary flux: give every trace node its own coordinate — degree-3 recovery was silently wrong (#459) A degree-3 trace carries TWO interpolation nodes per boundary edge point, but the node gather emitted one entry per DMPlex point with the point's single (midpoint) coordinate: one of the two edge-interior reactions was never read, the coordinate keys collapsed, and the de-smear proceeded down the P2 path — returning 0.57-0.74 of an exact unit flux from 17 of the 25 trace nodes, with no error raised. The fix keys by interpolation node, not by point: * _boundary_field_nodes emits (point, slot, coord) per NODE; the true edge-interior node coordinates come from interpolating the mesh coordinate field into a matching-degree space (_trace_interior_coords, the same createDefault/node_endpoints=False construction used for every UW3 field FE, so the per-point slot ordering matches the field section by construction). The build is collective; whether it is needed is agreed by allreduce so empty ranks participate. * boundary_flux reads the reaction per (point, slot) — the second P3 edge dof is no longer dropped; vector reactions read node-major. * _desmear derives the trace order structurally from the field section (no more midpoint-key sniffing) and assembles a general Lagrange line mass from the measured node parameters; P1/P2 keep their exact hardcoded matrices byte-for-byte. * mass="auto" now selects the consistent solve for 2D traces of degree >= 3: PETSc places the interior nodes asymmetrically (Gauss-Jacobi), where row-sum lumping is only O(h) pointwise (measured 1.6e-2 flat error at res 8 for a linear flux, vs 5.8e-4 consistent). P1/P2 keep lumped, which is exact for linear flux on their symmetric layouts. * The de-smear now REFUSES a key collapse exactly (len(nodeR) vs len(xs), both 2D and 3D branches) instead of silently overwriting — the cheap detection the issue asked for, kept as a guard. Measured on 8x8 unit-flux conduction (T = 1 - y, exact at every degree): degree 1: 8.0e-5, degree 2: 1.2e-4 (both unchanged, bit-identical), degree 3: 4.3e-1 before -> 3.1e-4 after, 25/25 nodes reporting. The 3D path is untouched: P3 traces there were already rejected legibly (facet/edge dof checks) and still are; the #469 mass="p1" mode is unchanged. Underworld development team with AI support from Claude Code * Tests for #459: degree-sweep oracle, interior-node placement, collapse negative control, np2 arm * test_boundary_flux_degree_sweep_2d: T = 1 - y unit flux at trace degrees 1/2/3 — every wall node exact, every trace node reporting (fail-before validated: degree 3 read 0.57-0.74 of the unit flux from 17 of 25 nodes on the unfixed build; degrees 1-2 passed). * test_boundary_flux_p3_interior_node_placement: T = xy gives a flux linear in x, so a slot swap or midpoint-collapsed node placement displaces the recovery by ~2e-2 — 36x the passing mid-wall error. The constant-flux oracle alone cannot see a swap (both edge-interior reactions are equal under uniform flux). * test_boundary_flux_p3_collapse_guard: negative control — force the pre-fix collapse (both edge-interior nodes keyed by the edge midpoint) and the de-smear must refuse with the #459 RuntimeError, proving the guard fires rather than silently overwriting. * parallel/test_1065: degree-3 unit-flux arm at np >= 2 — the per-node coordinate build is collective, so ranks owning none of the flux boundary must still participate (passes np2 and np4). Underworld development team with AI support from Claude Code
What this is
Workstream C first pass (Louis + Claude; @gthyagi for review):
uw.systems.FreeSurfaceruns end to end on a 3D spherical shell, built by making the surface machinery dimension-general rather than porting it piecewise. On the way, the 2D strong-datum compatibility floor was fully resolved (three stacked causes, each measured before fixed).The 2D fix (rides first, benefits everything)
Strong-datum consistent solves on a deforming ring stalled honestly at rel ~2e-3, residual 100% in the pressure rows (block-split evidence). Three causes, closed in order, each verified by rerunning the power-law acceptance:
Φ = ∮ũ_n(n̂·Γ)ds,S = ∮(n̂·Γ)ds,mesh.Gamma), so the constrained field carries exactly zero discrete net flux.Result: every step of the power-law annulus acceptance converges (3, 3, then 1 warm-started increment per step; previously honest stalls).
The 3D machinery
boundary_fluxmass="p1"— P1-projected recovery (edge-midpoint loads folded exactly onto vertices via the P1⊂P2 interpolation transpose; sound P1 lumped triangle mass; midpoints read back as the P1 interpolant). Chosen overmass="consistent"because the P2 vertex-integral checkerboard (Fix 3D boundary reaction flux recovery #404 hold) sits exactly at the vertices the P1 h∞ field consumes. The manager translates itslumpeddefault top1on 3D meshes.dim != 2guard; the ring's gather generalises via lexicographic coordinate keys for its remaining diagnostic uses.Evidence (
~/+Simulations/FreeSurface/spherical_fs/)Spherical Y20 topographic relaxation (constant-density shell,
background_buoyancy="analytic", resolved timestep): clean exponential decay at rate 6.1–8.2 vs the half-space Cathles reference 12.5 — an O(1) shell/fixed-base correction in the physically correct direction — with the equilibrium modal bias falling 16.3% → 2.2% of A0 over one resolution step (matching the 2D production-resolution figure of ~2.4%; the #431-class discrete recovery defect, resolution-convergent, not structural).Tests: new
test_1072(3D capability, 17s); full 2D battery green throughout (test_1018 18/18, 1070, 1071 serial; 1070 + 1066 at np2 and np4 for the quadrature/datum tests).Deliberately delegated to the review/benchmarking pass (maintainer decision)
This PR ships the capability with first-evidence physics; the detailed benchmarking is left open, and this is the note recording that decision:
Related: #457 (TI consistent tangent, found during workstream B) still limits Newton-rate claims for anisotropic rheology in any dimension.
An adversarial review follows as the first comment.
🤖 Generated with Claude Code
Underworld development team with AI support from Claude Code