diff --git a/src/underworld3/constitutive_models.py b/src/underworld3/constitutive_models.py index 3829776f..1c807d4a 100644 --- a/src/underworld3/constitutive_models.py +++ b/src/underworld3/constitutive_models.py @@ -1034,6 +1034,15 @@ def yield_anchor(self, value): # rebuilt, not merely revalued — and _get_yield_softness only builds it # alongside δ. (The power mean carries the anchor in _combine_yield itself, so # for that family the _reset is the whole of the update.) + # + # Discarding the softness atom ORPHANS the δ held by any + # YieldHomotopyControl built before this call: the control still ramps + # the atom it captured, and the model now reads a fresh one. That cannot + # bite a march, because `yield_continuation` refuses `anchor=` together + # with a ready-made control — so the only way to reach it is to build a + # control by hand, set the anchor afterwards, and read δ back off the + # control for diagnostics. Set the anchor BEFORE building the control + # (#490). self._yield_offset_expr = None self._yield_softness_expr = None self._reset() @@ -1191,6 +1200,18 @@ def viscosity_min_rounding(self): @viscosity_min_rounding.setter def viscosity_min_rounding(self, value): + # A rounding scale is a width, so it is non-negative by construction: + # zero is the exact hard Max, positive rounds the corner. A negative + # value has no reading — it would sharpen the corner past Max — and + # since it reaches the tangent only through the compiled expression, + # the symptom of setting one is a solver that will not converge rather + # than anything naming this property (#490). + if value is not None and not isinstance(value, sympy.Basic): + if float(value) < 0.0: + raise ValueError( + f"viscosity_min_rounding is a rounding WIDTH and cannot be " + f"negative; got {value}. Use 0 for the exact Max.") + self._viscosity_min_rounding = value self._reset() diff --git a/src/underworld3/cython/petsc_generic_snes_solvers.pyx b/src/underworld3/cython/petsc_generic_snes_solvers.pyx index dcadfe23..f3df7cf6 100644 --- a/src/underworld3/cython/petsc_generic_snes_solvers.pyx +++ b/src/underworld3/cython/petsc_generic_snes_solvers.pyx @@ -6783,19 +6783,36 @@ class SNES_Stokes_SaddlePt(SolverBaseClass): """ pushed = self._owned_option_pushes.get(key) user = self._owned_option_user.get(key) - if self.petsc_options.hasName(key): - try: - current = type(default)(self.petsc_options.getString(key)) - except Exception: - return default - if pushed is None or current != pushed: - # Never pushed by us, or the user has moved it since. Latch: from here on - # the option is theirs, because the next solve will read back OUR push of - # THEIR value and would otherwise mistake it for our own default. - self._owned_option_user[key] = current - return current - if user is not None: - return user + if not self.petsc_options.hasName(key): + # The key is gone from the options DB, so any value latched from a + # previous solve is gone with it. Without this the latch outlives + # the option: set snes_max_it=200, delete it, and the next solve + # correctly uses the default — but the one after that reads back + # OUR push of the default, finds `current == pushed`, falls through + # to the latched 200 and resurrects it (#490). + self._owned_option_user.pop(key, None) + return default + + try: + current = type(default)(self.petsc_options.getString(key)) + except Exception: + # The stored value will not convert to the option's type: a user who + # wrote `snes_max_it = "lots"`, or a key set as a bare flag and so + # holding None. Neither is a number this solve can use, so it takes + # its own default and leaves the value in the DB for PETSc to object + # to in its own terms (Charter: say what is swallowed and why). + return default + + if pushed is None or current != pushed: + # Never pushed by us, or the user has moved it since. Latch: from here on + # the option is theirs, because the next solve will read back OUR push of + # THEIR value and would otherwise mistake it for our own default. + self._owned_option_user[key] = current + return current + + if user is not None: + return user + return default def _push_owned_option(self, key, value): @@ -9350,13 +9367,19 @@ class SNES_Stokes_SaddlePt(SolverBaseClass): homotopy_options : dict, optional March settings passed to :func:`~underworld3.systems.yield_continuation.yield_continuation` — - ``smoother``, ``delta0``, ``down``, ``dmin``, ``entry_maxit``, - ``step_maxit``, ``retries``. All are defaulted; tuning them is optional. - ``smoother`` picks the soft-min family — ``"powermean"`` (default, - approaches the yield surface from below) or ``"sqrt"`` (from above). Which - gives the better cold entry is problem-dependent, so it is worth trying both - when a march will not start. Leave ``delta0`` unset unless you have a - reason: each family supplies its own entry, and the two δ are not the same + ``smoother``, ``anchor``, ``delta0``, ``down``, ``dmin``, + ``entry_maxit``, ``step_maxit``, ``retries``. All are defaulted; tuning + them is optional. + + ``smoother`` picks the soft-min family — ``"powermean"`` (default) or + ``"sqrt"``. Which gives the better cold entry is problem-dependent, so + it is worth trying both when a march will not start. + + Which SIDE of the exact ``Min`` the softened yield sits on belongs to + ``anchor``, not to the family: under the default onset anchor both + families sit below ``Min`` near yield. See ``yield_anchor`` on the + constitutive model. Leave ``delta0`` unset unless you have a reason: + each family supplies its own entry, and the two δ are not the same parameter. Returns diff --git a/tests/test_0204_owned_option_latch.py b/tests/test_0204_owned_option_latch.py new file mode 100644 index 00000000..9f81fe1b --- /dev/null +++ b/tests/test_0204_owned_option_latch.py @@ -0,0 +1,84 @@ +"""An owned option's user latch dies with the option (#490). + +`solve()` re-pushes the options the solver owns, so `_resolve_owned_option` has +to tell its own previous push from a value the user set. It does that by +latching the user's value the first time it sees one it did not push. + +The latch outlived the option. Deleting the key made the next solve fall back +correctly, but that solve pushed the default, and the one after read the default +back, found it equal to what it had pushed, and returned the latched value +instead. Measured on `development`: + + user sets 200 -> 200 + user deletes the key -> 50 + next solve -> 200 <-- resurrected, and permanent + and the next -> 200 + +The sequence is driven through `_resolve_snes_max_it` / `_push_snes_max_it` +rather than through `solve()`: that is the pair `solve()` itself calls, and it +takes no solve to exercise, so the test says what it means and runs in a second. +""" + +import pytest + +import underworld3 as uw + +pytestmark = [pytest.mark.level_1, pytest.mark.tier_a] + +DEFAULT = 50 +USER = 200 + + +@pytest.fixture +def solver(): + mesh = uw.meshing.StructuredQuadBox(elementRes=(3, 3)) + v = uw.discretisation.MeshVariable("Vlatch", mesh, mesh.dim, degree=2) + p = uw.discretisation.MeshVariable("Platch", mesh, 1, degree=1) + + return uw.systems.Stokes(mesh, velocityField=v, pressureField=p) + + +def _solve_cycle(solver): + """One solve's worth of the resolve-then-push pair.""" + + resolved = solver._resolve_snes_max_it(DEFAULT) + solver._push_snes_max_it(resolved) + + return resolved + + +def test_a_user_value_is_honoured_and_keeps_being_honoured(solver): + """The control: the latch does its job while the option is there.""" + + solver.petsc_options["snes_max_it"] = USER + + assert _solve_cycle(solver) == USER + assert _solve_cycle(solver) == USER + assert _solve_cycle(solver) == USER + + +def test_deleting_the_option_does_not_leave_the_value_behind(solver): + """Deleting the key gives the default back, and it stays given back.""" + + solver.petsc_options["snes_max_it"] = USER + assert _solve_cycle(solver) == USER + + del solver.petsc_options["snes_max_it"] + + assert _solve_cycle(solver) == DEFAULT + assert _solve_cycle(solver) == DEFAULT, ( + "the deleted user value came back on the second solve after deletion — " + "the latch outlived the option it was latched from") + assert _solve_cycle(solver) == DEFAULT + + +def test_a_second_user_value_replaces_the_first(solver): + """A moved value is picked up rather than shadowed by the latched one.""" + + solver.petsc_options["snes_max_it"] = USER + assert _solve_cycle(solver) == USER + + solver.petsc_options["snes_max_it"] = 7 + + assert _solve_cycle(solver) == 7 + assert _solve_cycle(solver) == 7 diff --git a/tests/test_1059_yield_anchor.py b/tests/test_1059_yield_anchor.py index 16765d85..7747d12d 100644 --- a/tests/test_1059_yield_anchor.py +++ b/tests/test_1059_yield_anchor.py @@ -69,8 +69,12 @@ def test_delta_zero_is_exact_min_for_both_anchors(): @pytest.mark.level_1 @pytest.mark.tier_a @pytest.mark.parametrize("smoother", ("sqrt", "powermean")) -def test_yield_anchor_keeps_onset_stress_exact(smoother): - """tau/tau_y = f*eta/eta_ve = 1 at f=1 for every delta. The onset anchor does NOT.""" +def test_the_yield_anchor_keeps_stress_exact_at_nominal_yield(smoother): + """tau/tau_y = f*eta/eta_ve = 1 at f=1 for every delta. The onset anchor does NOT. + + Named for the anchor it exercises: this drives ``yield_anchor="yield"``, and + "onset" is the OTHER anchor — the one shown here to miss (#490). + """ c = _model(smoother); c.yield_anchor = "yield" for d in DELTAS[smoother]: assert abs(eta(c, 1.0, d) - 1.0) < 1e-10, d