Add monotone_mode kwarg to AdvDiffusionSLCN constructor#189
Merged
Conversation
PR #186 landed the monotone limiter on SemiLagrangian_DDt. To use it via the high-level solver, users had to construct the solver and then mutate the DDt instances after the fact: adv_diff = uw.systems.AdvDiffusionSLCN(mesh, u_Field=T, V_fn=v.sym) adv_diff.DuDt.monotone_mode = "clamp" adv_diff.DFDt.monotone_mode = "clamp" This change adds a one-line constructor idiom: adv_diff = uw.systems.AdvDiffusionSLCN( mesh, u_Field=T, V_fn=v.sym, monotone_mode="clamp") The new kwarg forwards to both internally-constructed SemiLagrangian DDts (DuDt and DFDt). When the caller supplies a pre-built DuDt explicitly, that DDt's monotone_mode is preserved as the source of truth — the kwarg only takes effect for the internally-constructed DFDt in that case (documented in the class docstring). Default unchanged (None = pure FE trace-back). No behaviour change for code that doesn't pass the kwarg. Regression test in tests/test_1054_advdiff_monotone_mode_kwarg.py covers: default None, clamp forwarded to both DuDt and DFDt, pick forwarded to both, and the explicit-DuDt-preserves-its-own-mode case. Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a monotone_mode keyword argument to the SNES_AdvectionDiffusion (AdvDiffusionSLCN) constructor, forwarding it to the internally-constructed SemiLagrangian_DDt instances for DuDt and DFDt. Defaults to None to preserve legacy behavior. When the caller supplies a pre-built DuDt, the kwarg only applies to the internally-built DFDt.
Changes:
- New
monotone_modekwarg onSNES_AdvectionDiffusion.__init__, plumbed through both internalSemiLagrangian_DDtconstructions. - Class docstring documents the new kwarg, its three modes, and the user-supplied-
DuDtprecedence rule. - New regression tests verifying default,
"clamp","pick", and caller-supplied-DuDt precedence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/underworld3/systems/solvers.py | Adds monotone_mode kwarg, docstring entry, and forwards it to both SemiLagrangian_DDt instances. |
| tests/test_1054_advdiff_monotone_mode_kwarg.py | New regression tests covering default, clamp, pick, and caller-supplied-DuDt precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lmoresi
added a commit
that referenced
this pull request
May 15, 2026
smooth_mesh_interior auto-pinning iterated every label in mesh.boundaries, but Annulus exposes a "Centre" pressure-pin label whose underlying DMLabel has an invalid communicator — any PETSc call on it (getNumValues, getValueIS, view) hard-aborts the interpreter (not a Python-catchable exception). Add "Centre" to the auto-pin skip list (it's a single-point pressure marker, not a geometric boundary) and keep the existing try/except guards in _pinned_mask for any future similar quirks. Annulus + smoother now runs end-to-end. AdvDiffusionSLCN.__init__: forward a new `theta` kwarg to both internally-constructed SemiLagrangian_DDt instances, mirroring the existing monotone_mode forwarding from #189. Lets callers configure the Adams-Moulton θ at construction time instead of patching adv_diff.DuDt.theta / DFDt.theta after the fact. Tested in the free-surface convection zoo: rk4 + theta=1.0 (BE) + monotone_mode="clamp" + smooth_mesh_interior every 2 steps runs 6 steps with T staying in [0,1] and no integrator instability. 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.
Summary
PR #186 landed the monotone-limiter functionality on
SemiLagrangian_DDt(instance attribute and per-call kwarg). To use it via theAdvDiffusionSLCNsolver, users had to construct the solver and then mutate the DDt instances after the fact:This PR adds a one-line constructor idiom:
The kwarg forwards to both internally-constructed
SemiLagrangian_DDtinstances (DuDtandDFDt). DefaultNonepreserves legacy behaviour exactly.Caller-supplied DuDt
If the caller passes a pre-built
DuDt, that DDt'smonotone_modeis preserved (the supplied DDt is the source of truth). Themonotone_modekwarg then only takes effect on the internally-constructedDFDt. Documented in the class docstring.Test plan
tests/test_1054_advdiff_monotone_mode_kwarg.py— 4 cases:Nonepropagates to both DDts\"clamp\"propagates to both\"pick\"propagates to bothFiles changed
src/underworld3/systems/solvers.py(+33 lines, docstring + kwarg + two forward calls)tests/test_1054_advdiff_monotone_mode_kwarg.py(+70 lines, new test file)Underworld development team with AI support from Claude Code