Skip to content

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

Open
lmoresi wants to merge 1 commit into
bugfix/snapshot-captures-expressionsfrom
bugfix/expression-redeclaration-explicit
Open

lmoresi wants to merge 1 commit into
bugfix/snapshot-captures-expressionsfrom
bugfix/expression-redeclaration-explicit

Conversation

@lmoresi

@lmoresi lmoresi commented Sep 22, 2026

Copy link
Copy Markdown
Member

Stacked on #775 — merge that first, then this retargets to development. Split out of #775 because it is a behaviour change users will notice, and it deserves its own review rather than riding along with a snapshot fix.

What was happening

An expression's identity is its name: uw.expression(r"\eta", ...) returns the existing container, which is what lets a formula written early keep seeing later edits to its contents. That much is deliberate.

But __init__ then ran on that returned object and overwrote its contents and its description from the arguments. So a line that reads like a declaration silently changed every formula already written against the name:

a = uw.expression(r'\eta', 1.0, 'first use')
b = uw.expression(r'\eta', 99.0, 'second, unrelated use')
a is b        # True
a.sym         # 99.0  — and the description is replaced too

The rule now

  • Same value → passes. Declaring the same thing twice changes nothing and no formula can tell. A factory that rebuilds an unmutated problem in one process is doing exactly that, and refusing it would be noise.
  • Different value → raises, naming the two things the caller might have meant (name.sym = value to change contents, uw.expression(name) to fetch). The existing contents are left untouched.
  • 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.

Blast radius, measured before choosing

Surveyed 576 tests (bands 00–03) with the refusal instrumented to record rather than raise:

re-declarations total 15
with the same value 6
with a different value 9, across 5 names
in library code 0

All nine were test factories rebuilding the same named problem in one process, where the silent overwrite was load-bearing.

The most instructive is \eta_{w302}, whose container still held

2.0*k_n302 + 0.05*{T302}(N.x, N.y)**2

from the first build — referencing the first build's mesh variable. The overwrite is what rebound it to the second build's variable, and nothing said so. That is the hazard in one line: not a stale value, a stale reference, invisible.

Those three factories now declare-or-assign through a small local helper, which makes the rebinding the visible act it always was:

def _param(name, value, description):
    try:
        return uw.expression(name, value, description)
    except ValueError:
        existing = uw.expression(name)
        existing.sym = value
        return existing

Tests

tests/test_00*|01*|02*|03*py: 576 passed. Two contract tests added — a name fetched twice is one container captured once, and re-declaring with a value raises while leaving the contents untouched.

Still open, not addressed here

_expr_names is a class attribute, so it outlives the model: after reset_default_model() a fresh model re-uses containers from the previous one, carrying their values until something sets them. Separate decision — clear the registry on reset, or namespace it per model.

Underworld development team with AI support from Claude Code

An expression's identity is its NAME: uw.expression(r"\eta", ...) returns the
existing container so a formula written early keeps seeing later edits. But
__init__ then ran on that returned object and overwrote its contents and its
description from the arguments - so a line that reads like a declaration
silently changed every formula already written against the name.

Re-declaration now:

  * with the SAME value, passes. Declaring the same thing twice changes nothing
    and no formula can tell; a factory that rebuilds an unmutated problem in one
    process is doing exactly that, and refusing it would be noise.
  * with a DIFFERENT value, raises, naming the two things the caller might have
    meant: `name.sym = value` to change the contents, uw.expression(name) to
    fetch. The existing contents are left untouched.

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.

Blast radius, measured before choosing rather than after. Surveying 576 tests
(bands 00-03) recorded 15 re-declarations in total: 6 with the same value and 9
with a different one, across five names - and NO library code among them. All
nine were test factories that rebuild the same named problem in one process,
where the silent overwrite was load-bearing. The most instructive is
\eta_{w302}, whose container still held `2.0*k_n302 + 0.05*{T302}(N.x, N.y)**2`
from the FIRST build: the overwrite is what rebound it to the second build's
mesh variable, and nothing said so.

Those three factories now declare-or-assign through a small local helper, which
makes the rebinding the visible act it always was.

tests/test_00*|01*|02*|03*py: 576 passed.

Underworld development team with AI support from Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant