Skip to content

Navier-Stokes with Eulerian SUPG momentum transport, and a partition-independent cell size (#687) - #688

Merged
lmoresi merged 55 commits into
developmentfrom
feature/navier-stokes-supg
Sep 8, 2026
Merged

Navier-Stokes with Eulerian SUPG momentum transport, and a partition-independent cell size (#687)#688
lmoresi merged 55 commits into
developmentfrom
feature/navier-stokes-supg

Conversation

@lmoresi

@lmoresi lmoresi commented Sep 5, 2026

Copy link
Copy Markdown
Member

uw.systems.NavierStokesSUPG: the incompressible Navier-Stokes equations on the Stokes saddle-point solver with the momentum advection assembled implicitly and stabilised by the vector SUPG term, the counterpart of the scalar Eulerian solver in #673 (this PR stacks on that branch). Crank-Nicolson at order 1, BDF2 at order 2, the velocity history on the mesh; no stress history, the viscous stress at an earlier level is rebuilt from the stored velocity through the constitutive model, and pressure has no history. The advecting velocity is a choice: the second-order extrapolation 2uⁿ − uⁿ⁻¹ (one linear solve per step, the default), Picard passes on the latest iterate, or the unknown itself under Newton.

What the measurements say (design note, "Navier-Stokes with SUPG momentum transport"; study directory ~/+Simulations/navier_stokes_supg/):

  • Kovasznay flow at Re 40: SUPG 6.6e-4, 2.6e-4, 6.7e-5 at h = 1/16, 1/32, 1/64; Galerkin (τ = 0) 1.1e-4, 1.6e-5; the semi-Lagrangian solver 5.8e-3, 2.9e-3 at seven times the cost per step. At this Reynolds number the stabilisation is not needed and costs accuracy; the missing viscous term in the first-derivative residual is the O(h²) consistency error. Newton, Picard and Courant 4 give the same steady state, as they must.
  • Lid-driven cavity: Re 100 within 4% of Ghia on a 1/32 mesh at a fixed point; Re 400 at 1/48 within 6% (the mesh), where the extrapolated step at Courant 2 keeps a localised alternating mode of the lagged coefficient that one Picard pass, or Courant 1, removes; Re 1000 with geometric multigrid on the velocity block steps at one Krylov iteration per step but needs a longer run than was made here.
  • Cylinder wake (DFG 2D-2, Re 100): Strouhal number 0.298 against the reference 0.295 to 0.305, the two fully implicit forms agreeing to three digits and the extrapolated step within 1% in frequency; the lift peak 11% low and the drag 23% low on the finer mesh, a deficit that does not close with resolution and points at the stabilisation's diffusion (the tau weights are the scalar solver's, untuned); the Galerkin control cannot run at this element Reynolds number, which is what the term is for. The semi-Lagrangian solver on the same mesh has the shedding 13% too slow at three times the cost.

Found and fixed on the way

Tests: the API contract (construction rules, one linear solve per step, Picard passes, the Stokes limit, runtime-constant timestep and theta), a two-rank Kovasznay error that matches serial to 1e-7, the scalar parallel reference re-recorded for the new cell size, and the Nitsche local-h tests reading the field's definition.

Deferred, recorded in the note: a recovered Laplacian to complete the residual for P2 velocity; the time-dependent (Cahouet-Chabard) pressure Schur approximation for large ρ|a|Δt/η; vector and tensor unknowns for the scalar solver and the viscoelastic stress equation, which is where the DDt V_fn contract gets generalised.

Underworld development team with AI support from Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL

Closes #701 (the DDt timestep helper non-dimensionalises a quantity; fixed in 4f65d0f).
Closes #696 (a UWexpression no longer folds on its current value; 92ed8ac). Closes #702 (never-populated swarm advection is a clear error; 15da74b).

lmoresi and others added 19 commits September 2, 2026 17:29
A solver that assembles its own weighted sum of history terms (an Eulerian
scheme applying a multistep rule to a spatial operator) needs the
constants-routed coefficient expressions, not just their current values.
Read-only accessors; no behaviour change.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…vars

A MeshVariable that is dropped and garbage-collected (the default Model
holds the only strong reference; uw.reset_default_model() releases it, and
the statistics helpers delete temporaries deliberately) leaves its PETSc
field in the DM. Mesh.update_lvec zipped mesh.vars.values() against the
field decomposition by position, and the JIT's petsc_a[] offsets were a
running count over the live variables, so every later variable was packed
into, and read from, the wrong slots. Measured: a P0 cell-size field landing
in a P2 slot as garbage, NaN residuals in one run and a subtly wrong answer
in the next, depending on when the collector ran.

update_lvec now packs by field name and zeroes an orphaned field; the JIT
reads component offsets from the DM's own field list and patches each
variable from its field_id. Regression test: 2 of its 3 checks fail without
the fix.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…r for scalar variables

A Gaussian carried round the origin by rigid rotation while diffusing is
exact at every time (rotation commutes with the Laplacian), so a transport
scheme's error can be measured directly and the round trip after one
revolution is an absolute check. AnalyticSolution.error(norm='integral')
added a 1x1 Matrix symbol to a scalar expression and had never been
exercised on a scalar variable.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…fusion solvers

The cell-crossing / diffusion-time reduction (isotropic or direction-aware,
minimum or percentile) becomes a module-level helper so the Eulerian
solver can call it rather than carrying a copy. SLCN behaviour unchanged.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…nditioner

A solver with no managed option block (_pc_option_prefix is None) sets its
own PC; installing the adapt child's PCMG hierarchy on it segfaulted inside
PETSc (additive-Schwarz PC, PCMG calls). The gate now treats that state as
the explicit choice it is, alongside preconditioner='gamg' and the user
override latch.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…from the symbolic history

uw.systems.AdvDiffusionSUPG(mesh, T, V_fn, order=N, integrator='bdf'|'am')
assembles the implicit weak form from the Eulerian DDt history: the BDF
stencil or the Adams-Moulton weights on the advective and diffusive terms
at every stored time level, plus the SUPG flux tau R u with the strong
residual of the same scheme. Timestep, multistep coefficients and the tau
weights are runtime constants of the compiled kernels, so a change of dt
costs nothing (the issue #657 prototype recompiled on every change).
Diffusivity comes from the constitutive model like every scalar solver.

Measured on the rotating Gaussian: stable at any cell Courant number, error
set by u dt against the feature width (dt^2 for the second-order schemes),
unchanged to three digits by a band refined to h/9 at local Courant 13;
Crank-Nicolson reproduces the prototype's numbers to four digits.
Tests: API and no-recompile contract, temporal convergence (slopes 0.8/0.9
for BDF1, 1.9 for BDF2), band invariance, round trip, np=2 = serial.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…om the integrator study

Rotating-Gaussian study at res 32, Courant 0.25 to 8, pure advection and
kappa 1e-3: Adams-Moulton above order 1 blows up from Courant 1 (bounded
stability region), BDF3 fails from Courant 4, Crank-Nicolson is three to
four times more accurate than BDF2 at the same timestep but rings once the
feature is under-resolved in time, backward Euler carries 20-40% error at
any practical timestep. Cost per step is the same for every scheme. BDF2 is
the robust default; the note records the alternatives and when to pick them.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…Courant number

BDF2 and Crank-Nicolson track their res-32 errors at the same u dt. BDF3's
stability region misses the imaginary axis near the origin, so the
low-frequency modes of a finer mesh grow: 31x the exact field after 590 steps
at Courant 1. Safe only with diffusion, below Courant 2. Note and docstring
updated; the BDF2 default stands.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…-in replacement

The constructor, order, theta, f, V_fn, constitutive_model, delta_t,
estimate_dt and solve keep the meaning they have for AdvDiffusionSLCN, so a
script changes the class name and nothing else. order=1 with theta=0.5 is
Crank-Nicolson and the default, as for SLCN; order=2 takes theta=1 (BDF2)
unless 0.5 is asked for explicitly, which is refused for the reason the SLCN
documentation gives. The trace-back-only arguments (restore_points_func,
monotone_mode, old_frame_traceback, DFDt) are accepted and ignored with a
warning. integrator is inferred and only needs setting to reach the higher
Adams-Moulton rules. delta_t is settable and solve() reuses it; the notebook
viewer reports the scheme. User page docs/advanced/eulerian-advection-diffusion.md
with the swap table and the when-to-use-which guidance.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…e scheme

The only schemes the argument added were Adams-Moulton at orders 2 and 3,
which the integrator study shows blowing up on advection from Courant 1.
The multistep family now follows the order (the theta rule at order 1, BDF
above); the higher Adams-Moulton assembly stays in the code, reachable only
by switching the family on the instance, which is how the study measured it.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…the module and note

estimate_dt now returns the step at which the field changes by a fraction
(0.02) of its range: from the advective rate |u . grad phi| at the vertices
before the first solve, and from the rate the last step actually produced
after it. The cell-crossing time the semi-Lagrangian solver reports is not
a stability limit for this scheme and says nothing about its accuracy; it
stays available as basis='resolution'. The estimate is mesh-independent,
which the band test now checks (the resolution estimate collapses 3x on
the refined child, the accuracy estimate moves under 25%), and at the
default fraction Crank-Nicolson completes the rotating-Gaussian round trip
under one per cent. The advective rate uses the vertex Clement gradient
rather than a point evaluation of a derivative expression, which fails on a
mesh carrying many variables.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…oner="fmg" a real switch on the SUPG solver

The Eulerian SUPG step took two Newton iterations on a linear operator:
the Krylov default (rtol 1e-5) does not reach the SNES tolerance (1e-8),
and the second Jacobian assembly cost more than every linear solve of
the step. The Krylov tolerance is now 1e-9 and a step is one Newton
iteration: 1.54 s to 0.91 s per step at 256^2 in serial.

Measured against geometric multigrid at matched tolerances (design note,
"Preconditioner"), GMRES with additive-Schwarz ILU is the cheaper linear
solve at every Courant number from 1/2 to 32 and its iteration count is
the same on one and eight ranks; the multigrid's cycle count grows with
the Courant number nearly as fast, and a cycle costs about three Schwarz
iterations. Schwarz stays the default on every mesh.

preconditioner = "fmg" now hands the block to the managed multigrid
route (custom-P transfers over the refinement hierarchy or an adapt
child's coarse tail, flexible GMRES outside) for the rank count where a
one-level method runs out of coarse space. The solver's solve() builds
through the base _build, where a preconditioner choice is resolved; the
pre-run of the three setup stages marked the solver set up first, so the
request was silently inert. The semi-Lagrangian solvers share that
pattern and the defect (#683).

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…lows

The shipped convection examples set adv_diff.theta = 0.5 after building
the solver; the Eulerian drop-in refused it. The blend is a runtime
constant refreshed from the history manager before every solve, so the
setter updates it without a recompile (order 1 only, the constructor's
rule). Vector and tensor unknowns join the design note's deferred list:
the solver is scalar, where the semi-Lagrangian trace-back carries them.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…independent cell size

uw.systems.NavierStokesSUPG: the incompressible Navier-Stokes equations
on the Stokes saddle-point solver with the momentum advection assembled
implicitly and stabilised by the vector SUPG term F1 = tau R (x) a, the
counterpart of the scalar Eulerian solver. Crank-Nicolson at order 1,
BDF2 at order 2, with the velocity history on the mesh; no stress
history, the viscous stress at an earlier level is rebuilt from the
stored velocity through the constitutive model. The advecting velocity
is a choice: the second-order extrapolation 2u^n - u^{n-1} (one linear
solve per step, the default), Picard passes on the latest iterate, or
the unknown itself under Newton. The strong residual the SUPG term sees
carries the pressure gradient; without it the term is O(1) at the exact
solution and costs fifty times the Galerkin error on Kovasznay flow.

mesh.cell_size() now reports each cell's own radius, the RMS distance of
its vertices from its own centroid, taken from the DM's coordinates. The
kd-tree radius it used to copy picks the nearest centroid among the
rank's cells, so the field differed with the partition (#687, found
because the two-rank Navier-Stokes answer differed from serial by 5e-4
and matched to 1e-15 with a constant h); after a deform it also read
stale vertex coordinates against fresh centroids. get_min_radius and
the other consumers of the kd-tree radii are unchanged.

Tests: the solver's API contract (construction rules, one linear solve
per step, Picard passes, the Stokes limit, runtime-constant timestep and
theta), a two-rank Kovasznay error that matches serial to 1e-7, the
scalar parallel reference re-recorded for the new cell size, and the
Nitsche local-h tests reading the field's definition.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
… section with Kovasznay and cavity results

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
@lmoresi

lmoresi commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Adversarial review (self), findings and evidence.

  1. The SUPG residual is incomplete for P2 velocity. It carries the time derivative, advection, pressure gradient and body force, not the viscous term. On Kovasznay this is the whole gap to Galerkin (6.6e-4 against 1.1e-4 at 1/16, converging at 1.4 to 2.0 against 3) and it will bias any smooth, resolved flow the same way. We ship it because the term needs second derivatives the kernels do not provide and the stabilisation is meant for convection-dominated cells; a recovered Laplacian is the fix and is on the list. Users at low element Reynolds number should set supg_weight = 0.
  2. The theta rule's level-n stress is exact for constant viscosity only. It is formed as 2η ε̇(uⁿ) with the current effective viscosity, so a strain-rate dependent viscosity gets the wrong level-n stress at order 1. Documented; BDF2 avoids it. Not measured.
  3. The extrapolated default has a failure mode, and we have seen it. Cavity Re 400 at Courant 2: a localised alternating mode of the lagged coefficient at the lid singularity, saturating at 0.1 in the max norm while the centreline values sit still. One Picard pass or Courant 1 removes it. The default stays extrapolated because Courant 1 is where the scheme is accurate anyway, but a user at Courant 2 and element Reynolds number 8 will meet this.
  4. Every linearisation gives the same steady state, so Kovasznay and the cavity cannot rank them. Only the cylinder wake did: fully implicit forms agree to three digits, the extrapolated step is 1% off in frequency and 8% on the lift peak at Courant 1. One time-dependent case, one mesh; more would be better.
  5. The cylinder drag is a quarter low and does not close with the mesh (28% at resolution 20, 23% at 40 with the cylinder cells at 1/160), while the Strouhal number and pressure difference sit on the reference. The likely cause is the streamline diffusion at the scalar solver's tau weights, untuned for momentum; the Galerkin control that would isolate it cannot run at this element Reynolds number (four Newton iterations and 22 s for the first step, the second never finishing). The drag against tau is the open measurement and the first thing to do after this lands.
  6. The Re 1000 cavity is not a converged comparison: 1200 steps to t = 19 at 89 to 91% of Ghia's extrema and the secondary vortex not yet formed. It shows the solver stepping at one Krylov iteration per step with geometric multigrid at element Reynolds number 16, nothing more. Two earlier attempts failed for driver reasons (a collective inside a rank-0 branch; the hang watchdog on silent ranks), not solver reasons, and the note says so.
  7. cell_size() changed meaning for every consumer: the own-centroid radius is 5 to 15% larger than the kd-tree radius on irregular meshes. The Nitsche penalty scaling and the scalar SUPG τ move by that much; test_1077's serial reference moved by 4e-7 relative and was re-recorded, test_1065 now reads the new definition. get_min_radius is untouched. Anyone with a tuned Nitsche γ on an irregular mesh will see a small change.
  8. Parallel coverage is two ranks on a small mesh. Four ranks ran the cavity and cylinder without incident but no four-rank number was checked against serial.

…693)

The velocity evaluated for the timestep estimate has shape (0, 1, dim) on
an empty rank, and reshape(0, -1) cannot infer the trailing size; the
empty-rank handling a few lines below never ran. Give reshape the size
explicitly. Found with passive tracers released at the inlet of the DFG
cylinder on four ranks, where every rank but the inlet's is empty at the
first step.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
@lmoresi

lmoresi commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

Added 9d3aa5f: Swarm.advection failed on any rank holding no particles (#693, the reshape of an empty velocity array in estimate_dt), found when releasing passive tracers at the inlet of the cylinder benchmark on four ranks. One-line fix, no behaviour change for non-empty ranks.

lmoresi and others added 7 commits September 5, 2026 18:47
…lues reach the kernels (#695)

uw.maths.Integral, BdIntegral and CellWiseIntegral compile their integrands
through the same JIT as the solvers, which routes every
uw.function.expression to PETSc's constants array, but none of them ever
called PetscDSSetConstants: the kernels read zeros, so any integrand with a
viscosity, a time or another expression in it integrated to nothing, and a
fresh Integral returned the same zero from the cache. Found on the DFG
cylinder drag, where the viscous traction (eta is an expression) vanished
and the drag read 23 to 28% low on two meshes without moving with the SUPG
weights.

Each class now packs the manifest and sets the constants right after the
objective; the boundary integral sets them on its sandbox DS, which has its
own discrete system. Regression test test_0503 covers the three classes, a
changed value without recompilation, and the constitutive-flux traction
that found it.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…, the vortex-decay benchmark, and #696

The DFG cylinder section is rewritten around what the tau sweep found: the
stabilisation moves the drag by 2.6% and the deficit was the boundary
integral dropping the viscous part (#695). With the integrals fixed and only
the cylinder cells refined through gmsh at a fixed time step, drag, pressure
difference and Strouhal number converge onto the reference bands on the 1/20
channel mesh, the traction and reaction measurements close on each other,
and the whole-mesh 1/40 run buys less than the 1/320 cylinder cells do. The
Galerkin form that "could not run" was the GAMG fallback; the refinement
callback gives FMG on the gmsh mesh.

New Taylor-Green vortex-decay subsection (dt and h sweeps for CN and BDF2,
Galerkin against SUPG, the viscosity range, the advecting-velocity choices),
and the two defects it found: #695 and the zero-valued expression folding
(#696, raised, not patched).

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…rough the circle callback)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…overshoot as the extrapolation lag

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…rd supersedes the branch's reshape fix

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…s run with #680

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
… four ranks)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
lmoresi and others added 3 commits September 6, 2026 21:56
…nded cell_size (#692) over the branch's #687 version

Same definition (RMS distance of a cell's vertices from its own centroid), one implementation.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…pped; Picard reductions on every pass

The merge of development took discretisation_mesh.py and test_1065 wholesale from
development, losing the orphaned-field packing by name (test_1058) and the rest of
the branch's non-conflicting edits; this is the hunk-by-hunk resolution with the
landed cell_size (#692). The Picard loop of the Navier-Stokes solver now takes its
two reductions on every pass, and the break predicate is recorded as rank-uniform
in the collective-guard scan.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…e, platform-tolerant comparison to the serial error

The GAMG fallback on a mesh without a hierarchy gave a platform-dependent answer
(7% on the Linux CI); the test now refines a 1/4 mesh once so the velocity block
runs geometric multigrid, and the serial reference (0.00132279) is met by two and
four ranks to 3e-10. Both tests compare to the serial error at 1e-6 relative:
the partition effect they guard against was 5e-4 (#687), platforms differ at 1e-7.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
@lmoresi
lmoresi changed the base branch from feature/eulerian-supg-transport to development September 7, 2026 05:09
@lmoresi

lmoresi commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Retargeted to development. This branch is a strict superset of #673 (no commit of feature/eulerian-supg-transport is missing here), so it carries the scalar SUPG solver, the DDt transport plugin, the Navier-Stokes solver and the naming (AdvDiffusion / NavierStokes are the composing solvers, *SLCN the semi-Lagrangian classes) as one change; #673 is closed as superseded. development is merged in (the landed cell_size of #692 replaces the branch's #687 version, same definition), the Picard loop's reductions are unconditional (test_0052), and the two-rank tests use a refinement hierarchy and a platform-tolerant comparison (they were red on CI at the 1e-8 and GAMG-fallback level); serial, two and four ranks agree to 3e-10. The #695 integral fix is also up separately as #700 for development.

🤖 Generated with Claude Code

https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL

From three reviews of the branch head (findings posted on #688):
- the base contract's shape helper collided with Symbolic's `_shape` attribute,
  so Symbolic.advection() raised instead of answering zero; renamed;
- a user-supplied EulerianSUPG on NavierStokes advected the stored level with
  the new velocity: the solver now sets V_fn and V_fn_history whoever built
  the manager, and its advection setter only steers such a manager;
- the change-rate bookkeeping read the manager's history `.array`, which fails
  for a SemiLagrangian history under units and for a swarm-backed history; it
  now diffs a copy of the unknown's data;
- a supplied manager silently overrode `order`/`theta`; a mismatch is an error,
  and the theta setter refuses a manager without theta;
- the 1-D tau shapes divided by the diffusivity (zoo at the manager's default);
- the timestep and SUPG knobs are created with unique names like the BDF
  coefficients, so they do not accumulate in the persistent registry;
- a bare scalar residual is accepted by stabilisation_flux.
Rename loose ends: an example that imported the bare NavierStokes name now uses
NavierStokesSLCN explicitly; tutorial 9 prose; the solver-unification design
table; API entries for the composing classes and the manager.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
@lmoresi

lmoresi commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Adversarial review of the branch head (three reviewers by area: the DDt plugin, the composing solvers and their tests, the development merge and the rename). Findings, evidence, and what was done.

Defects found and fixed (commit after dd17f95):

  1. Symbolic.advection() raised TypeError because the base contract's _shape() helper was shadowed by the attribute Symbolic._shape; the helper is renamed. Every history flavour now answers zero.
  2. A user-supplied EulerianSUPG on NavierStokes advected the stored level with the new velocity (V_fn_history was set only on the default path), a different scheme from the default and a u^{n+1} dependence in the Jacobian of the history term. The solver now sets the advecting velocity and the level carriers whoever built the manager.
  3. The change-rate bookkeeping in both solvers read the manager's psi_star[0].array: with a SemiLagrangian manager under a units model that raised, and with a swarm-backed manager it broadcast particle-shaped data against mesh dofs. It now diffs a copy of the unknown; the drop-in solves under units and with a Lagrangian_Swarm history.
  4. A supplied manager silently overrode the order/theta arguments; a mismatch is now an error, and the theta setter refuses a manager without theta.
  5. tau() divided by the diffusivity in the two 1-D shapes (zoo at the manager's default of zero); guarded.
  6. delta_t and the SUPG knobs were registered as persistent named expressions (never collected, and shared on a repeated name); they are created with unique names like the BDF coefficients.
  7. stabilisation_flux accepts a bare scalar residual.
  8. Rename: Ex_Navier_Stokes_Lid_Driven_Flow_2d.py imported the bare NavierStokes name and had flipped meaning; it now uses NavierStokesSLCN. Tutorial 9 prose, the solver-unification design table, and the API page (entries for the two composing classes and EulerianSUPG) corrected.

Verified and unchanged: the composed residuals of both solvers are symbolically identical to the pre-refactor ones at uniform SUPG weight for orders 1 and 2 and both advection modes (at the default Péclet weight 4 the flux carries the weight factor, by design, and the recorded rows were re-run with it). The merge with development is exactly the automatic merge with development taken in the seven cell-size conflict hunks, every branch-side change present and nothing from development lost. The advection switch to "implicit" puts the unknown in the manager and Newton converges quadratically. test_1078 runs full geometric multigrid on the velocity block on two ranks with the error at 2e-13 of the serial reference.

Known and not patched here:

  • _DDtBase._dt stores the magnitude of a dimensional step; the composing solvers non-dimensionalise before writing it, but a manager driven by an SLCN solver with a Pint step carries an unscaled delta_t. The root is the existing TODO(BUG) on uw.non_dimensionalise receiving a raw quantity in ddt._as_float; to be raised as an issue.
  • The repointed generic names change defaults for old scripts: the old NavierStokes was order 2 with discontinuous pressure and accepted flux_order/DFDt; the new one is order 1 with continuous pressure and refuses them. Scripts that need the semi-Lagrangian class use NavierStokesSLCN/AdvDiffusionSLCN. The published post on symbolic time derivatives shows the generic name with the semi-Lagrangian meaning and will need its examples updated at release.
  • Restoring a pre-solve snapshot leaves the kernel delta_t at its last value until the next update_pre_solve (which rewrites it).

🤖 Generated with Claude Code

https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL

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.

🟡 Changes recommended

There are at least two correctness issues in the new/modified code (Navier–Stokes estimate_dt() unit semantics and shared mutable bcs list storage) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Introduces Eulerian, fully implicit SUPG transport for both scalar advection–diffusion and incompressible Navier–Stokes by composing solvers from a DDt “transport manager” (history + assembled advection + stabilisation), while also fixing several core infrastructure issues uncovered by the new workflows (partition-independent per-cell size, robust aux-field packing after dropped MeshVariables, and JIT constant propagation into integral kernels).

Changes:

  • Add composed Eulerian SUPG solvers: uw.systems.AdvDiffusion (scalar) and uw.systems.NavierStokes (momentum), powered by a new ddt.EulerianSUPG transport manager with runtime-constant timestep/weights.
  • Fix correctness bugs affecting stabilisation/integrals and auxiliary-field layout: partition-independent mesh.cell_size(), stable aux packing when DM fields outlive Python variables, and PETSc DS constants set for integrals.
  • Update/expand tests, examples, and docs to reflect new solver exports and to validate parallel/temporal-order/contract behavior.
File summaries
File Description
tests/test_1110_advDiffAnnulus.py Switch test to AdvDiffusionSLCN naming
tests/test_1100_AdvDiffCartesian.py Switch test to AdvDiffusionSLCN naming
tests/test_1100_advdiff_supg_rotating_gaussian.py New accuracy/behavior tests for Eulerian SUPG scalar transport
tests/test_1058_dropped_meshvariable_aux_layout.py New regression tests for aux-field packing after dropped MeshVariables
tests/test_1057_ddt_transport_plugin.py New contract tests for DDt “transport plugin” behavior/shapes
tests/test_1056_navier_stokes_supg_api.py New API/contract tests for Eulerian SUPG Navier–Stokes
tests/test_1055_advdiff_supg_api.py New API/contract tests for Eulerian SUPG AdvDiffusion
tests/test_0820_template_parameter_propagation.py Update AdvDiffusion solver reference to SLCN
tests/test_0650_recursion_prevention_regression.py Update AdvDiffusion solver reference to SLCN
tests/test_0610_navier_stokes_slcn_projection.py Update NavierStokes solver reference to SLCN
tests/test_0506_tensor_evaluate.py Update NavierStokes solver reference to SLCN
tests/test_0503_integral_expression_constants.py New regression tests for expression constants in integrals
tests/test_0200_solver_smoke.py Update AdvDiffusion solver reference to SLCN
tests/test_0052_collective_guard_scan.py Add collective-safety reasoning for NavierStokes Picard loop
tests/test_0008_snapshot_realsolver.py Update AdvDiffusion solver reference to SLCN
tests/test_0006_memory_leak.py Update AdvDiffusion solver reference to SLCN
tests/parallel/test_1078_navier_stokes_supg_parallel.py New parallel partition-independence test for NS SUPG
tests/parallel/test_1077_advdiff_supg_parallel.py New parallel partition-independence test for AdvDiff SUPG
src/underworld3/utilities/custom_mg.py Prevent MG hierarchy pickup when solver owns its PC (_pc_option_prefix is None)
src/underworld3/utilities/_jitextension.py Fix aux component offsets using DM field list (handles orphaned DM fields)
src/underworld3/systems/solvers.py Factor shared adv/diff resolution dt + shared dt dimensionalisation
src/underworld3/systems/navier_stokes_eulerian.py New composed Eulerian SUPG Navier–Stokes solver implementation
src/underworld3/systems/ddt.py Add transport-manager contract methods + new EulerianSUPG manager
src/underworld3/systems/advection_diffusion_eulerian.py New composed Eulerian SUPG advection–diffusion solver
src/underworld3/systems/init.py Rewire exports: AdvDiffusion/NavierStokes now composed Eulerian; add *SLCN names
src/underworld3/discretisation/discretisation_mesh.py Partition-independent cell_size() field backed by per-cell radii
src/underworld3/cython/petsc_maths.pyx Ensure DS constants are set for integral kernels (expression constants)
src/underworld3/analytic/transport.py Add uw.analytic.RotatingGaussian oracle
src/underworld3/analytic/_base.py Fix scalar-vs-1x1 symbol mismatch in analytic error calculation
src/underworld3/analytic/init.py Export/register RotatingGaussian
docs/examples/WIP/developer_tools/SOpt.py Update AdvDiffusion reference to SLCN
docs/examples/Tutorial_Thermal_Convection_Units.py Update AdvDiffusion reference to SLCN
docs/examples/heat_transfer/advanced/Ex_Poisson_v.SLCN.py Update AdvDiffusion reference to SLCN
docs/examples/fluid_mechanics/README.md Add new Eulerian NS SUPG example entries
docs/examples/fluid_mechanics/advanced/Ex_NavierStokesRotationTest.py Update NavierStokes reference to SLCN
docs/examples/fluid_mechanics/advanced/Ex_Navier_Stokes_SUPG_Taylor_Green_Vortex.py New Eulerian NS SUPG validation example
docs/examples/fluid_mechanics/advanced/Ex_Navier_Stokes_SUPG_Lid_Driven_Cavity.py New Eulerian NS SUPG cavity benchmark example
docs/examples/fluid_mechanics/advanced/Ex_Navier_Stokes_Lid_Driven_Flow_2d.py Update imports/usage to NavierStokesSLCN
docs/examples/fluid_mechanics/advanced/Ex_Navier_Stokes_Benchmarks_NS_DFG_2d.py Update NavierStokes reference to SLCN
docs/examples/fluid_mechanics/advanced/Ex_Navier_Stokes_Benchmarks_NS_DFG_2d_SLCN.py Update NavierStokes reference to SLCN
docs/examples/convection/intermediate/Ex_MoresiSolomatov_Convection_Cartesian.py Update AdvDiffusion reference to SLCN
docs/examples/convection/intermediate/Ex_Convection_Cartesian_ThermoChem.py Update AdvDiffusion reference to SLCN
docs/examples/convection/intermediate/Ex_Convection_5_SLCN_Cartesian-Yield.py Update AdvDiffusion reference to SLCN
docs/examples/convection/intermediate/Ex_Convection_4_SLCN_Cartesian-NL.py Update AdvDiffusion reference to SLCN
docs/examples/convection/advanced/Ex_Convection_Cylinder.py Update AdvDiffusion reference to SLCN
docs/examples/convection/advanced/Ex_Convection_Cartesian-Swarm.py Update AdvDiffusion reference to SLCN
docs/examples/convection/advanced/Ex_AdvectionDiffusionSwarm_RotationTest.py Update AdvDiffusion reference to SLCN
docs/examples/convection/advanced/Ex_AdvectionDiffusionSUPG_RotationTest.py New Eulerian AdvDiffusion SUPG example using RotatingGaussian
docs/examples/convection/advanced/Ex_AdvectionDiffusionSLCN_RotationTest.py Update AdvDiffusion reference to SLCN
docs/developer/index.md Add Eulerian SUPG design note to toctree
docs/developer/design/SOLVER_UNIFICATION_DESIGN.md Update naming note for NavierStokesSLCN vs composed NavierStokes
docs/beginner/tutorials/9-Unsteady_Flow.ipynb Update tutorial to use NavierStokesSLCN
docs/beginner/tutorials/8-Timestepping-coupled.ipynb Update tutorial to use AdvDiffusionSLCN
docs/beginner/tutorials/7-Timestepping-simple.ipynb Update tutorial text/code to use AdvDiffusionSLCN
docs/beginner/tutorials/15-Thermal-convection-with-units.ipynb Update tutorial to use AdvDiffusionSLCN
docs/beginner/tutorials/14-Timestepping-with-physical-units.ipynb Update tutorial to use AdvDiffusionSLCN
docs/api/solvers.md Document new composed solvers + EulerianSUPG transport manager
docs/advanced/semi-lagrangian-time-integration.md Add section describing Eulerian alternative
docs/advanced/index.md Add Eulerian solver docs to advanced toctree
docs/advanced/eulerian-navier-stokes.md New advanced doc page for composed Eulerian Navier–Stokes
docs/advanced/eulerian-advection-diffusion.md New advanced doc page for composed Eulerian AdvDiffusion
Review details
  • Files reviewed: 62/62 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +479 to +482
dt = fraction * hi / rate if rate > 0.0 else np.inf
if np.isinf(dt) or hi <= 0.0:
return SNES_Stokes.estimate_dt(self)
return dt

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in cef446a: the accuracy estimate goes through _dimensionalise_dt, as the scalar solver's does, so both bases return a time quantity under a scaling model and a plain number otherwise (test_1056 test_estimate_dt_carries_time_units_on_both_bases).

# EulerianSUPG assembles it in the solver's residual instead.
self._advection_mode = "split"
self.theta = theta
self.bcs = bcs

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in cef446a: the default is bcs=None and becomes a fresh list. A caller's list is kept by reference on purpose, and the code now says so: the solvers pass their live essential_bcs so that conditions added after construction reach the manager's projections; copying would silently drop them. The older flavours (Eulerian, SemiLagrangian, Symbolic) carry the same bcs=[] default and are left as they are in this PR.

lmoresi and others added 3 commits September 7, 2026 09:07
… kernels (#701)

_as_float took the magnitude of a Pint or UW quantity, so a semi-Lagrangian solver
stepped with 100 kyr under a 1 Myr reference time wrote 100 (not 0.1) into the
manager's runtime timestep and into the variable-step BDF bookkeeping. It now goes
through uw.non_dimensionalise, which handles both quantity types; without reference
scales the magnitude is what comes back. Test with a negative control in test_1057.

Closes #701.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…702)

DMSwarm reports a local size of -1 until particles are added on some rank, and
the advection then failed inside numpy with 'negative dimensions are not
allowed'. The empty rank of a populated swarm (size 0) is unchanged.

Closes #702.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…ts value (#696)

A runtime constant's value can change after construction, so sympy must not fold
on its current sign or on it being zero: exp(c) with c created at 0 evaluated to 1
at construction and a time ramp that started at t = 0 stayed frozen (found on the
Taylor-Green Dirichlet case). The three assumptions now answer None, as for a
plain Symbol; the value is read when the expression is unwrapped for compilation.
Control in test_0503: exp(c) survives, integrates to 1 at c = 0 and to e at c = 1.
Level-1 suite: 1704 passed.

Closes #696.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
…accuracy estimate; EulerianSUPG takes no mutable default bcs

The accuracy basis returned a bare non-dimensional number while the resolution
fallback returns a quantity under a scaling model; both now come back through
_dimensionalise_dt (test under reference scales). The manager's bcs default is
None -> a fresh list; a caller's list is still kept by reference on purpose, so a
solver's live essential_bcs reach the projections.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018T2VHUGaZiQVJ95qQ4DiSL
@lmoresi
lmoresi merged commit 1d5e8c1 into development Sep 8, 2026
2 checks passed
lmoresi added a commit that referenced this pull request Sep 8, 2026
…ure/quadrature-point-space

Conflict in _jitextension.py ccode_patch_fns: keep both the integration-point
derivative guard and the component_offsets from #688. uw.systems.AdvDiffusion
now names the composed Eulerian solver, so the SLCN tests and docs use
AdvDiffusionSLCN explicitly.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSGAFeA7qYXgkuw9ud8F2G
lmoresi added a commit that referenced this pull request Sep 8, 2026
The composed AdvDiffusion (#688) applies its theta-rule diffusive flux to
the history level, which differentiates the slot; the JIT guard refuses a
gradient of a delta field. The SLCN solver's separate nodal DFDt is the
structure this history needs.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSGAFeA7qYXgkuw9ud8F2G
gthyagi added a commit to gthyagi/underworld3 that referenced this pull request Sep 8, 2026
…manager

Resolve the PR underworldcode#688 architecture transition on existing PR underworldcode#689. Retain the upstream composing AdvDiffusion solver and EulerianSUPG implicit manager; move fixed and residual-converged predictor-corrector rate history, tau, stability estimate, corrections and reusable state into EulerianSUPGPC. Keep PDE assembly in the composing solver through small manager execution hooks.

Preserve matching-graph snapshots for manager history and solver timestep/change estimates. Reject cross-solver manager reuse; keep theta validation transactional and rewire changed velocity expressions. Retain explicit-tau anisotropic behavior. Exclude unrelated mantle Stokes, geoid and dependency changes.

Validate frozen-source equivalence, independent analytical recurrence and temporal order, live manager mutations, workspace reuse, and serial/eight-rank disk/fresh-process restarts. The focused MPI group passes 49 tests plus five formerly skipped disk replays; all four fresh-process MPI methods pass. CN and converged PC are order 2; fixed CitcomS remains approximately order 1. Full repository Level 1 run remains in progress and is reported separately.
lmoresi added a commit that referenced this pull request Sep 8, 2026
…DF2 and theta = 1

The composed solver (#688) takes the history as its transport manager. At
order 2 and at theta = 1 no spatial term sits on the old level, so the
integration-point history runs there and matches the SLCN solver's field
to 3e-3 on a rotating Gaussian. theta = 1 needed one change: the old-level
Adams-Moulton weight is identically zero there but was a runtime constant,
so 0 * grad(psi*) was still differentiated at code generation and the
guard fired on a dead term; spatial_weights now returns a literal zero for
theta = 1 (for every manager). The Crank-Nicolson flux differentiates the
old level and stays with AdvDiffusionSLCN; the guard's refusal is tested.

Underworld development team with AI support from Claude Code

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSGAFeA7qYXgkuw9ud8F2G
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