Issue quick-fix batch 2: six small independent fixes (#415 #447 #413 #411 #451 #414) - #456
Conversation
… P1 trace flux, residual-variant inserts, example scale, curved-boundary caveats Six small, independent fixes: - #415: UWexpression.is_comparable now returns False (plain-Symbol semantics). Advertising the wrapped contents' comparability made sympy.Max/Min attempt an immediate numeric comparison and crash on Float internals (_prec). Max/Min of an expression and a number now construct symbolically and resolve on unwrap, staying live to later constant ramps. - #447: constitutive Parameter assignment guards the two self-reference cycles: a read-back write (p = Params.x; Params.x = p) is a no-op instead of nesting the container in itself (RecursionError on the next .value), and a composite self-reference (Params.x = Params.x * 2) snapshots the current contents. - #413: 2D boundary-flux recovery detects the trace order from the reaction data (edge-midpoint DOFs present or not) and assembles the matching P1 two-node or P2 three-node line mass, lumped and consistent; mixed traces raise legibly. Previously P1 traces died with a bare KeyError. - #411: the two Stokes-only residual diagnostics (compute_volume_residual_fields and compute_boundary_residual_fields) get the same DMPlexInsertBoundaryValues insert as _assemble_volume_reaction (#407) — they still evaluated against zero-filled essential values and returned garbage on g != 0 walls. Regression test pins exact agreement with the validated core and fails on the pre-fix build. - #451: the Spiegelman notch example builds its gmsh geometry in non-dimensional units of H (S = 1.0) — building in km made the solved domain 30x too large in every direction. - #414: curved-boundary warning (P2 vertex values converge slowly; midpoints superconvergent) on boundary_flux, boundary_normal_traction, and dynamic_topography docstrings, with the measured numbers. Gate: level_1+tier_a green in the worktree; new tests shown to fail pre-fix (#415 AttributeError, #447 RecursionError, #411 divergence) where demonstrable. Underworld development team with AI support from Claude Code
Adversarial review — live probes on the batch worktreeVerdict: APPROVE with one scope caveat, addressed in a response commit. All six changes hold under attack. The caveat: the #447 guard as submitted covers the two direct cycles but not a self-reference nested inside a wrapper expression — and that hole's worst mode is silent garbage, not a crash. Hardened in the response commit (recursive cycle detection → legible FindingsMAJOR (scope) — deeper #447 cycle was unguarded — PROBED, fixed in response commit. MINOR — Matrix-valued composite snapshot produced malformed sympy — PROBED, fixed in response commit. NOTE — pre-existing P3 silent-wrong boundary flux — PROBED both sides, filed as a separate issue. A degree-3 trace returns finite but wrong flux (−0.57…−0.74 vs exact −1.0) — byte-identical on NOTE — doc touch-ups, fixed in response commit: stale "km" comment in the Spiegelman example (line ~123) contradicting the corrected framing; "Flat boundaries are pointwise exact" softened to "exact up to solver tolerance" (the tests assert 1e-3). Attacks that failed (evidence)
Adversarial review per project policy — Underworld development team with AI support from Claude Code |
…ix snapshot guard, doc touch-ups From the adversarial review of this PR: - The #447 guard now detects cycles RECURSIVELY through nested UWexpression contents (visited-set bounded walk): a self-reference hidden inside a wrapper expression — previously accepted, then either RecursionError at flux build or a silently ~2^50-scaled result from the depth-capped unwrap — is rejected with a legible ValueError, in both the reference branch and the plain-sympy branch. Cross-container cycles (a -> b -> a) are caught by the same walk. - Matrix-valued composite snapshot raises instead of xreplace-ing a Matrix into a matrix entry (malformed sympy stored silently). - Stale 'coordinate units (km)' comment in the Spiegelman example corrected; 'pointwise exact' softened to 'exact up to solver tolerance' in the boundary_flux warning. - New regression: wrapper-cycle assignment raises, parameter unmodified, flux still builds. Underworld development team with AI support from Claude Code
Summary
Six small, independent fixes, each with fail-first evidence where demonstrable. Closes #415, closes #447, closes #413, closes #411, closes #451, closes #414 (manual close on merge — PRs target development).
UWexpression.is_comparable→False(plain-Symbol semantics).sympy.Max/Min(expr, number)previously crashed on Float internals (_prec); now constructs symbolically, resolves on unwrap, stays live to constant ramps. Pre-fixAttributeErrorreproduced.RecursionErroron.value); compositeP.x = P.x * 2snapshots current contents. Pre-fixRecursionErrorreproduced.KeyErroron P1.compute_volume_residual_fields+compute_boundary_residual_fieldsget the #407DMPlexInsertBoundaryValuesinsert (withresidual_timeplumbing). Regression test pins exact agreement with_assemble_volume_reactionand fails on the pre-fix build.S = 1.0) — was 30× too large in every direction.Validation
tests/test_0104_expression_parameter_guards.py(3 tests);test_1019_boundary_flux.py+3 (P1 trace lumped+consistent, compute_volume_residual_fields / boundary sibling still strip Dirichlet values (the #407 fix was applied to one of three duplicated variants) #411 exact-agreement).level_1 and tier_a: 523 passed, 0 failed (17 skipped, 1 xfail).Adversarial review to follow on this thread before merge, per project policy.
Underworld development team with AI support from Claude Code