tests: cover SNES_Scalar.constant_nullspace#235
Merged
Conversation
Development's constant_nullspace (added in 074a660 for the Monge-Ampere smoother's pure-Neumann solve) had no test coverage. This adds a tier_a/level_1 regression test salvaged from the now-closed #201 (which proposed a duplicate petsc_use_constant_nullspace flag): - a pure-Neumann Poisson (-Δu = x - 1/2, no Dirichlet BCs) is singular up to a constant; with constant_nullspace=True PETSc returns the minimum-norm (discrete zero-mean) solution. The test checks convergence, that the non-constant part matches the analytic solution to FE accuracy, and that the mean drops vs the no-nullspace solve; - property-setter round-trip and default. Verified against current development: constant_nullspace=True gives mean 3.7e-16 vs -0.224 drift when off. Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Adds tier_a/level_1 regression coverage for SNES_Scalar.constant_nullspace by exercising a pure-Neumann Poisson solve and verifying the solver’s “canonical” (minimum-norm / near-zero-mean) behavior when the constant nullspace is declared.
Changes:
- Add
tests/test_1056_constant_nullspace.pycovering a canonical pure-Neumann Poisson case with/withoutconstant_nullspace. - Add a small property round-trip/default test for
constant_nullspace.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+100
| mean_on = T_on.data[:, 0].mean() | ||
| assert abs(mean_on) < abs(mean_off), ( | ||
| f"with nullspace declaration, |mean(T)| should drop; " | ||
| f"got |mean_off|={abs(mean_off):.3e}, |mean_on|={abs(mean_on):.3e}" | ||
| ) |
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.
What
Adds a
tier_a/level_1regression test forSNES_Scalar.constant_nullspace, which currently has no test coverage on development (it was added in074a660for the Monge-Ampère smoother's pure-Neumann solve).The test is salvaged from the now-closed #201, which proposed a duplicate
petsc_use_constant_nullspaceflag. #201 was closed as redundant — development already provides this capability asconstant_nullspace(with the cached nullspace object that review on #201 had suggested). This PR keeps the useful part of #201: its test, renamed to the native attribute.Coverage
-Δu = x − ½, no Dirichlet BCs) is singular up to a constant. Withconstant_nullspace=True, PETSc returns the minimum-norm (discrete zero-mean) solution. The test asserts convergence, that the non-constant part matches the analytic solutionu = -x³/6 + x²/4 + Cto FE accuracy, and that the mean drops relative to the no-nullspace solve.Verified against current development:
constant_nullspace=True→ mean 3.7e-16 vs −0.224 drift when off (2 passed locally).A small follow-up worth noting: development's
constant_nullspacesetter doesn't setis_setup = False, so toggling it after a solve won't re-attach the nullspace. Not addressed here (test-only PR), but flagging it.Underworld development team with AI support from Claude Code