Skip to content

Return the whole boundary traction from the constrained solver (#607) - #617

Merged
lmoresi merged 4 commits into
developmentfrom
bugfix/multiplier-traction
Sep 3, 2026
Merged

Return the whole boundary traction from the constrained solver (#607)#617
lmoresi merged 4 commits into
developmentfrom
bugfix/multiplier-traction

Conversation

@lmoresi

@lmoresi lmoresi commented Aug 19, 2026

Copy link
Copy Markdown
Member

Closes #607.

Stokes_Constrained assembles the momentum row's boundary term as
(h + r(n·u − g))·n, so the traction holding the boundary is that sum.
multiplier() returns h, and topography() was built on h alone — short by
the augmented-Lagrangian share, which is r times the discrete constraint
residual.

With the viscosity-weighted default r = 1e4·μ(x) that share is a few per cent
of the surface traction on a uniform-viscosity annulus. Across SolCx's 1e6
viscosity step, where r reaches 1e10 on the stiff half, it is most of it:

route max abs(h) corr with exact relative l2
multiplier() as returned 0.042 −0.53 1.04
traction() (this PR) 0.382 +0.999 0.047
exact SolCx.topography_top 0.383
projected n·σ·n, same solve 0.383 +0.997 0.075

The corrected read is the most accurate surface stress available on that problem
— better than the projection — and the solve itself was never at fault
(velocity error 8.8e-06, max abs(n·u) 1.7e-10).

What changed

  • traction(boundary) — new, returns h + r(n·u − g) symbolically.
  • topography() — built on it, so the public dynamic-topography path is
    correct with no change at the call site.
  • multiplier() — unchanged behaviour (it returns h) and now documents what
    it is not.
  • Docstrings that claimed accuracy is independent of r now say the CONSTRAINT
    and the traction() read are independent of it, and h alone is not.

Why it survived validation

The existing check scored corr(λ, −n·σ·n) = 0.9999. A correlation is scale-free
and cannot see a systematic amplitude deficit, which is precisely what a missing
share of the load is. tests/test_1063_constrained_traction.py scores a relative
l2 against the exact SolCx topography and carries the bare multiplier as its
negative control (it must read > 0.5), so a regression that quietly reverts to
h cannot pass. test_1061's API test asserted topography == h/scale, which
pinned the defect; it now asserts the outcome.

The identity behind it

At convergence M_Γ(h + r(n·u − g)) balances the volume residual restricted to
the boundary, which is the CBF nodal load (Zhong, Gurnis & Hulbert 1993). So the
multiplier route and boundary_normal_traction on a rotated constraint are the
same computation; measured across two solves they agree to 3–5%, inside each
route's own error. This is also why the free-surface work was right to reject the
multiplier as returned and keep the rotated lid + CBF.

Documentation

  • CONSTRAINED_FREESLIP_MULTIPLIER.md: dated correction, corrected CBF identity.
  • docs/advanced/curved-boundary-conditions.md: the penalty free-slip recipe now
    uses mesh.boundary_normal rather than mesh.Gamma, with the measurement that
    a facet-normal penalty does not converge — velocity error 0.60 flat and stress
    error 0.21 → 0.26 under refinement, while the leak reads 1e-5 throughout. The
    projected-normal recipe is marked superseded.
  • CHANGELOG.md entry.

Tests

tests/test_1061, 1062, 1063, 1064, 1024: 26 passed, 2 xfailed.

Related, filed separately and NOT addressed here: #608 (the reaction reported at
a corner shared with an essential BC), #614 (boundary_flux returns ~1e12 on
Stokes_Constrained), #616 (rotated free slip does not reproduce the equivalent
component condition on a flat wall, 2e-3, with an exact linear solve on both
sides).

Underworld development team with AI support from Claude Code

The momentum row carries (h + r(n.u - g)).n, so the traction holding the
boundary is that sum. multiplier() returns h, and topography() was built on h
alone, which is short by the augmented-Lagrangian share: r times the discrete
constraint residual. With the viscosity-weighted default r = 1e4.mu that share is
a few per cent of the surface traction on a uniform-viscosity annulus, and across
SolCx's 1e6 viscosity step it is most of it -- h alone reads a tenth of the exact
topography and is ANTI-CORRELATED with it.

- traction(boundary) returns h + r(n.u - g), the quantity the CBF
  back-calculation recovers: at convergence M_Gamma(h + r(n.u - g)) balances the
  volume residual restricted to the boundary, which is the CBF nodal load.
- topography() is built on it, so the public dynamic-topography path is correct
  without a change at the call site.
- multiplier() still returns h and now says what it is not.

Why this survived validation: the existing check scored corr(lambda, -n.sigma.n)
= 0.9999. A correlation is scale-free and cannot see a systematic amplitude
deficit, which is exactly what a missing share of the load is. The new guard
scores a relative l2 against the exact SolCx surface topography (0.047 with the
share, 1.04 without) and carries the bare multiplier as its negative control, so
a regression that quietly reverts to h cannot pass.

test_1061's API test asserted topography == h/scale, which pinned the defect. It
now asserts the outcome: topography is the traction over the scale, and the
traction is not the multiplier.

Docs: the design note carries a dated correction and the corrected CBF identity;
the user-facing curved-boundary page now writes the penalty recipe against
mesh.boundary_normal rather than mesh.Gamma, with the measurement that a
facet-normal penalty does not converge (velocity error 0.60 flat, stress error
0.21 -> 0.26 under refinement, while the leak reads 1e-5); CHANGELOG entry added.

Underworld development team with AI support from Claude Code
Copilot AI lite review requested due to automatic review settings August 19, 2026 09:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…614)

build_rotation skipped any boundary node with a velocity DOF constrained out of
the global vector -- which is every node where a rotated wall meets a wall held by
an essential condition. The node was dropped from the rotation entirely, so the
WALL-NORMAL COMPONENT WAS NEVER CONSTRAINED THERE. The wall leaked at its own end
points while every interior node was exact.

Measured, unit box, rotated lid, component free slip on the other three walls,
uniform viscosity: max|u_y| on the lid was 4.0e-3 against |u|max 2.5e-2 -- 16%,
entirely at the two corners -- and the solve differed from the equivalent
component-Dirichlet lid by 2.0e-3 globally, with an EXACT LINEAR SOLVE on both
sides, so it was never a convergence artefact. After the fix the lid holds u.n to
0.0 and the two solves agree with a direct reference to 8.9e-9.

The fix: a partially constrained node keeps the constraint on what is left. With
the pinned components at zero, n.v = 0 reduces to n_F.v_F = 0 on the free
subspace, so the frame is built there and its normal rows constrained. A node
whose normal lies entirely in the pinned subspace is already implied and is still
skipped. A node carrying a prescribed v_n datum keeps the old behaviour and says
so once -- reducing the affine constraint against the pinned values is not
implemented.

This also removes the mechanism behind #608: the corner reaction was the
essential constraint's because the rotated constraint was not there.

boundary_flux() now raises on a multiplier-constrained boundary instead of
returning a quiet ~0 (#614). There is no reaction to read there: the constraint
enters the same row it constrains, so the assembled residual is balanced at
convergence (measured rms 3.9e-13 against a traction of 0.37). The traction is
the multiplier, and traction() returns it. My original report of that issue said
1e12; that was a ratio with a near-zero denominator, and the issue is corrected.

Guards: tests/test_1066_rotated_meets_essential.py asserts the lid holds u.n to
round-off and that both lids reproduce a monolithic direct solve. Existing suites
unaffected: 42 rotated/nitsche/constrained tests pass.

Underworld development team with AI support from Claude Code
@lmoresi

lmoresi commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Extended to cover the other two paths, so the fix lands as one consistent set
rather than leaving them to follow.

The corner was never constrained (#616, and the mechanism behind #608)

build_rotation skipped any boundary node with a velocity DOF constrained out of
the global vector — every node where a rotated wall meets a wall held by an
essential condition. The node was dropped from the rotation, so the wall-normal
component was never constrained there
. The wall leaked at its own end points
while every interior node was exact.

Unit box, rotated lid, component free slip on the other three walls, uniform
viscosity:

before after
max abs(u_y) on the lid 4.0e-03 (16% of abs(u)max, both corners) 0.0
vs a monolithic LU solve of the component-Dirichlet problem 2.0e-03 8.9e-09
same, with _rotated_use_lu on both sides 2.1e-03 8.9e-09

The second row with an exact linear solve on both sides is the point: it was
never a convergence artefact, it was a different discrete problem.

The fix: a partially constrained node keeps the constraint on what is left. With
the pinned components at zero, n·v = 0 reduces to n_F·v_F = 0 on the free
subspace, so the frame is built there and its normal rows constrained. A node
whose normal lies entirely in the pinned subspace is already implied and is still
skipped; a node carrying a prescribed v_n datum keeps the old behaviour and says
so once (reducing the affine constraint against the pinned values is not
implemented).

Guard: tests/test_1066_rotated_meets_essential.py. Existing suites unaffected —
42 rotated / Nitsche / constrained tests pass.

boundary_flux on a constrained boundary (#614) — and a correction

I reported that it returns ~1e12. It returns ~0 (rms 3.9e-13 against a
traction of 0.37); the 1e12 was a ratio whose denominator was that near-zero. And
it is expected: a multiplier-constrained boundary has no reaction left to read,
because the constraint enters the same row it constrains. That is the #607
identity from the other side.

It now raises and points at traction() rather than handing back a quiet zero.

Issue #614 is corrected on the issue itself.

lmoresi and others added 2 commits September 3, 2026 08:25
The only conflict was an append collision in docs/developer/CHANGELOG.md --
this branch's traction entry and #649's recovery-mass entry both land under
2026 Q3. The file is newest-first, so the traction entry goes above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
@lmoresi
lmoresi merged commit a9945bc into development Sep 3, 2026
2 checks passed
lmoresi added a commit that referenced this pull request Sep 3, 2026
…inting at traction()

#617 landed its own inline refusal for the same case while this branch was
open, so the merge had two guards for one defect. Kept this branch's named
helper -- it carries the measurement in its docstring and it is the one with a
test (test_1019 asserts NotImplementedError and holds a negative control that
the Dirichlet boundary of the SAME solver still recovers something with spatial
content).

Took #617's correction to where the caller is sent. The message pointed at
solver.multiplier(), which is exactly the quantity #617 showed to be incomplete:
it returns h alone, short by the augmented-Lagrangian share r(n.u - g). It now
points at solver.traction(), and says what multiplier() is missing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
lmoresi added a commit that referenced this pull request Sep 3, 2026
The consistent-boundary-flux back-calculation reads the VELOCITY residual. On a
boundary held by `add_constraint_bc` the traction has been moved into the
multiplier block, so the velocity rows retain only a CONSTANT traction, and
de-smearing hands that constant straight back.

That is not an inaccuracy, it is a different quantity. Measured on SolCx, the
raw reaction along the boundary took exactly two magnitudes in the ratio
4 : 2 : 1 at midpoint / interior vertex / end vertex -- the P2 line-trace lumped
mass, i.e. R = c*m_i to the digit -- so the recovered flux had a standard
deviation of 2e-15 across 63 nodes where the equivalent `Stokes` solve varied
correctly and correlated -0.997 with the exact topography.

The issue reported this as ~1e12, which is no longer what happens; today it
returns a plausible O(0.1) number that happens to carry no spatial information
at all. That is worse, not better: 1e12 announces itself and 0.11 does not, and
a caller taking a mean or a peak off it gets something entirely reasonable
looking.

Nothing here is corrupted -- the reaction assembly and the de-smear both do
exactly what they are told. The primitive reads a place that, on this solver,
no longer holds the quantity. So this refuses instead, naming multiplier() and
topography(), which is where the traction actually is.

The guard is on the BOUNDARY, not the solver: the regression checks that the
Dirichlet boundary of the same constrained solve still recovers, and that what
it recovers has spatial content rather than being constant too.

This is the cheap half. The full fix is for the traction accessor to DISPATCH
on how each boundary is held -- rotated reaction, multiplier, or CBF -- so a
caller does not have to know. The solver already has what it needs to decide
(`_block_constraint_bcs`, `_rotated_freeslip_info`); what is missing is a common
return shape, since multiplier() returns a field where the others return nodal
arrays. That belongs with #607/#617, which is already making the constrained
route return the whole traction.

Underworld development team with AI support from Claude Code

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants