From b01ddbcc0378245e5d2611f57c4964556d665ddb Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 10:57:56 +0000 Subject: [PATCH 1/4] fix(language): a long chain of named expressions loads, a typo beside a formal is refused, and a fault in an entry hides no other The review of #626 through #638, addressed on top of #638. - A template's by= is checked before its formal columns send the call back, so sum(x, by=nope, over=a, into=b) is refused again; the over= and by= refusals say "or a formal of this macro" again; a formal along= beside a by= is pinned as building nothing. - Validation no longer raises after the macros and the expressions: entries, so a fault there hides no constraint's fault. A use of a refused entry says it does not load, and the refusal is listed with it. - A cycle closed through a macro names the macro in its chain, and one closed through a case's when is pinned. - Named expressions are resolved from a worklist in dependency order, so a chain of any length costs no stack, and the resolved tree is held to MAX_RESOLVED_DEPTH, three times what one text may nest, with a refusal that names the depth rather than the parser's message about a tree that was not deep. - A refused call builds nothing for the call around it, so sum(sum(p, into=g)) reports one fault. - A named offset or window is held to dtype: int where it is read, so offset=-tag says the dtype before the sign. - A piecewise block is refused on the link the file wrote for a str or bool breakpoint parameter and for an lp x-link with no variable; an undeclared over: is one line. - The exclusivity rewrite quotes a literal as the file wrote it. - fan_in reads through a Named; dimensions uses program.children; the walk prints "otherwise" for the last region without testing it; the census names no line that runs. - Tests pin the error class per rule, the unused template and entry refusals, +p as a named amount, and a link through a refused entry. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015h57WkBDnpxrknuJ5zZy9F --- docs/reference/language/expressions.md | 2 + docs/reference/language/named.md | 3 +- src/math_spec/_where_parser.py | 6 +- src/math_spec/dimensions.py | 22 +---- src/math_spec/exclusivity.py | 3 +- src/math_spec/expansion.py | 8 ++ src/math_spec/model.py | 8 +- src/math_spec/program.py | 8 +- src/math_spec/resolution.py | 126 +++++++++++++++++++++---- src/math_spec/typesetting/walk.py | 11 +-- src/math_spec/validation.py | 36 +++++-- tests/fixtures.py | 10 +- tests/test_dimensions.py | 44 +++++++-- tests/test_exclusivity.py | 2 + tests/test_expansion.py | 60 ++++++++++-- tests/test_lowering.py | 16 +++- tests/test_piecewise.py | 54 +++++++++++ tests/test_validation.py | 68 ++++++++++++- tests/typesetting/test_golden.py | 1 - 19 files changed, 405 insertions(+), 83 deletions(-) diff --git a/docs/reference/language/expressions.md b/docs/reference/language/expressions.md index 6ea56fba..61d03a99 100644 --- a/docs/reference/language/expressions.md +++ b/docs/reference/language/expressions.md @@ -27,6 +27,8 @@ NUMBER ::= integer | float | "inf" | ".inf" - A float may carry an exponent, as in `1e5` or `2.5e-3`. - The same keyword twice in one call is an error. - An expression nests at most 100 levels deep, and so does a `where:` string. + With every named expression it reads written in, an expression nests at most + 300 levels deep. ## Where a product of two variables is allowed diff --git a/docs/reference/language/named.md b/docs/reference/language/named.md index fba50ddc..527eb529 100644 --- a/docs/reference/language/named.md +++ b/docs/reference/language/named.md @@ -180,7 +180,8 @@ macros: collide with a declared dimension. - The number of arguments is checked at each call site. A cycle is reported with its reference chain. -- Every template is parsed and name-checked at load, whether or not it is called. +- Every template is held at load to every rule a call site is, whether or not it + is called. A formal is left for the call site to bind. Anything composed out of the [built-in operators](operators.md) belongs here. What the language cannot express is under diff --git a/src/math_spec/_where_parser.py b/src/math_spec/_where_parser.py index ef8d1038..d3137cb4 100644 --- a/src/math_spec/_where_parser.py +++ b/src/math_spec/_where_parser.py @@ -119,7 +119,7 @@ class UnresolvedComparisonNode: #: Every node a parsed where string is built of: the connectives and literals, #: the unresolved leaves, and the arithmetic and the two side nodes under a #: comparison. What the depth measurement walks. -_ParsedWhere = Predicate | UnresolvedWhereNode | ArithmeticNode | ColumnNode | KeywordNode +ParsedWhere = Predicate | UnresolvedWhereNode | ArithmeticNode | ColumnNode | KeywordNode # --------------------------------------------------------------------------- @@ -252,7 +252,7 @@ def _named_rewrite(text: str, loc: int) -> str | None: ) -def _nested(node: _ParsedWhere) -> tuple[_ParsedWhere, ...]: +def nested(node: ParsedWhere) -> tuple[ParsedWhere, ...]: """What a where string nests through: a connective's operands, a comparison's sides, and a call's predicate.""" if isinstance(node, UnresolvedCountNode): return (node.call, node.value) @@ -284,5 +284,5 @@ def parse_where(text: str) -> Predicate | UnresolvedWhereNode: """ return cast( 'Predicate | UnresolvedWhereNode', - parse_text(_WHERE_GRAMMAR, text, 'where string', _named_rewrite, _nested, _DEEP_REWRITE), + parse_text(_WHERE_GRAMMAR, text, 'where string', _named_rewrite, nested, _DEEP_REWRITE), ) diff --git a/src/math_spec/dimensions.py b/src/math_spec/dimensions.py index 7fd84e42..e421f93e 100644 --- a/src/math_spec/dimensions.py +++ b/src/math_spec/dimensions.py @@ -49,6 +49,7 @@ Variable, VariableDefined, WindowSum, + children, ) if TYPE_CHECKING: @@ -81,7 +82,7 @@ def dims_of(node: Expression, schema: Spec, context: str) -> frozenset[str]: return frozenset().union(*(dims_of(region.value, schema, context) for region in node.regions)) if isinstance(node, Negate | Add | Multiply | Power | Divide): - return frozenset().union(*(dims_of(child, schema, context) for child in _operands(node))) + return frozenset().union(*(dims_of(child, schema, context) for child in children(node))) inner = dims_of(node.operand, schema, context) if isinstance(node, Sum): @@ -96,16 +97,6 @@ def dims_of(node: Expression, schema: Spec, context: str) -> frozenset[str]: assert_never(node) -def _operands(node: Negate | Add | Multiply | Power | Divide) -> tuple[Expression, ...]: - if isinstance(node, Negate): - return (node.operand,) - if isinstance(node, Add | Multiply): - return (node.left, node.right) - if isinstance(node, Power): - return (node.base, node.exponent) - return (node.numerator, node.divisor) - - def _named_dims(node: Named, schema: Spec, context: str) -> frozenset[str]: """A cased entry's declared frame rather than the union of its arms — a narrower arm broadcasts — and a plain entry's body.""" declared = schema.expressions[node.name].dims @@ -238,19 +229,12 @@ def _check_joined(call: str, use: Direction | Partition, inner: frozenset[str], def _check_named_amount( node: Translate | WindowSum, verb: str, inner: frozenset[str], schema: Spec, context: str ) -> None: - """The rules that hold of an ``offset=`` or ``window=`` naming a parameter; a literal breaks none of them.""" + """The two rules of an ``offset=`` or ``window=`` naming a parameter that need the operand's dims; resolution holds it to its dtype.""" kwarg, amount = ('offset', node.offset) if isinstance(node, Translate) else ('window', node.width) if not isinstance(amount, str): return words = AMOUNTS[verb] declared = schema.parameters[amount] - if declared.dtype != 'int': - raise DimensionError( - f'{context}: {verb}({kwarg}={amount}) counts positions along ' - f"'{node.along}', but '{amount}' is declared dtype: {declared.dtype}. A count of " - f'positions is integral — declare it dtype: int, which binds only an integer ' - f'column, so a fractional {words.noun} has nowhere to arrive from.' - ) if node.along in declared.dims: raise DimensionError( f'{context}: {verb}({kwarg}={amount}) steps along ' diff --git a/src/math_spec/exclusivity.py b/src/math_spec/exclusivity.py index 13325fdc..bd9cb857 100644 --- a/src/math_spec/exclusivity.py +++ b/src/math_spec/exclusivity.py @@ -243,7 +243,8 @@ def _expression_rewrite(node: ExpressionComparison) -> str: if number is not None and isinstance(right, Parameter): return ( f'the literal is on the left, and a comparison is read as arithmetic there — write it as ' - f'the same test the other way round, {right.name} {_FLIPPED[node.op]} {number:g}' + f'the same test the other way round, {right.name} {_FLIPPED[node.op]} ' + f'{int(number) if number.is_integer() else number}' ) return ( 'it compares expressions, whose values only the data decides — compare one parameter against a ' diff --git a/src/math_spec/expansion.py b/src/math_spec/expansion.py index 5200e9ab..8090c971 100644 --- a/src/math_spec/expansion.py +++ b/src/math_spec/expansion.py @@ -74,6 +74,14 @@ def parse_template(name: str, macro: MacroBlock, context: str) -> ArithmeticNode def _expand(node: ArithmeticNode, ns: Namespace, context: str, stack: tuple[str, ...]) -> ArithmeticNode: + """*node* with every macro call under it substituted; *stack* is the macros this walk is inside. + + A name a template reads is checked against the entries being resolved + here, where the macros it came through are known, so a cycle closed + through a macro is reported with the macros in its chain. + """ + if isinstance(node, NameNode) and stack and (refusal := ns.cycle(node.name, context, stack)) is not None: + raise SchemaError(refusal) if isinstance(node, FunctionCallNode) and node.name in ns.schema.macros: if node.name in stack: msg = f'{context}: circular macro reference: {" -> ".join([*stack, node.name])}' diff --git a/src/math_spec/model.py b/src/math_spec/model.py index c756590f..e2b08e14 100644 --- a/src/math_spec/model.py +++ b/src/math_spec/model.py @@ -1124,14 +1124,20 @@ def _sos_emitted_names(self) -> Iterator[str]: yield from self._collisions(f"Sos '{sname}'", Emitted.of(sname, block.type).by_kind) def _piecewise_references(self) -> Iterator[str]: - """A curve runs along a declared dimension through values parameters carrying it, gated by a binary, masked by a bool.""" + """A curve runs along a declared dimension through numeric values parameters carrying it, gated by a binary, masked by a bool.""" for name, pw in self.piecewise.items(): context = f"piecewise '{name}'" if pw.over not in self.dimensions: yield undeclared_dimension('piecewise', name, pw.over) + continue for i, link in enumerate(pw.links): if link.values not in self.parameters: yield f"{context}: link {i} values references undeclared parameter '{link.values}'" + elif (dtype := self.parameters[link.values].dtype) not in NUMERIC_DTYPES: + yield ( + f"{context}: link {i} values parameter '{link.values}' is declared dtype: {dtype}, and a " + f'breakpoint is a number. Declare it dtype: float or int.' + ) elif pw.over not in self.parameters[link.values].dims: yield ( f"{context}: link {i} values parameter '{link.values}' must carry dim " diff --git a/src/math_spec/program.py b/src/math_spec/program.py index 4521e973..b55f0232 100644 --- a/src/math_spec/program.py +++ b/src/math_spec/program.py @@ -392,12 +392,16 @@ class Named: ) -def fan_in(expression: Expression) -> FanIn: +def fan_in(expression: Expression | Named) -> FanIn: """How *expression*'s output rows relate to its input slots. For the absence rules, both classes other than ``'one-to-one'`` sum - several input slots into an output row. + several input slots into an output row. A :class:`Named` answers as its + body does, so a :attr:`~math_spec.model.Spec.resolved` tree is asked as a + program's is. """ + if isinstance(expression, Named): + return fan_in(expression.body) if isinstance(expression, (Sum, GroupSum)): return 'many-to-one' if isinstance(expression, WindowSum): diff --git a/src/math_spec/resolution.py b/src/math_spec/resolution.py index dcf68ee6..9fbf1e0c 100644 --- a/src/math_spec/resolution.py +++ b/src/math_spec/resolution.py @@ -20,6 +20,7 @@ import math_spec.degree as degree from math_spec._expression_parser import ( + MAX_DEPTH, ArithmeticNode, BinaryOperatorNode, ComparisonNode, @@ -29,6 +30,7 @@ NameNode, NumberNode, UnaryOperatorNode, + depth, nodes, shown, ) @@ -38,6 +40,7 @@ UnresolvedCountNode, UnresolvedPredicateCallNode, UnresolvedWhereNode, + nested, parse_where, ) from math_spec.dimensions import dims_of, pulled_back_dims @@ -100,13 +103,15 @@ VariableDefined, WindowSum, carries_variable, + children, walk, ) if TYPE_CHECKING: from collections.abc import Iterable, Mapping - from math_spec._expression_parser import ComparisonOperator + from math_spec._expression_parser import ComparisonOperator, ParsedNode + from math_spec._where_parser import ParsedWhere from math_spec.model import DeclaredDtype, ExpressionBlock, Spec @@ -119,6 +124,12 @@ #: the vacated positions contribute where it does not. _Edge = tuple[bool, float | None] +#: How deep a resolved tree may be with every named expression it reads +#: written in — the tree every pass after resolution recurses over. Three +#: times what one text may nest, since a text reads other texts: a chain of +#: 200 entries survived every pass on a default stack and 250 did not. +MAX_RESOLVED_DEPTH = 3 * MAX_DEPTH + class Namespace: """The declared names of one schema, by kind — the whole of what a file may name, read once. @@ -171,7 +182,8 @@ def __init__(self, schema: Spec) -> None: #: named expression -> its resolved node, or ``None``, and its refusals; #: filled the first time anything reads the name. self._named: dict[str, tuple[Named | None, tuple[str, ...]]] = {} - #: The named expressions being resolved, outermost first — a cycle's chain. + #: The named expressions waiting to be resolved, the one asked for + #: first — each above the entries it reads, so it is a cycle's chain. self._loading: list[str] = [] def named(self, name: str, context: str) -> Named: @@ -183,28 +195,78 @@ def named(self, name: str, context: str) -> Named: Raises: SchemaError: The entry reads itself, or does not load. """ - if name in self._loading: - chain = ' -> '.join([*self._loading[self._loading.index(name) :], name]) - msg = f'{context}: circular expression reference: {chain}' - raise SchemaError(msg) + if (refusal := self.cycle(name, context)) is not None: + raise SchemaError(refusal) node, _ = self.named_entry(name) if node is None: msg = f"{context}: named expression '{name}' does not load. Its refusal is listed with it." raise SchemaError(msg) return node + def cycle(self, name: str, context: str, through: Iterable[str] = ()) -> str | None: + """The refusal for reading *name* while it is being resolved, or ``None``; *through* names the macros the read went through.""" + if name not in self._loading: + return None + chain = ' -> '.join([*self._loading[self._loading.index(name) :], *through, name]) + return f'{context}: circular expression reference: {chain}' + def named_entry(self, name: str) -> tuple[Named | None, tuple[str, ...]]: - """The ``expressions:`` entry *name* resolved, or ``None``, with every refusal it earned.""" - if name not in self._named: - errors: list[str] = [] - self._loading.append(name) - try: - node = _named(name, self.schema.expressions[name], self, errors) - finally: + """The ``expressions:`` entry *name* resolved, or ``None``, with every refusal it earned. + + The entries it reads are resolved before it, walked from a stack that + holds the path of reads from *name* rather than by recursing into + each, so a chain of entries however long costs no stack. An entry + that reads one on the path is a cycle, which :meth:`named` refuses + with that path when the resolution reaches the read. + """ + base = len(self._loading) + self._loading.append(name) + while len(self._loading) > base: + top = self._loading[-1] + if top in self._named: self._loading.pop() - self._named[name] = (node, tuple(errors)) + continue + waiting = [n for n in self._references(top) if n not in self._named and n not in self._loading] + if waiting: + self._loading.append(waiting[0]) + continue + errors: list[str] = [] + node = _named(top, self.schema.expressions[top], self, errors) + self._named[top] = (node, tuple(errors)) + self._loading.pop() return self._named[name] + def _references(self, name: str) -> tuple[str, ...]: + """The ``expressions:`` entries the texts of entry *name* read, macros expanded, in first-mention order. + + A text that does not parse or expand reads nothing here: the + resolution that follows reports it. + """ + block, context = self.schema.expressions[name], f"Named expression '{name}'" + arithmetic: list[ParsedNode] = [] + for text in (block.expression, *(case.expression for case in (block.cases or {}).values()), block.otherwise): + if text is not None: + try: + arithmetic.append(parse_and_expand(text, self, context)) + except ValueError: + continue + for case in (block.cases or {}).values(): + try: + pending: list[ParsedWhere] = [parse_where(case.when)] + except ValueError: + continue + while pending: + node = pending.pop() + if isinstance(node, ArithmeticNode): + try: + arithmetic.append(expand(node, self, context)) + except ValueError: + continue + else: + pending.extend(nested(node)) + names = (n.name for n in nodes(*arithmetic) if isinstance(n, NameNode) and n.name in self.schema.expressions) + return tuple(dict.fromkeys(names)) + def kind(self, name: str) -> DeclarationKind | None: """What *name* was declared as, or ``None`` where the file declares it nowhere.""" if name in self.variables: @@ -357,9 +419,10 @@ def resolve_expression( where a name in *formals* stands under *node*: a macro template is checked by the rules a call site is before anything calls it, and only the call site that binds its formals has a tree to build. + """ before = len(errors) - resolved = _Resolver(ns, context, errors, formals=formals).arith(node) + resolved = _Resolver(ns, context, errors, formals=formals).build(node) return None if len(errors) > before else resolved @@ -451,7 +514,7 @@ def resolve_constraint_text( return None found = len(errors) resolver = _Resolver(ns, context, errors) - left, right = resolver.arith(ast.left), resolver.arith(ast.right) + left, right = resolver.build(ast.left), resolver.build(ast.right) if len(errors) > found or left is None or right is None: return None if any(_over_the_ceiling(side, context, errors, ceiling=2) for side in (left, right)): @@ -567,6 +630,24 @@ def _formal(self, value: ArithmeticNode) -> bool: # -- expressions ------------------------------------------------------- + def build(self, node: ArithmeticNode) -> Expression | None: + """The program tree *node* stands for, held to :data:`MAX_RESOLVED_DEPTH` before anything walks it. + + The depth is measured with every named expression written in, since + that is the tree every later pass recurses over, and measured with an + explicit stack, since a recursion would be the crash it prevents. + """ + resolved = self.arith(node) + if resolved is None or (found := depth(resolved, children)) <= MAX_RESOLVED_DEPTH: + return resolved + self.errors.append( + f'{self.context}: the expression nests {found} deep with every named expression it reads written in, ' + f'past the {MAX_RESOLVED_DEPTH} levels the language admits. Reduce over a dimension with sum() rather ' + f'than writing the terms out, or precompute the deepest part as a parameter — a named expression ' + f'stands inline where it is read, so naming a part does not make the tree shallower.' + ) + return None + def arith(self, node: ArithmeticNode) -> Expression | None: """The program node *node* stands for, or ``None``. @@ -686,7 +767,8 @@ def _call(self, node: FunctionCallNode) -> Expression | None: args = [self.arith(a) for a in node.args] with_relation = any(k in node.kwargs for k in builtin.relation_kwargs) roles = {k: v for k, v in node.kwargs.items() if builtin.kind_of(k, with_relation=with_relation) == 'role'} - if roles and 'by' not in node.kwargs: + unrelated = bool(roles) and 'by' not in node.kwargs + if unrelated: self.errors.append( f'{self.context}: {node.name}({", ".join(f"{k}=" for k in roles)}) names a column of a relation, ' f'and no by= names the relation. Write {builtin.usage}' @@ -709,6 +791,7 @@ def _call(self, node: FunctionCallNode) -> Expression | None: read = self._relation_ref(node.kwargs['by'], node.name, 'by', roles, dims.get('along')) unread = ( shape_error is not None + or unrelated or not args or args[0] is None or None in dims.values() @@ -832,6 +915,13 @@ def _amount(self, value: ArithmeticNode, operator: str, key: str) -> int | str | if self._name(bare) is not None: self.errors.append(f'{self.context}: {operator}({key}=...) {words.form}') return None + if (dtype := self.ns.dtypes[bare.name]) != 'int': + self.errors.append( + f"{self.context}: {operator}({key}={bare.name}) counts positions, but '{bare.name}' is declared " + f'dtype: {dtype}. A count of positions is integral — declare it dtype: int, which binds only an ' + f'integer column, so a fractional {words.noun} has nowhere to arrive from.' + ) + return None if isinstance(value, UnaryOperatorNode) and value.op == '-': self.errors.append( f'{self.context}: {operator}({key}=-{bare.name}) negates a named {words.noun}. {words.negated}' @@ -1358,7 +1448,7 @@ def _expression_comparison(self, node: UnresolvedComparisonNode) -> ExpressionCo except ValueError as e: self.errors.append(prefixed(context, e)) continue - if (resolved := self.arith(expanded)) is not None: + if (resolved := self.build(expanded)) is not None: sides.append(resolved) if len(self.errors) > found: return node diff --git a/src/math_spec/typesetting/walk.py b/src/math_spec/typesetting/walk.py index 2f6c7aec..7d018928 100644 --- a/src/math_spec/typesetting/walk.py +++ b/src/math_spec/typesetting/walk.py @@ -57,7 +57,6 @@ VariableDefined, WindowSum, ) -from math_spec.resolution import remainder from math_spec.typesetting.format import Entry, Line, OperatorName if TYPE_CHECKING: @@ -792,15 +791,13 @@ def _arms(self, node: Cases, ctx: _Context) -> list[tuple[str, str]]: """Each region as its value and the words saying where it applies. Which region is the fallback is a fact about the math, so the *walk* - chooses between "if" and "otherwise" and a Format only stacks the - rows: the last region is the ``otherwise`` where its mask is the - remainder of the others, which is how resolution builds it. + says "if" or "otherwise" and a Format only stacks the rows: the last + region is the ``otherwise``, since resolution builds it as the + remainder of the others, and its mask is never printed. """ *stated, last = node.regions arms = [(self._expression(region.value, ctx), self._arm_condition(region.when, ctx)) for region in stated] - left_over = bool(stated) and last.when == remainder(region.when for region in stated) - when = self.format.prose('otherwise') if left_over else self._arm_condition(last.when, ctx) - return [*arms, (self._expression(last.value, ctx), when)] + return [*arms, (self._expression(last.value, ctx), self.format.prose('otherwise'))] def _arm_condition(self, when: Mask, ctx: _Context) -> str: return f'{self.format.prose("if ")} {self._predicate(when.root, ctx, need=_WHERE_PRECEDENCE["and"])}' diff --git a/src/math_spec/validation.py b/src/math_spec/validation.py index 1e47c205..fe79d07b 100644 --- a/src/math_spec/validation.py +++ b/src/math_spec/validation.py @@ -22,6 +22,7 @@ Mask, ObjectiveDeclaration, VariableDefined, + carries_variable, ) from math_spec.resolution import ( Namespace, @@ -34,10 +35,11 @@ ) if TYPE_CHECKING: + from collections.abc import Iterator from pathlib import Path - from math_spec.model import AssumptionBlock - from math_spec.program import Named + from math_spec.model import AssumptionBlock, PiecewiseBlock + from math_spec.program import Expression, Named def to_spec(model: str | Path | Mapping[str, object] | Spec) -> Spec: @@ -121,8 +123,6 @@ def validate_expressions(schema: Spec) -> Resolved: errors.extend(refusals) if node is not None: expressions[ename] = node - if errors: - raise SchemaError('\n'.join(errors)) variables = { vname: mask_of(resolve_where_text(vdef.where, ns, f"Variable '{vname}'", errors, self_variable=vname)) @@ -159,8 +159,12 @@ def validate_expressions(schema: Spec) -> Resolved: resolve_expression_text(link.expression, ns, f"piecewise '{pname}' link {i}", errors, ceiling=1) for i, link in enumerate(pdef.links) ] - if all(link is not None for link in links): - piecewise[pname] = tuple(link for link in links if link is not None) + if any(link is None for link in links): + continue + typed = tuple(link for link in links if link is not None) + if pdef.method == 'lp': + errors.extend(_domain_decides_nothing(pname, pdef, typed)) + piecewise[pname] = typed if errors: raise SchemaError('\n'.join(errors)) @@ -172,6 +176,26 @@ def validate_expressions(schema: Spec) -> Resolved: return resolved +def _domain_decides_nothing(name: str, pw: PiecewiseBlock, links: tuple[Expression, ...]) -> Iterator[str]: + """The refusal for a ``method: lp`` curve whose x-link carries no variable. + + The method bounds the curve's domain with two rows comparing the x-link + against the first and the last breakpoint, and a row with no variable + decides nothing. Decided here, on the link the file wrote, rather than on + the row the expansion would write under a name the file never declared. + """ + x = pw.curve[0] + i = next(i for i, link in enumerate(pw.links) if link is x) + if carries_variable(links[i]): + return + yield ( + f"piecewise '{name}' link {i}: method: lp bounds the curve's domain by rows comparing this link's expression " + f'against its first and last breakpoint, and {x.expression!r} carries no variable, so those rows decide ' + f'nothing. Name a variable in the link, or use method: convex, sos2 or adjacency, whose weights pin the ' + f'domain themselves.' + ) + + def _assumption(name: str, block: AssumptionBlock, ns: Namespace, errors: list[str]) -> Assumption | None: """One ``assumptions:`` entry typed, or ``None`` once anything in it failed. diff --git a/tests/fixtures.py b/tests/fixtures.py index 777b9751..a98a2a4f 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -13,7 +13,7 @@ from math_spec import Spec from math_spec._expression_parser import ComparisonNode from math_spec._yaml import parse_yaml, read_yaml -from math_spec.errors import LanguageError +from math_spec.errors import SchemaError from math_spec.expansion import parse_and_expand from math_spec.resolution import Namespace, mask_of, resolve_expression, resolve_where_text from math_spec.validation import to_spec @@ -98,13 +98,13 @@ def raw_of(source: str | Path | dict[str, Any]) -> dict[str, Any]: def expression_of(text: str, ns: Namespace, context: str) -> Expression: - """Parse, expand and resolve one expression into its program tree, raising every problem at once rather than collecting.""" + """Parse, expand and resolve one expression into its program tree, raising every problem at once as `to_spec` would.""" errors: list[str] = [] ast = parse_and_expand(text, ns, context) assert not isinstance(ast, ComparisonNode), 'a comparison is a constraint, which comparison_of reads' resolved = resolve_expression(ast, ns, context, errors) if errors: - raise LanguageError('\n'.join(errors)) + raise SchemaError('\n'.join(errors)) assert resolved is not None return resolved @@ -116,7 +116,7 @@ def comparison_of(text: str, ns: Namespace, context: str) -> tuple[Expression, s assert isinstance(ast, ComparisonNode), 'a value is an expression, which expression_of reads' left, right = (resolve_expression(side, ns, context, errors) for side in (ast.left, ast.right)) if errors: - raise LanguageError('\n'.join(errors)) + raise SchemaError('\n'.join(errors)) assert left is not None and right is not None return left, ast.op, right @@ -126,5 +126,5 @@ def where_of(text: str | None, ns: Namespace, context: str, self_variable: str | errors: list[str] = [] resolved = resolve_where_text(text, ns, context, errors, self_variable) if errors: - raise LanguageError('\n'.join(errors)) + raise SchemaError('\n'.join(errors)) return mask_of(resolved) diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 01c5932d..b286b362 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -11,7 +11,7 @@ import pytest from math_spec.dimensions import DimensionError, _check_where_dims, dims_of -from math_spec.errors import LanguageError +from math_spec.errors import SchemaError from math_spec.program import Mask, RelationPairComparison from math_spec.resolution import Namespace from math_spec.validation import to_spec @@ -247,83 +247,114 @@ def test_a_bare_name_reaches_the_variable_a_dual_the_same_named_constraint(): @pytest.mark.parametrize( - ('expr', 'match'), + ('expr', 'error', 'match'), [ pytest.param( 'sum(p, over=bus)', + DimensionError, r'sum\(over=bus\) but the expression has dims', id='sum-consuming-an-absent-dim-is-an-error-not-a-noop', ), pytest.param( 'sum(sum(p))', + SchemaError, r'the expression is already a scalar', id='a-bare-sum-of-a-scalar-is-an-error-not-a-noop', ), + pytest.param( + 'sum(sum(p, over=bus))', + SchemaError, + r'sum\(over=bus\) but the expression has dims', + id='a-dim-fault-under-a-bare-sum-is-met-while-the-sum-is-built', + ), + pytest.param( + 'sum(sum(p, over=bus), over=generator)', + DimensionError, + r'sum\(over=bus\) but the expression has dims', + id='and-the-same-fault-under-a-sum-over-a-named-dim-is-the-dim-rules', + ), pytest.param( 'sum(load, by=gen_bus, over=generator, into=bus)', + DimensionError, r"sum\(by=gen_bus\) consumes \['generator'\], the dims it reads from", id='sum-requires-the-grouped-dim', ), pytest.param( "shift(cost, along=snapshot, offset=1, edge='wrap')", + DimensionError, r'shift\(along=snapshot\) but the expression has dims', id='shift-requires-the-dim', ), pytest.param( "shift(p, along=snapshot, offset=cost, edge='wrap')", + SchemaError, r'declared dtype: float', id='a-named-offset-is-integral-58', ), pytest.param( "shift(p, along=snapshot, offset=horizon, edge='wrap')", + DimensionError, r'varies over the axis it steps along is a permutation rather than a lag', id='a-named-offset-does-not-span-the-axis-it-steps-along', ), pytest.param( 'sum_back(p, along=snapshot, window=cost)', + SchemaError, r'declared dtype: float', id='a-named-width-is-integral', ), pytest.param( 'sum_back(p, along=snapshot, window=horizon)', + DimensionError, r'no longer "the last n"', id='a-named-width-does-not-span-the-summed-axis', ), pytest.param( "shift(p, along=snapshot, offset=-spinup, edge='wrap')", + SchemaError, r'negates a named offset', id='a-named-offset-is-not-negated-at-the-call-62', ), pytest.param( 'sum_back(p, along=snapshot, window=-spinup)', + SchemaError, r'which way a window reaches is the operator', id='a-named-width-has-no-direction-to-negate', ), pytest.param( "shift(p, along=snapshot, offset=bus_lead, edge='wrap')", + DimensionError, r"varies over \['bus'\], which that coordinate does not carry", id='a-named-offset-is-read-where-the-expression-has-a-coordinate', ), pytest.param( 'sum(cost, by=gen_zone, over=generator, into=zone)', + DimensionError, r"sum\(by=gen_zone\) joins on \['snapshot'\]", id='a-grouped-sum-needs-the-keys-it-joins-on', ), pytest.param( 'at(zone_cap, by=gen_zone, into=generator, over=zone)', + DimensionError, r"at\(by=gen_zone\) joins on \['snapshot'\]", id='a-pullback-needs-the-keys-it-joins-on', ), pytest.param( "shift(cost, along=generator, offset=1, edge='wrap', by=gen_zone, within=zone)", + DimensionError, r"by=gen_zone\) joins on \['snapshot'\]", id='a-partition-needs-the-keys-it-joins-on', ), ], ) -def test_an_ill_dimensioned_expression_is_rejected(expr, match): - """A rule on the operand's dims is the dim checker's; one on the form of an amount is resolution's, so the class is the language's.""" - with pytest.raises(LanguageError, match=match): +def test_an_ill_dimensioned_expression_is_rejected(expr, error, match): + """The class says which pass refused: what resolution needs to build a node is a `SchemaError`, and a rule on a built tree's dims a `DimensionError`. + + A dim fault under a bare `sum()` is met while the sum is built, since the + dims it reduces are the operand's, and the same fault under a sum over a + named dim is met by the dim rules. + """ + with pytest.raises(error, match=match): _dims(expr) @@ -421,8 +452,9 @@ class TestTheEdgeRulesAreDecidedAtLoad: } def _refused(self, expression: str) -> str: + """The message `to_spec` refuses *expression* with — a `SchemaError`, since every rule here is resolution's.""" raw = override(self.BASE, **{'constraints.k.expression': expression}) - with pytest.raises(LanguageError) as caught: + with pytest.raises(SchemaError) as caught: to_spec(raw) return str(caught.value) diff --git a/tests/test_exclusivity.py b/tests/test_exclusivity.py index c64a75c2..d6659db6 100644 --- a/tests/test_exclusivity.py +++ b/tests/test_exclusivity.py @@ -213,9 +213,11 @@ def test_a_literal_written_first_is_named_as_the_order_it_is(self, schema: Spec) pytest.param('-2 < capacity', 'capacity > -2', id='a-signed-literal'), pytest.param('2.5 >= capacity', 'capacity <= 2.5', id='a-fraction-and-a-flipped-comparator'), pytest.param('0 != age', 'age != 0', id='a-comparator-that-is-its-own-mirror'), + pytest.param('123456789 < capacity', 'capacity > 123456789', id='a-literal-longer-than-six-digits'), ], ) def test_the_rewrite_is_the_same_test_with_the_sides_swapped(self, schema: Spec, when: str, rewrite: str): + """The literal is quoted as the file wrote it: rounded to six digits, `123456789` became `1.23457e+08`, a different test.""" [refusal] = refusals(schema, {'one': when, 'two': 'cyclic'}) assert rewrite in refusal diff --git a/tests/test_expansion.py b/tests/test_expansion.py index 5117cfbe..6bc8f6ba 100644 --- a/tests/test_expansion.py +++ b/tests/test_expansion.py @@ -13,7 +13,7 @@ from math_spec.errors import LanguageError from math_spec.expansion import parse_and_expand from math_spec.lowering import inline -from math_spec.program import Multiply, Named, Parameter, Sum, Variable +from math_spec.program import Multiply, Named, Parameter, Sum, Translate, Variable from math_spec.resolution import Namespace from tests.fixtures import DISPATCH_MODEL, SMALL_MODEL, comparison_of, expression_of, schema_of @@ -148,6 +148,37 @@ def test_a_bad_named_expression_is_refused_at_load(expressions, match): schema(expressions=expressions) +@pytest.mark.parametrize( + ('expressions', 'macros', 'chain'), + [ + pytest.param({'a': 'b + 1', 'b': 'a + 1'}, {}, 'a -> b -> a', id='through-an-entry'), + pytest.param( + {'a': 'm(load)'}, {'m': {'args': ['x'], 'template': 'x + a'}}, 'a -> m -> a', id='through-a-macro' + ), + pytest.param( + {'a': 'b + 1', 'b': 'm(1)'}, + {'m': {'args': ['x'], 'template': 'x + a'}}, + 'a -> b -> m -> a', + id='through-an-entry-and-a-macro', + ), + pytest.param( + { + 'a': {'dims': ['snapshot'], 'cases': {'x': {'when': 'b > 0', 'expression': '1'}}, 'otherwise': '2'}, + 'b': 'a + 1', + }, + {}, + 'a -> b -> a', + id='through-the-when-of-a-case', + ), + ], +) +def test_a_cycle_is_reported_with_the_chain_that_closes_it(expressions, macros, chain): + """A cycle closed through a macro was reported as `a -> a`, the macro left out, and one closed through a case's `when` was a `RecursionError`.""" + with pytest.raises(LanguageError, match=f'circular expression reference: {chain}$') as exc: + schema(expressions=expressions, macros=macros) + assert str(exc.value).count('circular') == 1, 'the cycle is reported once, where it closes' + + def test_a_refusal_names_its_context_once(): with pytest.raises(LanguageError) as exc: schema(expressions={'a': 'a + 1'}) @@ -248,6 +279,11 @@ def test_macro_collisions_rejected(patch, match): r"Macro 'reduced'.*sum\(over=nope\) does not name a declared dimension or a formal of this macro", id='a-typo-in-a-dimension', ), + pytest.param( + {'lag': {'args': ['x'], 'template': 'shift(x, along=snapshot, offset=0.5)'}}, + r"Macro 'lag'.*shift\(offset=...\) must be a whole number", + id='a-fractional-offset', + ), ], ) def test_macro_templates_validated_even_when_unused(macros, match): @@ -255,12 +291,19 @@ def test_macro_templates_validated_even_when_unused(macros, match): The relation beside formal columns loaded once the columns were formals, because the formals sent the call back before the relation's name was - read. + read; a fractional offset in a template nothing calls loaded before the + form of an amount was decided in resolution. """ with pytest.raises(LanguageError, match=match): schema(macros=macros) +def test_an_entry_nothing_reads_is_held_to_the_rules_a_use_is(): + """`sum(k)` over a scalar loaded as an unread entry, since the bare sum was only decided where the math read it.""" + with pytest.raises(LanguageError, match=r"Named expression 'e1': sum\(\) with no over= or by=.*already a scalar"): + schema_of(SMALL_MODEL, expressions={'e1': 'sum(k)'}) + + @pytest.mark.parametrize( ('template', 'match'), [ @@ -307,14 +350,13 @@ def test_a_formal_stands_where_a_call_site_will_bind_it(formals, template): ) -def test_a_call_binding_the_dimension_a_partition_steps_along_loads(): - """The call site is where the formal gets its kind, so the partition is read there.""" +def test_a_call_binding_the_dimension_a_partition_steps_along_builds_it(): + """The call site is where the formal gets its kind, so the partition is built there.""" template = 'shift(x, along=d, offset=1, by=lk, within=h)' - schema_of( - SMALL_MODEL, - macros={'m': {'args': ['x', 'd'], 'template': template}}, - constraints={'c': {'dims': ['g'], 'expression': 'm(p, g) <= 1'}}, - ) + ns = Namespace(schema_of(SMALL_MODEL, macros={'m': {'args': ['x', 'd'], 'template': template}})) + node = expression_of('m(p, g)', ns, 'expression') + assert isinstance(node, Translate) and node.along == 'g' + assert node.partition is not None and node.partition.name == 'lk', 'the relation is read once along= is bound' def test_a_named_expression_is_resolved_once_however_many_uses(monkeypatch): diff --git a/tests/test_lowering.py b/tests/test_lowering.py index ff7586c9..cd2d1258 100644 --- a/tests/test_lowering.py +++ b/tests/test_lowering.py @@ -40,6 +40,7 @@ GroupSum, Mask, Multiply, + Named, Negate, Not, Or, @@ -602,6 +603,11 @@ def test_a_power_resolves_to_a_node_of_its_own(dispatch_schema): Translate(Variable('p'), 'g', offset='lead', wrap=False, fill=0.0), id='a-named-offset-crosses-as-the-parameter-name', ), + pytest.param( + "shift(p, along=g, offset=+lead, edge='wrap')", + Translate(Variable('p'), 'g', offset='lead', wrap=True, fill=None), + id='a-named-offset-written-with-a-plus-is-the-parameter', + ), pytest.param( 'shift(p, along=g, offset=1, by=lk, within=h, edge=0)', Translate( @@ -620,8 +626,8 @@ def test_a_power_resolves_to_a_node_of_its_own(dispatch_schema): id='a-window-is-one-node-rather-than-a-fold-of-translations', ), pytest.param( - 'sum_back(p, along=g, window=k)', - WindowSum(Variable('p'), 'g', width='k', wrap=False), + 'sum_back(p, along=g, window=lead)', + WindowSum(Variable('p'), 'g', width='lead', wrap=False), id='a-named-width-crosses-as-the-parameter-name', ), pytest.param( @@ -856,6 +862,12 @@ def test_a_node_answers_its_fan_in(node, expected): assert fan_in(node) == expected +def test_fan_in_reads_through_a_named_expression(): + """`fan_in` on a `Spec.resolved` tree, which holds `Named`, ended in `assert_never`.""" + named = Named('total', Sum(Variable('p'), ('g',))) + assert fan_in(named) == 'many-to-one', 'a use of an entry fans in as the entry does' + + def test_a_relation_is_declared_as_the_file_declares_it(): """One group keyed by name, each entry its columns and its key, and nothing nested under a dimension.""" program = to_program( diff --git a/tests/test_piecewise.py b/tests/test_piecewise.py index 46601e11..e603e567 100644 --- a/tests/test_piecewise.py +++ b/tests/test_piecewise.py @@ -227,6 +227,18 @@ def test_any_affine_expression_is_a_legal_link(link): 'activity is not supported with method: lp', id='lp-with-an-activity-and-nothing-to-gate', ), + pytest.param( + NONCONVEX_YAML, + {'parameters.bp_x.dtype': 'bool'}, + "link 0 values parameter 'bp_x' is declared dtype: bool, and a breakpoint is a number", + id='values-that-are-not-numbers', + ), + pytest.param( + LP, + {'piecewise.cost_curve.links': [['load', 'bp_x'], ['op_cost', 'bp_y', '>=']]}, + "link 0: method: lp bounds the curve's domain by rows comparing this link's expression", + id='lp-with-an-x-link-carrying-no-variable', + ), ], ) def test_a_malformed_block_is_refused(model, patch, match): @@ -252,6 +264,48 @@ def test_a_link_outside_the_language_is_named_where_the_user_wrote_it(link_expre assert "piecewise 'cost_curve' link 0" in str(exc.value) +@pytest.mark.parametrize( + ('model', 'patch'), + [ + pytest.param(NONCONVEX_YAML, {'parameters.bp_x.dtype': 'str'}, id='a-label-as-a-breakpoint'), + pytest.param( + LP, + {'piecewise.cost_curve.links': [['load', 'bp_x'], ['op_cost', 'bp_y', '>=']]}, + id='a-variable-free-x-link', + ), + ], +) +def test_a_block_is_refused_on_the_link_the_file_wrote_and_not_on_a_row_it_would_emit(model, patch): + """Both were refused only once written out, under `cost_curve_increasing` or `cost_curve_domain_lo` — rows the file never declared.""" + with pytest.raises(SchemaError) as exc: + schema_of(model, **patch) + assert "piecewise 'cost_curve'" in str(exc.value) and 'link 0' in str(exc.value) + assert 'cost_curve_' not in str(exc.value), 'the refusal names the block, not a declaration the expansion writes' + + +def test_an_undeclared_breakpoint_dimension_is_refused_once(): + """`over: nope` also said, per link, that the values parameter must carry `nope` — lines that follow from the first.""" + with pytest.raises(SchemaError) as exc: + schema_of(NONCONVEX_YAML, **{'piecewise.cost_curve.over': 'nope'}) + assert str(exc.value).splitlines() == [ + "piecewise 'cost_curve' references undeclared dimension 'nope'. Declare it under 'dimensions:'." + ] + + +def test_a_link_reading_a_refused_entry_names_it_and_its_refusal_is_listed(): + """A link through a failing entry said `Its refusal is listed with it`, and nothing listed the refusal.""" + with pytest.raises(SchemaError) as exc: + schema_of( + NONCONVEX_YAML, + **{'expressions': {'bad': 'nope'}, 'piecewise.cost_curve.links': [['bad', 'bp_x'], ['op_cost', 'bp_y']]}, + ) + message = str(exc.value) + assert "Named expression 'bad': 'nope' not found" in message + assert ( + "piecewise 'cost_curve' link 0: named expression 'bad' does not load. Its refusal is listed with it." in message + ) + + def test_a_link_reading_a_nonlinear_entry_is_refused(): """A named entry, nonlinear and so legal on its own, is refused where the link reads it. diff --git a/tests/test_validation.py b/tests/test_validation.py index 93bdd3f9..4990a6a9 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -112,6 +112,25 @@ def test_multiple_errors_collected(self): assert "'nope' not found" in message assert 'exactly one comparison' in message, 'the second fault is reported beside the first, not behind it' + @pytest.mark.parametrize( + 'patch', + [ + pytest.param({'macros': {'m': {'args': ['x'], 'template': 'x + nope'}}}, id='a-macro'), + pytest.param({'expressions': {'e': 'nope'}}, id='a-named-expression'), + ], + ) + def test_a_fault_in_a_macro_or_an_entry_hides_no_other_fault(self, patch): + """Validation raised after the macros and the `expressions:` entries, so a constraint's fault waited for the next load.""" + message = _refusal(**patch, constraints={'c': {'dims': ['g'], 'expression': 'p <= also_nope'}}) + assert "'nope' not found" in message + assert "Constraint 'c': 'also_nope' not found" in message, 'every declaration is read, whatever an entry did' + + def test_a_refused_call_is_not_read_by_the_call_around_it(self): + """`sum(sum(p, into=g))`: the inner call names a column with no `by=`, and the outer bare sum then said its operand was already a scalar, because the refused call was still built.""" + message = _refusal(objective={'expression': 'sum(sum(p, into=g))'}) + assert 'no by= names the relation' in message + assert 'already a scalar' not in message, 'a refused call builds nothing for the call around it to read' + @pytest.mark.parametrize( ('patch', 'fragments'), [ @@ -464,7 +483,7 @@ def test_the_position_it_is_declared_for_still_takes_it(self, dtype, where): def test_a_named_amount_keeps_its_own_sentence(self): """`offset=` has a stricter rule of its own — a count of positions is integral — and that sentence arrives.""" - with pytest.raises(LanguageError, match='counts positions along'): + with pytest.raises(SchemaError, match="counts positions, but 'lag' is declared dtype: str"): _schema( **{ 'parameters.lag': {'dims': [], 'dtype': 'str'}, @@ -472,6 +491,17 @@ def test_a_named_amount_keeps_its_own_sentence(self): } ) + def test_a_negated_named_amount_is_first_held_to_its_dtype(self): + """`offset=-lag` with `lag` a str parameter said only that the offset was negated, and nothing about the dtype.""" + message = _refusal( + **{ + 'parameters.lag': {'dims': [], 'dtype': 'str'}, + 'objective': {'expression': "sum(shift(p, along=g, offset=-lag, edge='wrap'))"}, + } + ) + assert "'lag' is declared dtype: str" in message + assert 'negates a named offset' not in message, 'the sign is read once the name is a count' + class TestVersion: """`version:` is refused when unknown, and does nothing else (#67).""" @@ -1836,7 +1866,10 @@ def test_a_fault_in_an_arm_names_the_declaration_and_is_reported_once(self): message = _refusal(model) assert message.count("'nope' not found") == 1, 'two constraints read it; the fault is reported once' assert "Named expression 'headroom', case 'opening'" in message - assert 'Constraint' not in message, "the arm is the declaration's, not the use site's" + for name in ('cap', 'floor'): + assert f"Constraint '{name}': named expression 'headroom' does not load" in message, ( + 'a use site names the entry it could not read, and does not repeat its fault' + ) def test_the_fallback_is_not_named_as_a_case(self): """`otherwise:` is what is left, not a region like the cases are. @@ -1966,6 +1999,37 @@ def test_an_expression_too_deep_to_walk_fails_as_a_language_error(patch, nests): to_spec(override(DISPATCH_MODEL, **patch)) +def _chain(n: int, *, deepest_first: bool) -> dict[str, str]: + """*n* named expressions, each reading the one before; the body of `e{n-1}` resolves `2n - 1` deep with every entry written in.""" + entries = {'e0': 'load'} | {f'e{i}': f'e{i - 1} + 1' for i in range(1, n)} + return dict(reversed(list(entries.items()))) if deepest_first else entries + + +@pytest.mark.parametrize('deepest_first', [True, False], ids=['declared-deepest-first', 'declared-deepest-last']) +def test_a_chain_of_named_expressions_is_held_to_the_resolved_depth_and_costs_no_stack(deepest_first): + """A chain of entries recursed once per entry, so a long one raised `RecursionError`, or the parser's refusal about a tree that was not deep. + + Where the interpreter's stack ran out decided the message: a chain of 120 + declared deepest first was a `RecursionError` out of `to_spec`, and one + whose parse was the deepest frame was refused as nesting past the 100 + levels a text may, which the text did not. The entries are now resolved + in an order that reads each one's dependencies first, from a worklist, + and the resolved tree is held to one depth however it was written. + """ + chain = _chain(150, deepest_first=deepest_first) + constraint = {'dims': ['snapshot'], 'expression': 'sum(p, over=generator) <= e149'} + spec = to_spec(override(DISPATCH_MODEL, expressions=chain, **{'constraints.c': constraint})) + to_markdown(to_program(spec) and spec) + + with pytest.raises(LanguageError, match='nests 301 deep with every named expression it reads written in') as caught: + to_spec(override(DISPATCH_MODEL, expressions=_chain(151, deepest_first=deepest_first))) + assert 'past the 300 levels' in str(caught.value) + assert "Named expression 'e150'" in str(caught.value), 'refused at the first entry past the depth, by name' + + with pytest.raises(LanguageError, match='past the 300 levels'): + to_spec(override(DISPATCH_MODEL, expressions=_chain(400, deepest_first=deepest_first))) + + def test_a_name_may_open_with_an_underscore(): """`expressions.md` said a name opens with a letter while the schema and the grammar both admitted `_`, so the page refused what the language accepts.""" schema = to_spec( diff --git a/tests/typesetting/test_golden.py b/tests/typesetting/test_golden.py index c063dd01..dc75b4d7 100644 --- a/tests/typesetting/test_golden.py +++ b/tests/typesetting/test_golden.py @@ -191,7 +191,6 @@ def test_the_golden_model_calls_every_operator_in_the_language(): #: model takes — a file declares at most one — and #: `test_a_model_with_no_objective_prints_the_rest` covers it. UNREACHABLE = { - 'if isinstance(node, Cases):', 'return self.format.cases(self._arms(node, ctx)), _ATOM', 'assert_never(node)', 'assert_never(check)', From 3f4b92c0221a877a7634614119ce6cd390b86479 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 10:59:26 +0000 Subject: [PATCH 2/4] chore(resolution): the resolved depth carries the number of the PR that measured it Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015h57WkBDnpxrknuJ5zZy9F --- src/math_spec/resolution.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/math_spec/resolution.py b/src/math_spec/resolution.py index 9fbf1e0c..05dd0113 100644 --- a/src/math_spec/resolution.py +++ b/src/math_spec/resolution.py @@ -127,7 +127,7 @@ #: How deep a resolved tree may be with every named expression it reads #: written in — the tree every pass after resolution recurses over. Three #: times what one text may nest, since a text reads other texts: a chain of -#: 200 entries survived every pass on a default stack and 250 did not. +#: 200 entries survived every pass on a default stack and 250 did not (#643). MAX_RESOLVED_DEPTH = 3 * MAX_DEPTH From b01a5b6a3d592919b766f1faf9bee573185d484e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 11:07:27 +0000 Subject: [PATCH 3/4] test(piecewise): every guard of a block's frame and references has the test that fails without it A mutation sweep over the guards #638 moved and added left nine green. Two were redundant with the check beside them and are gone: a formal amount is caught under its sign, and a role that named no column is caught where the roles are counted. The other seven are pinned. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015h57WkBDnpxrknuJ5zZy9F --- src/math_spec/resolution.py | 8 ++----- tests/test_piecewise.py | 46 +++++++++++++++++++++++++++++++++++++ tests/test_validation.py | 16 +++++++++++++ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/src/math_spec/resolution.py b/src/math_spec/resolution.py index 05dd0113..2357a7bf 100644 --- a/src/math_spec/resolution.py +++ b/src/math_spec/resolution.py @@ -894,8 +894,6 @@ def _amount(self, value: ArithmeticNode, operator: str, key: str) -> int | str | every parameter an amount carries, and so that a program's ``offset`` and ``width`` are the ``int | str`` they say. """ - if self._formal(value): - return None words = AMOUNTS[operator] if (literal := _literal(value)) is not None: if not (literal.value.is_integer() and literal.value >= words.minimum): @@ -1029,15 +1027,13 @@ def _relation_ref( if any(n in self.formals for v in roles.values() for n in names_in(v)): return None read = {k: self._role_name(v, operator, k) for k, v in roles.items()} - if any(r is None for r in read.values()): - return None named = {k: r for k, r in read.items() if r is not None} if operator in ('shift', 'sum_back'): if 'within' not in named: - return None # the call shape refused it already, with the wording that names the rewrite + return None # refused already, by the call shape or by the role that named no column return self._partition(name, operator, along, named['within']) if not ({'over', 'into'} <= set(named)): - return None # the call shape refused it already, with the wording that names the rewrite + return None # refused already, by the call shape or by the role that named no column return self._direction(name, operator, named['over'], named['into']) def _role_name(self, value: ArithmeticNode, operator: str, key: str) -> tuple[str, ...] | None: diff --git a/tests/test_piecewise.py b/tests/test_piecewise.py index e603e567..9166e9b2 100644 --- a/tests/test_piecewise.py +++ b/tests/test_piecewise.py @@ -239,6 +239,52 @@ def test_any_affine_expression_is_a_legal_link(link): "link 0: method: lp bounds the curve's domain by rows comparing this link's expression", id='lp-with-an-x-link-carrying-no-variable', ), + pytest.param( + NONCONVEX_YAML, + {'parameters.bp_x.dims': []}, + "link 0 values parameter 'bp_x' must carry dim 'bp'", + id='a-breakpoint-parameter-without-the-breakpoint-dim', + ), + pytest.param( + NONCONVEX_YAML, + {'piecewise.cost_curve.points': 'nope'}, + "points references undeclared parameter 'nope'", + id='undeclared-points', + ), + pytest.param( + NONCONVEX_YAML, + {'parameters.reach': {'dims': [], 'dtype': 'bool'}, 'piecewise.cost_curve.points': 'reach'}, + "points parameter 'reach' must carry dim 'bp'", + id='points-without-the-breakpoint-dim', + ), + pytest.param( + NONCONVEX_YAML, + {'piecewise.cost_curve.links': [['p + bp_x', 'bp_x'], ['op_cost', 'bp_y']]}, + "link 0 expression already carries the breakpoint dim 'bp'", + id='a-link-carrying-the-breakpoint-dim', + ), + pytest.param( + NONCONVEX_YAML, + {'variables.u': {'dims': ['snapshot', 'bp'], 'domain': 'binary'}, 'piecewise.cost_curve.activity': 'u'}, + "activity already carries the breakpoint dim 'bp'", + id='a-gate-carrying-the-breakpoint-dim', + ), + pytest.param( + NONCONVEX_YAML, + {'dimensions.generator': {'dtype': 'str'}, 'parameters.bp_x.dims': ['generator', 'bp']}, + r"values parameter 'bp_x' carries \['generator'\], which no link expression does", + id='a-breakpoint-varying-along-a-dim-no-link-carries', + ), + pytest.param( + NONCONVEX_YAML, + { + 'dimensions.generator': {'dtype': 'str'}, + 'parameters.reach': {'dims': ['generator', 'bp'], 'dtype': 'bool'}, + 'piecewise.cost_curve.points': 'reach', + }, + r"points parameter 'reach' carries \['generator'\], which the links do not", + id='a-mask-adding-a-coordinate-the-curve-does-not-have', + ), ], ) def test_a_malformed_block_is_refused(model, patch, match): diff --git a/tests/test_validation.py b/tests/test_validation.py index 4990a6a9..d9ab9993 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -339,6 +339,22 @@ def test_a_dim_kwarg_typo_is_rejected(self, expression, fragments): def test_declared_dimensions_still_pass(self, expression, dims): to_spec(_kwarg_model(expression, dims)) + @pytest.mark.parametrize( + ('expression', 'fragment'), + [ + pytest.param('sum(p, over=1)', 'sum(over=...) must name a dimension', id='a-number-as-a-dimension'), + pytest.param("sum(p, by='lk', over=g, into=h)", 'sum(by=...) must name a relation', id='a-quoted-relation'), + pytest.param( + 'sum(p, by=lk, over=1, into=h)', + 'sum(over=...) names columns of the relation', + id='a-number-as-a-column', + ), + ], + ) + def test_a_kwarg_that_names_nothing_is_refused(self, expression, fragment): + """A kwarg that takes a name and gets a number or a label is refused by the kind the operator declares for it.""" + assert fragment in _refusal(objective={'expression': expression}) + def test_macro_formals_are_not_mistaken_for_dimensions(self): """A formal in a dim position is legal inside the template body.""" _schema( From ce399f8f07f3a0f0a210a71f4f839df756b40976 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 23 Sep 2026 11:10:43 +0000 Subject: [PATCH 4/4] chore(resolution): a formal amount is read once, where a bare name is Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_015h57WkBDnpxrknuJ5zZy9F --- src/math_spec/resolution.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/math_spec/resolution.py b/src/math_spec/resolution.py index 2357a7bf..16ff529d 100644 --- a/src/math_spec/resolution.py +++ b/src/math_spec/resolution.py @@ -907,8 +907,6 @@ def _amount(self, value: ArithmeticNode, operator: str, key: str) -> int | str | f'Precompute it as a parameter.' ) return None - if self._formal(bare): - return None if self.ns.kind(bare.name) != 'parameter': if self._name(bare) is not None: self.errors.append(f'{self.context}: {operator}({key}=...) {words.form}')