VEP investigation: bug fixes, benchmark suite, TI-VEP fault stability#141
Closed
lmoresi wants to merge 0 commit intodevelopmentfrom
Closed
VEP investigation: bug fixes, benchmark suite, TI-VEP fault stability#141lmoresi wants to merge 0 commit intodevelopmentfrom
lmoresi wants to merge 0 commit intodevelopmentfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR bundles three fixes discovered during the VEP loading/unloading investigation, aiming to preserve legacy defaults while improving robustness for VE/VEP workflows and SNES divergence handling.
Changes:
- Restores the required
timestep=kwarg intests/run_ve_vep_oscillatory_plot.pyso the VE/VEP oscillatory script runs end-to-end again. - Prevents infinite recursion in
MathematicalMixin.__getattr__whensymproperty access fails during partial initialization. - Adds an opt-in
divergence_retrieskwarg to SNES-basedsolve()methods and implements warm-start retries in the Cython SNES base layer.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/run_ve_vep_oscillatory_plot.py |
Fixes test-script invocation by passing timestep=dt to VE_Stokes.solve(). |
src/underworld3/utilities/mathematical_mixin.py |
Guards __getattr__('sym') to avoid recursion when sym raises AttributeError during early init. |
src/underworld3/systems/solvers.py |
Threads divergence_retries through high-level solver wrappers and documents the new kwarg. |
src/underworld3/cython/petsc_generic_snes_solvers.pyx |
Implements _snes_solve_with_retries and wires it into SNES solve paths (Scalar/Vector/MultiComponent/Stokes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2184
to
+2188
| verbose : bool | ||
| divergence_retries : int, default=0 | ||
| Forwarded to each per-component SNES solve. 0 preserves | ||
| legacy behaviour. | ||
| """ |
Comment on lines
+1217
to
+1221
| the just-computed iterate plus the freshly-advected stress | ||
| history, which is often enough for VEP at yield onset (Min/softmin | ||
| kinks) to step off a bad Newton iterate. ``0`` preserves legacy | ||
| behaviour (divergence is terminal). Typical useful value is 1. | ||
| Only applies in the VE/VEP branch (``DFDt is not None``). |
7f2fa9f to
9b87e1d
Compare
9b87e1d to
259958b
Compare
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
Cumulative work from the VEP loading/unloading investigation. Three groups of changes, all preserving default behaviour for existing callers.
Bug fixes
MathematicalMixin.__getattr__infinite recursion onsym— when a subclass'ssym@Property raisesAttributeErrorduring partial__init__(e.g.SwarmVariable.symbeforeself._meshVaris set), Python falls back to__getattr__, which calls_validate_sym, which accessesself.sym, which re-enters__getattr__forname="sym"— unbounded loop, 100% CPU, no output. Reproduces in any test that creates a VE solver after a previous one in the same process. Added"sym"to the existing guard.VEP yield-surface drift under variable dt— projection-snapshot fix inSemiLagrangianDDt that prevents σ from breaching ±τ_y by ~30% under repeated halve/double-dt changes. Two new regression tests (test_1052_VEP_stability_regression).TI-VEP BDF-2 instability with spatial yield_stress— newly diagnosed in this session. Atorder=2with aninfluence_function-localisedyield_stressfield (i.e. realistic fault simulations), the simulation drifts unstably: |σ_xy| → 10⁸ over ~10 t_r at θ=±15°. Iso VEP with the same spatial τ_y was masked stable by yield clipping; pure VE on the same mesh+BC+IC also drifts. Recommended workaround: useorder=1for TI-VEP fault problems (the class default). Thebdf_blendknob is exposed for users who must use order=2 on a uniform-yield_stress problem and want explicit damping. Defaultbdf_blend=1.0(no silent default-degradation). Three planning items filed for the deeper fix (flux_with_history_order(k)API, UWexpression mutation audit, exponential-integration alternative).Restore timestep=dt in run_ve_vep_oscillatory_plot.py— one-line test-script fix.Smooth yield-mode retired — under-clipped the yield surface by ~50% with no compensating benefit. Setter now redirects users to softmin.
New solver APIs
DDt.set_initial_history(values, dt=None)onSemiLagrangianandEulerian— public API for planting BDF history at the start of a run. Two use cases: (a) analytical IC for benchmarks (no startup transient), (b) checkpoint restart (resume multistep history without re-rampingeffective_orderfrom BDF-1). Replaces a four-private-attribute pattern inbench_ve_harmonic.py. 6 unit tests intest_1052_ddt_set_initial_history.SaddlePt.set_jacobian_F1_source(F1_alt, linesearch=\"cp\")— optional override that autodiffs an alternate F1 expression for the velocity-block Jacobian while the residual F1 stays unchanged. Enables inexact-Newton tricks (smooth-Jacobian / sharp-residual). Default-installscplinesearch becausebt(PETSc default) rejects useful steps when the Jacobian is inexact.divergence_retrieskwarg on all SNES solvers (default 0). Warm-start re-solve helps Newton step off transient kinks.Benchmark suite
A self-contained run / log / replot architecture for VE/VEP analytical benchmarks (in
docs/advanced/benchmarks/):snes_linesearch_typefrombttocpfor inexact-Jacobian problems eliminates 196/413 spurious line-search rejections without losing accuracy.The benchmark npz files are self-contained (parameters + traces + analytical reference); replotting is decoupled from the slow simulation.
Empirical impact (from the benchmark traces, all on this branch)
bt)cplinesearch)order=1order=2(spatial τ_y)order=1insteadTest plan
./uw buildsucceedsorder=1for all (θ, τ_y) combinationsOut of scope / follow-ups
Filed in
~/Library/CloudStorage/Box-Box/Planning/underworld.md(Bugs and Nice to Have):cm.flux_with_history_order(k)API and the inexact-Newton path investigatedconstitutive_models.py(5 sites) — silent state aliasing, latent disastersympy.simplify=Truedefaults inevaluate*and visualisation — flip themUnderworld development team with AI support from Claude Code