fix(meshing): mmpde metric — monotone RBF bake from nodal values#266
Merged
Conversation
Restores the intended "RBF metric eval" robustness. The mmpde mover's RBF/Shepard metric path baked the metric via an FE evaluation at the FIXED reference cloud `ref`; on a deformed mesh that reference can mis-locate / drift outside the deformed interior and the FE evaluation returns garbage (the P1 density — strictly positive by construction — comes back NEGATIVE, even at a field's own DOFs). A negative density is a non-SPD metric, which the mover then either NaN-bails on (a hidden stall) or, with the #259 SPD-floor, acts on as "coarsen hard here" → giant cells / holes / divergence in the adapted mesh. Fix: bake the metric at the CURRENT mesh NODES (its own DOF locations) and Shepard-interpolate from there. A Shepard (positive-weight, convex) average of the positive nodal values is GUARANTEED >= 0 (monotone) — the metric can never go non-SPD from the eval — and nodes are always inside the mesh (no out-of-domain / drift) and need no per-step cell location (fast). `ref` is kept for the _edge_mats reference frame. Best paired with #264 (deformed-mesh point-location fix), which makes the nodal FE bake exact; together the adapted mesh stays clean under forced every-step adaptation at R=5 (folded=0, cell-area-ratio flat) where it previously tore holes. Validated in a sibling worktree (this is a one-line source change; CI runs the full mover suite). Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts the MMPDE mover’s metric_eval="rbf" fast-path to improve robustness on deformed meshes by changing where the metric is sampled (“baked”) before Shepard (k-NN inverse-distance) interpolation.
Changes:
- Bake the analytic metric at the current mesh node coordinates (
coords) instead of the cached fixed reference cloud (ref). - Build the k-NN tree from the node coordinates and interpolate metric values from those samples.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+3152
to
+3155
| # * MONOTONE: a P1 density is positive by construction; Shepard is a convex | ||
| # (positive-weight) average of the sampled node values, so the result is | ||
| # GUARANTEED positive — no negative/non-SPD garbage, the SPD floor / NaN | ||
| # bail never has to fire. |
Comment on lines
3161
to
+3164
| if metric_eval == "rbf": | ||
| from scipy.spatial import cKDTree | ||
| M_ref = _eval_M_analytic(ref) # one analytic pass | ||
| _tree = cKDTree(ref) | ||
| M_src = _eval_M_analytic(coords) # nodal values (positive) | ||
| _tree = cKDTree(coords) |
Member
Author
|
This is a minor detail - the metric evaluation for the adaptive meshing needs to be 1) fast and 2) monotonic ... rbf is a good choice, especially since it does not require the full mesh interpolation infrastructure to be valid during mesh movement (a difficult task to sequence correctly) |
lmoresi
added a commit
that referenced
this pull request
Jul 6, 2026
…og, value-first call-site sweep (WE-01..03,05,06,08,09,10) (#338) * docs(WE-01): adopt the one-governing-doc-per-topic authority map Repoint CLAUDE.md's Data Access 'Authoritative Reference' from the stale UW3_Style_and_Patterns_Guide.md to subsystems/data-access.md (the guide it crowned teaches patterns the code deprecates at runtime — DOC-04), and record the Style Charter §10 authority table in docs/developer/index.md as the master authority index. The Charter is added to the Getting Started toctree (removes a baseline 'not included in any toctree' warning). Finding: DOC-04 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-02): de-drift the Style Guide's four stale normative sections Rewrites the sections DOC-01 verified as contradicting the settled standards: - Docstring format: the 'Markdown Docstrings for pdoc/pdoc3' section is replaced by the NumPy/Sphinx RST standard (worked example with :math: and Parameters/Returns/Examples/Notes; conversion tracked in docs/plans/docstring-conversion-plan.md), per Style Charter section 6. - Doc file format: Quarto .qmd prescription (zero .qmd files exist in the repo) replaced by MyST .md/Sphinx guidance matching CLAUDE.md; migration table row updated. - Data access examples: 'Preferred' coordinate examples now use the real, runnable API — mesh.X.coords (read), mesh.deform() (coordinate changes), and the swarm.coords getter/setter for particle positions. The previous 'Preferred' example swarm.data += displacement raises AttributeError (getter-only property — SWARM-13 evidence); mesh.data warns at runtime. The private-attribute migration advice (swarm._particle_coordinates, mesh._deform_mesh presented as the NEW pattern) is deleted. - Front matter: the 21-line Quarto YAML header is replaced by a minimal MyST title block, and the guide now states that the UW3 Style Charter is the normative contract and wins on conflict. All replacement examples verified against current source: Swarm.coords setter (swarm.py), Mesh.deform (discretisation_mesh.py:3133), uw.synchronised_array_update / NDArray_With_Callback.delay_callbacks_global. Findings: DOC-01, SWARM-13 (style-guide part). Underworld development team with AI support from Claude Code * docs(WE-03): regenerate the docstring review queue; add the sweep to the release checklist The queue (last generated 2026-01-13, cdf5bb2) misrepresented the codebase both ways: it flagged now-complete items (solve, SNES_Scalar) as missing and contained zero entries for the June 2026 API (DOC-02). Regenerated over src/underworld3/**/*.py + **/*.pyx at the current tip. Two bugs in scripts/docstring_sweep.py's regex-based Cython parser made the regenerated queue lie about .pyx docstrings and are fixed as part of making the regeneration meaningful: - the indent group '(\s*)' with re.MULTILINE consumed preceding blank lines, shifting the computed definition line so the docstring search started ON the def/class line and always missed; - the docstring search started at the definition line rather than after the (possibly multi-line) signature, so long signatures hid their docstrings; - raw-string docstrings (r""", the norm in the solver .pyx) were not recognised. DOC-02 cross-validation on the regenerated queue now passes: solve / SNES_Scalar in the solver pyx are no longer flagged 'none'; the queue contains the June API (add_nitsche_bc, add_rotated_freeslip_bc, boundary_flux, set_custom_fmg, consistent_jacobian: 13 mentions) and flags the DOC-05 targets (Swarm.advection x2, read_timestep, write_proxy) as undocumented. Also adds the sweep to the quarterly release checklist (guides/release-process.md) so the queue cannot go stale unnoticed again. Findings: DOC-02 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-05): backfill the changelog for May - early July 2026; add the changelog sweep to the release checklist The changelog (the quarterly CIG/stakeholder record) ended in April 2026 while ~117 first-parent commits landed May through early July (DOC-03). Backfilled at the existing conceptual granularity — 14 grouped entries, grouped by subsystem rather than by PR, matching the established format (### Title (Month Year), bold lead sentence, hyphen bullets, inline PR references): - New '2026 Q3 (July - September)' section: the July 2026 quality campaign (#309-#313, #317, #322-#326, #329, #334 as grouped entries), rotated strong free-slip / boundary traction / dynamic topography (#293, #294, #298, #306), generalized geometric multigrid via custom prolongation (#290, #297), consistent Jacobian tangent (#258), swarm correctness (#216, #313, #323, #329), numpy 2 support (#301, #305). - Extended '2026 Q2' section with the May-June entries: mesh adaptation movers (#190, #209, #213, #228, #259, #264, #266), moving-mesh field transfer / deform() (#246, #249, #251), semi-Lagrangian accuracy controls (#164, #183, #185-#189, #208, #220), snapshot/checkpoint toolkit (#146, #195, #196, #198), Stokes_Constrained (#224, #229, #240, #265), local-h Nitsche + boundary-slip surfaces (#225, #241, #275), units interoperability (#277, #278, #283, #284), memory/evaluation/solver infrastructure (#161, #177-#179, #181, #182, #222, #237, #250, ...). Every entry is backed by a merged commit on development (verified against git log --first-parent aed517f..3184a40). Also adds a quarterly-changelog sweep step beside the docstring sweep in the release checklist (guides/release-process.md) per DOC-03's proposed fix. Findings: DOC-03 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-06): status headers on the unmarked design docs (per-doc git verification) Adds one-to-three-line Status markers to the 13 design docs that lacked one, following the directory's existing conventions (**Status**: line under the title; status: key inside existing YAML frontmatter for the three frontmatter-only docs), and corrects the stale 'Design Phase' marker on MATHEMATICAL_MIXIN_DESIGN.md (the mixin ships in utilities/mathematical_mixin.py). Every stamp was verified against git history (git log --follow dates) and the current source tree before writing: - Implemented: jacobian-consistent-tangent (PR #258, c63cd70), fmg-checkpoint-hierarchy (3cd73cd), petsc-dmplex-checkpoint-reload-plan (PR #146, write_timestep(petsc_reload=True) in tree), fault-refinement-simplification (smooth_mesh_interior / metric_density_from_gradient / fault_comb_metric all in tree), MATHEMATICAL_MIXIN_DESIGN. - Current reference/contract: mesh-adaptation-formulation, ND_UNITS_BOUNDARY_CONTRACT (PR #278, e0ece9a). - Investigation records (preserved via PR #245, 34a9dd4; production geometric-MG is custom prolongation, PR #290): snesfas-feasibility, snesfas-vanka-feasibility-study. - Design notes / prototypes with honest gaps: in_memory_checkpoint_design (not implemented, per its own trailing Status section), submesh-solver-architecture (extract_region/extract_surface exist; coarsened_companion does not). - Historical: ARCHITECTURE_ANALYSIS (persistence.py layout superseded), COORDINATE_MIGRATION_GUIDE (transition shipped), WHY_UNITS_NOT_DIMENSIONALITY (decision record). The audit's ~16 estimate over-counted: re-derived at this tip, 13 docs were unmarked plus one marked-but-stale (DOC-07). Findings: DOC-07 (docs/reviews/2026-07/DOCS-STANDARDS-COHERENCE.md). Underworld development team with AI support from Claude Code * docs(WE-08): convert units.py public docstrings Google -> NumPy style Docstring-only conversion of the 18 public module-level functions that carried Google-style Args:/Returns:/Raises:/Examples: labels (check_units_consistency, get_dimensionality, get_units, non_dimensionalise, show_nondimensional_form, simplify_units, create_quantity, convert_units, to_base_units, to_reduced_units, to_compact, get_scaling_coefficients, set_scaling_coefficients, validate_expression_units, assert_dimensionality, validate_coordinates_dimensionality, enforce_units_consistency, require_units_if_active, convert_angle_to_degrees) to the NumPy/Sphinx standard (Style Charter section 6). dimensionalise was already NumPy style; one-line docstrings and private helpers are untouched. No code, signature, or behaviour changes (verified: every diff hunk is inside a docstring; ast.parse clean). Finding: API-12 (docs/reviews/2026-07/API-CONSISTENCY-REVIEW.md). Underworld development team with AI support from Claude Code * docs(WE-09): sweep call sites of the newer BC methods to value-first (conds, boundary, ...) order Wave C (#334) made the ORIGINAL value-first order canonical for add_nitsche_bc / add_rotated_freeslip_bc / add_constraint_bc (maintainer decisions D2/D3; Style Charter section 6) with deprecation shims for the legacy boundary-first and g= spellings. This sweep updates every call site of those THREE methods to the canonical order so nothing in the repository exercises the shims — 74 sites total: - tests/: 63 call sites across 12 files (test_1017, test_1018, test_1060, test_1061, test_1062, test_1064, test_1065 x2 serial; parallel test_1017, test_1062, test_1063, test_1064). tests/test_0641_wave_c_api_shims.py is deliberately untouched — its legacy-order calls ARE the deprecation contract. - docs/: 7 sites (curved-boundary-conditions.md x4, CONSTRAINED_FREESLIP_MULTIPLIER.md call + signature line, examples/submesh_investigation/test_region_ds_nitsche.py). - .claude/skills/: 3 sites (adapt-on-top-faults x2, free-surface-convection x1). - CLAUDE.md: 1 signature reference (free-slip BC preference section). The ~1,370 legacy-trio (add_dirichlet_bc/add_natural_bc/add_essential_bc) sites already conform and are untouched per the D2 decision. The audit review documents under docs/reviews/2026-07/ record the pre-decision state as evidence and are not swept. Discovered while verifying the swept tests run warning-free: the Wave C zero-datum guard in add_rotated_freeslip_bc rejects FLOAT zero (sympy.sympify(0.0) != 0 is structurally True), so the canonical add_rotated_freeslip_bc(0.0, boundary) raises NotImplementedError while conds=0 works. Filed as issue #336 with a TODO(BUG) marker at the guard (comment-only src touch); the swept call sites use the working integer form add_rotated_freeslip_bc(0, boundary). No fix applied here (Charter section 9 scope discipline). Findings: API-01/API-02 sweep (WE-09, REMEDIATION-WORKLIST.md). Underworld development team with AI support from Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restores the intended RBF metric-eval robustness for the mmpde mover (a design decision that had been undercut by an FE bake).
Problem
The mover's
metric_eval="rbf"path baked the metric via an FE evaluation at the fixed reference cloudref, then Shepard-interpolated. On a deformed mesh that FE evaluation mis-locates points and the P1 density — strictly positive by construction — comes back negative (measured ≈ −3, even at a field's own DOF coords). A negative density is a non-SPD metric, which the mover either NaN-bails on (a hidden stall) or, with the #259 SPD-floor, acts on as "coarsen hard here" → giant cells / holes / divergence in the adapted mesh.Fix
Bake the metric at the current mesh NODES (its own DOF locations) and Shepard-interpolate from there:
refis kept for the_edge_matsreference frame.Relationship to #264
Best paired with #264 (deformed-mesh point-location fix), which makes the nodal FE bake exact. With both, the adapted mesh stays clean under forced every-step adaptation at R=5 (folded=0, cell-area-ratio flat, vrms→18, Nu→1.86) where it previously tore holes (area-ratio spikes 189/96/875).
One-line source change; validated in a sibling worktree. CI runs the full mover suite.
Underworld development team with AI support from Claude Code