Add PETSc pressure nullspace support to Stokes - #90
Conversation
lmoresi
left a comment
There was a problem hiding this comment.
Review: Good feature, a few suggestions
This is a useful addition — the PETSc nullspace approach is the right way to handle pressure gauge freedom, especially for annulus and spherical-shell benchmarks where pressure Dirichlet BCs are not physically meaningful.
Design question: rigid-body modes
The current implementation handles the constant-pressure nullspace (1 mode). For spherical shell / annulus models, the system can also have rigid-body rotation null modes (net rotation for free-slip shells). Does the architecture support extending the nullspace basis to include these? The PETSc.NullSpace().create(vectors=(...)) call accepts multiple basis vectors, so it should be straightforward to add rotation modes later — worth noting in the docstring.
Suggestions
-
Branch naming: The project uses descriptive kebab-case branch names like
feature/pressure-nullspaceorbugfix/...— seedocs/developer/guides/branching-strategy.mdfor the convention. Thecodex/prefix doesn't follow this pattern. -
Test marker: The test uses
@pytest.mark.level_3but it's a 3x3 mesh solve that runs in seconds.level_1would be more appropriate (seedocs/developer/TESTING-RELIABILITY-SYSTEM.md). -
_pressure_dirichlet_bcsfield ID: Hardcoding{1}as the pressure field ID is fragile. Consider usingself.p.field_idexclusively (with a fallback error if not available), rather than assuming 1 as a default. -
Multiple
_attach_pressure_nullspacecalls: The call appears in 4 places in the solve paths. This is fine (cheap operation), but a comment explaining why it's needed after eachsetFromOptions()would help future readers.
Summary
The implementation follows PETSc best practices (ex62.c, ex69.c patterns). The validation check for conflicting pressure BCs is a nice touch. Good to merge after the minor items above.
|
Addressing the review items directly:
The current fork branches |
Summary
This PR adds PETSc constant-pressure nullspace support to the Underworld Stokes saddle-point solver.
What changed
petsc_use_pressure_nullspaceproperty onSNES_Stokes_SaddlePt.Why this is important
At the moment, users often make singular Stokes systems solvable by imposing a pressure Dirichlet condition, sometimes on an entire boundary. That removes the pressure gauge freedom, but it is stronger than the mathematical requirement and can change the solution.
This PR makes it possible to follow the standard PETSc approach instead:
This matters for annulus and spherical-shell benchmarks, especially cases like the Thieulot benchmark
k=0branch, where pressure is only defined up to a constant and a boundary-wide pressure Dirichlet condition is not the right tool.It also makes Underworld align better with PETSc/ASPECT-style pressure handling for incompressible Stokes problems.
PETSc precedent
This follows the same idea used in PETSc Stokes examples, where PETSc builds a pressure nullspace and attaches it to the operator instead of forcing pressure pointwise on a boundary:
CreatePressureNullSpace()CreatePressureNullSpace()Validation
pixi run -e amr-dev pytest tests/test_1013_stokes_pressure_nullspace.py -qThis test passes locally and verifies that a Stokes solve without pressure Dirichlet BCs converges with the nullspace attached.