Uh oh!
There was an error while loading. Please reload this page.
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
lmoresi
commented
Jul 28, 2026
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
Uh oh!
There was an error while loading. Please reload this page.
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