Refuse a re-declaration that would change what an expression container holds

src/underworld3/function/__init__.py

@@ -64,6 +64,7 @@
 
 
 from .expressions import UWexpression as expression
+from .expressions import template_expression
 from .expressions import UWDerivativeExpression as _derivative_expression
 from .quantities import quantity, UWQuantity
 from .expressions import substitute as fn_substitute_expressions

src/underworld3/function/expressions.py

@@ -683,6 +683,49 @@ def substitute_expr(fn, sub_expr, keep_constants=True, return_self=True):
 # UWexpression Class - Simplified (no UWQuantity inheritance)
 # ============================================================================
 
+def reset_expression_registry():
+    """Drop the named expression containers, so a new model starts clean.
+
+    ``_expr_names`` is a CLASS attribute: it belongs to the process, not to a
+    Model. Without this, containers outlive the model that made them, and the
+    next model looking up a familiar name — ``\\eta``, ``t``, ``x`` — silently
+    inherits the previous one's contents. In a test session that is every
+    unrelated test sharing one ``\\eta``; in an interactive session it is the
+    previous run's viscosity turning up in the next.
+
+    Live objects are unaffected: a solver or a formula holds the container
+    itself, not its name, and keeps working. What resets is LOOKUP — the next
+    ``uw.expression(r"\\eta", ...)`` builds a fresh container instead of finding
+    the old one, which is what "a new model" should mean.
+
+    Called by :func:`~underworld3.model.reset_default_model`.
+    """
+    UWexpression._expr_names.clear()
+    UWexpression._ephemeral_expr_names.clear()
+
+
+def _wrap_with_units(sym, units):
+    """Wrap a plain number in a UWQuantity, leaving anything symbolic alone.
+
+    One definition, because two callers need it: construction, and
+    :func:`template_expression` refreshing a container that already exists. A
+    second copy of this is how a refresh ends up dropping the units that
+    construction applied.
+    """
+    if units is None or isinstance(sym, UWQuantity):
+        return sym
+    if isinstance(sym, (int, float, np.integer, np.floating)):
+        return UWQuantity(sym, units)
+    if sym is not None and not isinstance(sym, (sympy.Basic, UWexpression)):
+        # numeric-like, e.g. a numpy scalar; anything that will not convert
+        # keeps the units parameter unapplied rather than failing the call
+        try:
+            return UWQuantity(float(sym), units)
+        except (TypeError, ValueError):
+            pass
+    return sym
+
+
 def live_expressions():
     """The persistent expression containers, in a stable order.
 
@@ -780,9 +823,19 @@ def __new__(
 
         instance_no = UWexpression._expr_count
 
-        # If the expression already exists, return it
+        # If the expression already exists, return it — that IS the container
+        # contract: identity is the name, so the same name reaches the same
+        # object and a formula written against it keeps seeing later edits.
+        #
+        # Flagged so __init__ knows it is running on an object that already has
+        # contents. Construction must not silently overwrite them: changing what
+        # a container holds is what `.sym =` is for, and a second construction
+        # that quietly replaced the value would reach every formula already
+        # written against the name, from a line that reads like a declaration.
         if name in UWexpression._expr_names.keys() and _unique_name_generation == False:
-            return UWexpression._expr_names[name]
+            existing = UWexpression._expr_names[name]
+            existing._reused_construction = True
+            return existing
 
         # Check both dicts for name collisions
         name_exists_persistent = name in UWexpression._expr_names
@@ -931,6 +984,27 @@ def _sympystr(self, printer):
         """
         return self._display_name
 
+    def _holds_same_value(self, incoming) -> bool:
+        """Is a re-declaration asking for what this container already holds?
+
+        Declaring the same thing twice changes nothing and no formula written
+        against the name can tell — a factory that rebuilds an unmutated problem
+        in one process is doing exactly that, and refusing it would be noise.
+        Declaring something DIFFERENT is the case that must not pass silently.
+
+        Anything that cannot be compared counts as different, so the loud path
+        is the default: a container whose contents we cannot reason about is the
+        last one to overwrite quietly.
+        """
+        try:
+            if isinstance(incoming, UWexpression):
+                incoming = incoming._sym
+            if isinstance(incoming, (sympy.Basic, sympy.matrices.MatrixBase)):
+                return bool(incoming == self._sym)
+            return bool(sympy.sympify(incoming) == self._sym)
+        except Exception:
+            return False
+
     def __init__(
         self,
         name,
@@ -940,6 +1014,27 @@ def __init__(
         units=None,  # Units for wrapping the value
         **kwargs,
     ):
+        # Running on a container that already exists (see __new__): its contents
+        # belong to whoever set them, so leave them alone and say so. Anything
+        # that wants to change them has `.sym =`.
+        if getattr(self, "_reused_construction", False):
+            self._reused_construction = False
+            incoming = sym if sym is not None else value
+            if incoming is not None and not self._holds_same_value(incoming):
+                raise ValueError(
+                    f"expression {name!r} already exists and holds "
+                    f"{self._sym!r}. A UW expression is a persistent container "
+                    f"whose identity is its NAME, so this call returns the "
+                    f"existing object rather than making a new one — and "
+                    f"overwriting its contents here would silently change every "
+                    f"formula already written against {name!r}, from a line "
+                    f"that reads like a declaration.\n"
+                    f"  to change what it holds:   {name}.sym = <value>\n"
+                    f"  to fetch it:               uw.expression({name!r})\n"
+                    f"  for an independent symbol: choose another name"
+                )
+            return
+
         # Handle legacy 'value' parameter
         if value is not None and sym is None:
             import warnings
@@ -951,19 +1046,7 @@ def __init__(
         if value is not None and sym is not None:
             raise ValueError("Both 'sym' and 'value' provided - use only one")
 
-        # If units are provided and sym is a plain numeric value, wrap it in UWQuantity
-        # Don't wrap if sym is already a UWQuantity, UWexpression, or SymPy expression
-        if units is not None and not isinstance(sym, UWQuantity):
-            # Only wrap plain numeric values - not expressions or other complex types
-            if isinstance(sym, (int, float, np.integer, np.floating)):
-                sym = UWQuantity(sym, units)
-            elif sym is not None and not isinstance(sym, (sympy.Basic, UWexpression)):
-                # Try to wrap other numeric-like things (e.g., numpy scalars)
-                try:
-                    sym = UWQuantity(float(sym), units)
-                except (TypeError, ValueError):
-                    # Can't convert to float - ignore units parameter
-                    pass
+        sym = _wrap_with_units(sym, units)
 
         # TRANSPARENT CONTAINER PRINCIPLE (2025-11-27):
         # Store the wrapped object directly - don't extract or decompose it.
@@ -1992,6 +2075,40 @@ def unpack_var_fns(exp):
     return mesh, varfns, derivfns
 
 
+def template_expression(name, sym, description="No description provided", units=None):
+    """Declare a derived quantity, or refresh what it already holds.
+
+    A solver's ``F0``/``F1`` are templates: each access re-derives them from the
+    parameters as they stand now, so the second access within a step carries a
+    different ``\\Delta t`` and different Adams-Moulton coefficients from the
+    first. Construction is the wrong verb for that. A plain
+    ``expression(name, ...)`` call reads as a declaration, and a declaration
+    that silently replaced the contents is exactly what the re-declaration
+    guard refuses -- rightly, because for any other caller a repeated name is
+    an accident.
+
+    So the owner says what it means: write through ``.sym``, the sanctioned
+    path for changing what a container holds, and declare only when the
+    container does not exist yet. Formulae already written against the name go
+    on seeing the refreshed contents, which is the container contract and the
+    behaviour these properties have always relied on.
+
+    The name must be unique to the owner. This function does deliberately what
+    the guard refuses, so a name two owners both reach is silently rebound by
+    it, which is the bug the guard exists to catch -- tag the name with the
+    owner (``self.instance_number``) and it cannot happen.
+    """
+    existing = UWexpression._expr_names.get(name)
+    if existing is None:
+        return UWexpression(name, sym, description, units=units)
+
+    # Through the same wrapping construction uses: a refresh that skipped it
+    # would quietly strip the units the container was declared with.
+    existing.sym = _wrap_with_units(sym, units)
+    existing._description = description
+    return existing
+
+
 # ============================================================================
 # Backward Compatibility Aliases
 # ============================================================================

src/underworld3/model.py

@@ -5966,6 +5966,14 @@ def reset_default_model():
     import underworld3 as uw
     uw.use_strict_units(True)
 
+    # The named expression containers belong to the PROCESS, not to a Model:
+    # UWexpression._expr_names is a class attribute. Leaving them behind is what
+    # made a fresh model inherit the previous one's parameters under familiar
+    # names, and what made unrelated tests in one pytest process share a single
+    # `\eta`. A reset that leaves that standing is not a reset.
+    from underworld3.function.expressions import reset_expression_registry
+    reset_expression_registry()
+
     return _default_model
 
 

src/underworld3/systems/navier_stokes_eulerian.py

@@ -39,6 +39,7 @@
 from underworld3.systems.ddt import _step_ratio
 import underworld3.timing as timing
 from underworld3.function import expression as public_expression
+from underworld3.function import template_expression
 from underworld3.systems.ddt import _DDtBase
 from underworld3.systems.ddt import EulerianSUPG as EulerianSUPG_DDt
 from underworld3.systems.advection_diffusion_eulerian import _check_supplied_manager
@@ -473,8 +474,21 @@ def _stabilisation_flux(self):
     @property
     def F0(self):
         """Pointwise momentum residual: strong residual of the time scheme."""
-        f0 = public_expression(
-            r"\mathbf{f}_0\left( \mathbf{u} \right)",
+        # Tagged with the instance number, as \rho, \mathbf{a} and
+        # \mathbf{u}^{n-1} are above. A UWexpression's identity is its NAME, so
+        # an untagged name is ONE container shared by every solver of this class:
+        # a second Navier-Stokes solver in the same process re-declared it with
+        # its own instance's symbols, which silently rebound the name under any
+        # formula already written against the first solver's residual.
+        #
+        # A template, not a declaration: each access re-derives the residual
+        # from the parameters as they stand, so the contents legitimately
+        # change from one access to the next as \Delta t and the Adams-Moulton
+        # coefficients move. template_expression writes through .sym, which is
+        # the sanctioned path for that; constructing again would read as a
+        # declaration and is what the re-declaration guard refuses.
+        f0 = template_expression(
+            rf"\mathbf{{f}}_0\left( \mathbf{{u}}_{{{self.instance_number}}} \right)",
             self._strong_residual(),
             "Navier-Stokes SUPG: strong residual (time derivative, advection, body force)",
         )
@@ -487,8 +501,9 @@ def F1(self):
         dim = self.mesh.dim
         mechanical_pressure = (
             self.p.sym[0] - self.penalty * self.constitutive_model.K * self.div_u)
-        F1 = public_expression(
-            r"\mathbf{F}_1\left( \mathbf{u} \right)",
+        # A template, refreshed on every access -- see F0.
+        F1 = template_expression(
+            rf"\mathbf{{F}}_1\left( \mathbf{{u}}_{{{self.instance_number}}} \right)",
             self._viscous_flux() + self._devss_flux() - sympy.eye(dim) * mechanical_pressure
             + self._stabilisation_flux(),
             "Navier-Stokes SUPG: viscous flux of the time scheme, pressure, tau R (x) a",

tests/test_0007_snapshot_inmemory.py

@@ -919,15 +919,33 @@ def test_capture_reads_the_persistent_container_registry():
     assert not (ephemeral_names & persistent_names & {r"\beta"})
 
 
-def test_a_reused_name_is_one_container_and_is_captured_once():
-    """Identity is the NAME: asking for the same name returns the same object,
+def test_a_name_fetched_twice_is_one_container_and_is_captured_once():
+    """Identity is the NAME: fetching the same name returns the same object,
     which is what lets a formula written early keep seeing later edits. Capture
-    must therefore record it once, not once per construction site."""
+    must therefore record it once, not once per reference."""
     uw, model, mesh = _fresh_model_and_mesh()
-    first = uw.expression(r"\gamma_{shared}", 1.0, "first use")
-    second = uw.expression(r"\gamma_{shared}", 2.0, "second use")
-    assert first is second
+    first = uw.expression(r"\gamma_{shared}", 1.0, "declared once")
+    again = uw.expression(r"\gamma_{shared}")          # fetch, do not redeclare
+    assert first is again
 
     snap = model.save_state()
     key = f"{type(first).__name__}_{first.instance_number}"
     assert [k for k, _s, _w in snap.expressions].count(key) == 1
+
+
+def test_redeclaring_a_name_with_a_value_is_refused():
+    """Changing what a container holds is what ``.sym =`` is for.
+
+    A second construction that quietly replaced the contents would reach every
+    formula already written against that name, from a line that reads like a
+    declaration — so it raises, and says which of the two things the caller
+    meant. The existing contents are left untouched.
+    """
+    uw, model, mesh = _fresh_model_and_mesh()
+    eta = uw.expression(r"\eta_{decl}", 1.0, "declared")
+    eta.sym = 42.0
+
+    with pytest.raises(ValueError, match="already exists"):
+        uw.expression(r"\eta_{decl}", 99.0, "redeclared")
+
+    assert float(eta.sym) == pytest.approx(42.0), "a refused call still wrote"

tests/test_0024_jit_graph_lowering.py

@@ -14,6 +14,7 @@
 from sympy.vector.scalar import BaseScalar
 
 import underworld3 as uw
+from underworld3.function.expressions import reset_expression_registry
 import underworld3.function.expressions as ex
 from underworld3.function import diff_wrt_field
 from underworld3.utilities import _jit_graph as jg
@@ -268,6 +269,15 @@ def test_the_emitted_source_does_not_depend_on_what_came_before(monkeypatch):
     uw.discretisation.MeshVariable("J0024f", other, 1, degree=1)
     mesh2 = uw.meshing.UnstructuredSimplexBox(
         minCoords=(0.0, 0.0), maxCoords=(1.0, 1.0), cellSize=0.5)
+    # A second, independent problem that happens to reuse the names: _poisson
+    # declares k_{0024f} and g_{0024f} against whichever mesh it is given, and
+    # g's contents name that mesh's variable, so on mesh2 they are genuinely
+    # different contents under the same name. Freeing the names says that
+    # outright, rather than leaning on a re-declaration quietly overwriting
+    # them. Only LOOKUP resets -- _expr_count keeps counting, so the counter
+    # shift this test turns on is untouched, and the first problem's live
+    # objects hold their containers directly and are unaffected.
+    reset_expression_registry()
     second = _header(_poisson("0024f", mesh2), monkeypatch)
     assert first == second
     assert "uwt_0" in first, "the kernel has no temporaries: nothing was lowered"