fix(language): a file whose terms read each other's sums is refused at load

CHANGELOG.md

@@ -12,6 +12,7 @@ it releases that version ([RELEASING.md](https://github.com/energy-models/mathsp
 
 ## Upcoming version
 
+- fix(language): a file whose terms read each other's sums is refused at load ([#780](https://github.com/energy-models/mathspec/pull/780))
 - fix(language): dual(c) is the rate at which the optimal objective rises with the right side of c, so an equality has a sign too ([#751](https://github.com/energy-models/mathspec/pull/751))
 - fix(language): a macro formal written inside a list takes the name the call binds to it ([#779](https://github.com/energy-models/mathspec/pull/779))
 - feat(language): a sum names several dimensions in one over= list ([#778](https://github.com/energy-models/mathspec/pull/778))

src/mathspec/lowering.py

@@ -39,9 +39,9 @@
     PiecewiseDeclaration,
     Program,
     SosDeclaration,
-    Variable,
     VariableDeclaration,
     VariableDefined,
+    variables_of,
     walk,
 )
 from mathspec.resolution import (
@@ -56,6 +56,8 @@
 from mathspec.validation import emitted_name_errors, reference_errors
 
 if TYPE_CHECKING:
+    from collections.abc import Mapping
+
     from mathspec.program import Expression
     from mathspec.spec import AssumptionBlock, Spec
 
@@ -144,13 +146,17 @@ def lower(schema: Spec) -> Program:
         if term is None:
             continue
         assert isinstance(term, Named), 'a term is a name, and a name resolves to the entry it names'
-        if any(isinstance(node, Variable) and node.name == gname for node in walk(term)):
-            errors.append(
-                f"{context}: its term {gdef.term!r} reads '{gname}', the sum the term adds to, so the sum would "
-                f'define itself. A term is what this file puts in: write it in what this file declares.'
-            )
-            continue
         terms[gname] = term
+    for gname, through in [(g, _loop(g, terms)) for g in terms]:
+        if through is None:
+            continue
+        via = f', through {" -> ".join(repr(name) for name in through)}' if through else ''
+        errors.append(
+            f"Given expression '{gname}': its term {schema.given.expressions[gname].term!r} reads '{gname}', the "
+            f'sum the term adds to{via}, so the sum would define itself. A term is what this file puts in: write '
+            f'it in what this file declares.'
+        )
+        del terms[gname]
 
     variables = {}
     for vname, vdef in schema.variables.items():
@@ -280,6 +286,26 @@ def lower(schema: Spec) -> Program:
     return program
 
 
+def _loop(target: str, terms: Mapping[str, Named]) -> list[str] | None:
+    """The sums *target*'s term reads *target* through, by this file's terms, or ``None`` where it does not read it.
+
+    Terms that read each other's sums in one file define each sum by itself
+    whatever the other files add, so the file decides it alone. ``[]`` is a
+    term that reads its own sum.
+    """
+    seen = {target}
+    stack: list[tuple[str, list[str]]] = [(target, [])]
+    while stack:
+        name, path = stack.pop()
+        for read in sorted(variables_of(terms[name])):
+            if read == target:
+                return path
+            if read in terms and read not in seen:
+                seen.add(read)
+                stack.append((read, [*path, read]))
+    return None
+
+
 def _frame_of(name: str, entry: Named, schema: Spec) -> tuple[str, ...]:
     """The dims an entry is read over: the ``dims:`` it declares, as written, else the body's in declaration order."""
     declared = schema.expressions[name].dims

tests/test_terms.py

@@ -194,6 +194,21 @@ def test_a_contributor_reads_the_name_as_the_whole_sum():
             r"Given expression 'injection': its term 'demand_injection' reads 'injection', the sum the term adds to",
             id='a-term-reading-the-sum-through-its-name',
         ),
+        pytest.param(
+            {
+                **DEMAND,
+                'given': {
+                    'expressions': {
+                        'injection': {'dims': BUS_FRAME, 'term': 'demand_injection'},
+                        'withdrawal': {'dims': BUS_FRAME, 'term': 'demand_withdrawal'},
+                    }
+                },
+                'expressions': {'demand_injection': 'withdrawal - load', 'demand_withdrawal': 'injection + load'},
+            },
+            r"Given expression 'injection': its term 'demand_injection' reads 'injection', the sum the term adds "
+            r"to, through 'withdrawal'",
+            id='two-terms-reading-each-other-s-sum',
+        ),
         pytest.param(
             _demand(term='-load'),
             r"Given expression 'injection': its term '-load' is no expression this file declares",
@@ -221,6 +236,21 @@ def test_what_a_term_may_not_be_is_refused_at_load(spec, message):
         to_spec(spec)
 
 
+def test_a_term_may_read_another_sum_its_file_adds_to():
+    """A read of another sum makes no loop until that sum's term reads back."""
+    spec = {
+        **DEMAND,
+        'given': {
+            'expressions': {
+                'injection': {'dims': BUS_FRAME, 'term': 'demand_injection'},
+                'withdrawal': {'dims': BUS_FRAME, 'term': 'demand_withdrawal'},
+            }
+        },
+        'expressions': {'demand_injection': 'withdrawal - load', 'demand_withdrawal': 'load'},
+    }
+    assert to_spec(spec).program.given.expressions['injection'].term is not None
+
+
 def test_a_term_may_be_quadratic():
     """A term is held to what an objective or a constraint admits, since one of them reads the sum."""
     square = {**FLEET, 'expressions': {'generator_injection': 'sum(gen_p * gen_p, over=generator)'}}