diff --git a/src/math_spec/dimensions.py b/src/math_spec/dimensions.py index a9300c46..4b6e4541 100644 --- a/src/math_spec/dimensions.py +++ b/src/math_spec/dimensions.py @@ -222,7 +222,7 @@ def _dims_call( raise DimensionError( f'{context}: {node.name}(over={over.name}) but the expression has dims {sorted(inner)}.' ) - _check_named_amount(node, over.name, schema, context) + _check_named_amount(node, over.name, inner, schema, context) partition = node.kwargs.get('by') if partition is not None: assert isinstance(partition, LookupNode) @@ -252,6 +252,20 @@ def _dims_call( #: rules below. _AMOUNTS = {'shift': ('offset', 'offset'), 'sum_back': ('within', 'width')} +#: Why negating a named amount at the call site is not what the caller means. +#: ``shift`` has a direction and keeps it in the data; a window's direction is +#: its own name, and a width is a count, which has none. +_NEGATED = { + 'shift': ( + 'A named offset carries its sign in its values, so that one row pointing backwards says ' + 'so where the data is read — negate the column instead.' + ), + 'sum_back': ( + 'A width counts positions and so has no direction; which way a window reaches is the ' + "operator's own name rather than the sign of its width." + ), +} + #: Why a named amount that varies along the axis it walks is not the thing its #: operator claims to be. _VARIES = { @@ -260,19 +274,24 @@ def _dims_call( } -def _check_named_amount(node: FunctionCallNode, over: str, schema: Model, context: str) -> None: - """The two rules that hold of an ``offset=`` or ``within=`` naming a parameter. +def _check_named_amount(node: FunctionCallNode, over: str, inner: frozenset[str], schema: Model, context: str) -> None: + """The rules that hold of an ``offset=`` or ``within=`` naming a parameter. - Both are about the *amount*, not about a dim set, but they live here + They are about the *amount* rather than about a dim set, but they live here because here is where the schema is in hand — a parameter's ``dtype`` and - its ``dims`` are read off the same declaration, and splitting the pair - across two passes would give one rule of a documented pair two voices. + its ``dims`` are read off the same declaration, and splitting a documented + set across two passes would give one rule of it several voices. - A literal is checked by the grammar already: it parses as a number and a - number carries no dims, so only a named amount can break either rule. + A literal breaks none of them: it parses as a number, and a number has + neither a dtype to declare nor dims to vary over. """ kwarg, noun = _AMOUNTS[node.name] amount = node.kwargs[kwarg] + if isinstance(amount, UnaryOperatorNode) and isinstance(amount.operand, ParameterNode): + raise DimensionError( + f'{context}: {node.name}({kwarg}={amount.op}{amount.operand.name}) negates a named ' + f'{noun}. {_NEGATED[node.name]}' + ) if not isinstance(amount, ParameterNode): return declared = schema.parameters[amount.name] @@ -290,6 +309,16 @@ def _check_named_amount(node: FunctionCallNode, over: str, schema: Model, contex f'carries it. A named {noun} that varies along the axis it walks is {_VARIES[node.name]} ' f"— declare '{amount.name}' over dims '{over}' is not one of." ) + partition = node.kwargs.get('by') + groups = frozenset(partition.into) if isinstance(partition, LookupNode) else frozenset() + if stray := sorted(frozenset(declared.dims) - inner - groups): + raise DimensionError( + f'{context}: {node.name}({kwarg}={amount.name}) reads its {noun} at the coordinate it ' + f"walks, but '{amount.name}' varies over {stray}, which that coordinate does not carry " + f'(dims {sorted(inner)}). A dim the coordinate does not have is no coordinate at all — ' + f"declare '{amount.name}' over dims the expression carries, or group by a lookup into " + f'one of {stray}, so that each group is reached by its own {noun}.' + ) # --------------------------------------------------------------------------- diff --git a/src/math_spec/typesetting/walk.py b/src/math_spec/typesetting/walk.py index 2954b372..fd388cba 100644 --- a/src/math_spec/typesetting/walk.py +++ b/src/math_spec/typesetting/walk.py @@ -85,15 +85,19 @@ def _amount(node: ArithmeticNode) -> int | str: - """``shift``'s ``by=``: a signed number, or the name of a parameter. + """``shift``'s ``offset=``: a signed number, or the name of a parameter. A negated literal parses as a unary minus over a number rather than as a - negative one, so reading the ``NumberNode`` alone both aborted on ``by=-1`` - and left the forward direction of every translation operator unreachable. - - A named offset comes back as its name. It is always backward — the language - refuses ``by=-p``, so the direction lives in the data — and it renders as - the parameter's own symbol rather than as a number. + negative one, so reading the ``NumberNode`` alone both aborted on + ``offset=-1`` and left the forward direction of every translation operator + unreachable. + + A named offset comes back as its name. It is always backward, because a + negated one is refused at load — :func:`math_spec.dimensions.check_schema`, + so the direction lives in the data — and it renders as the parameter's own + symbol rather than as a number. The assert below is that rule's + precondition rather than a hope: before it was enforced, this raised a bare + ``AssertionError`` out of the typesetter on a model that had loaded clean. """ if isinstance(node, ParameterNode): return node.name @@ -241,8 +245,8 @@ def translation(self, step: _Step, group: str = '') -> str: the group the translation stays inside. """ backward, forward = _TRANSLATIONS[step.policy] - # a named offset is always backward: `by=-p` is refused, so the sign is - # in the data and the operator cannot read it off the call + # a named offset is always backward: `offset=-p` is refused at load, so + # the sign is in the data and the operator cannot read it off the call operator = self.op(backward if isinstance(step.by, str) or step.by > 0 else forward) if step.fill: operator = self.format.subscript(operator, [step.fill]) diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 501998ff..1b32a56f 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -31,7 +31,11 @@ 'generator': {'values': ['wind', 'gas']}, 'bus': {'values': ['n', 's']}, }, - 'lookups': {'gen_bus': {'over': 'generator', 'into': 'bus'}}, + 'lookups': { + 'gen_bus': {'over': 'generator', 'into': 'bus'}, + # over `snapshot`, so it can partition the axis the translations walk + 'snap_bus': {'over': 'snapshot', 'into': 'bus'}, + }, 'parameters': { 'p_max': {'dims': ['generator']}, 'cost': {'dims': ['generator']}, @@ -40,6 +44,8 @@ # need a parameter that obeys them and one that spans the axis walked 'spinup': {'dims': ['generator'], 'dtype': 'int'}, 'horizon': {'dims': ['snapshot'], 'dtype': 'int'}, + # over a dim `p` does not carry, so it is readable only through a `by=` + 'bus_lead': {'dims': ['bus'], 'dtype': 'int'}, }, 'variables': {'p': {'foreach': ['snapshot', 'generator'], 'bounds': {'lower': 0, 'upper': 'p_max'}}}, 'constraints': { @@ -86,6 +92,9 @@ def test_the_base_model_typechecks(): ("shift(p, over=snapshot, offset=1, edge='wrap')", {'snapshot', 'generator'}), ("shift(p, over=snapshot, offset=spinup, edge='wrap')", {'snapshot', 'generator'}), ('sum_back(p, over=snapshot, within=spinup)', {'snapshot', 'generator'}), + # the same offset a `by=` makes readable: one lag per group it maps into + ("shift(p, over=snapshot, offset=bus_lead, edge='wrap', by=snap_bus)", {'snapshot', 'generator'}), + ('sum_back(p, over=snapshot, within=bus_lead, by=snap_bus)', {'snapshot', 'generator'}), ], ) def test_dim_inference(expr, expected): @@ -157,6 +166,24 @@ def test_dim_inference(expr, expected): r'no longer "the last n"', id='a-named-width-does-not-span-the-summed-axis', ), + # Before this was refused, it loaded clean and then raised a bare + # `AssertionError` out of the typesetter (#62) — on the very rule + # `walk.py` reads a named offset as always-backward *because of*. + pytest.param( + "shift(p, over=snapshot, offset=-spinup, edge='wrap')", + r'negates a named offset', + id='a-named-offset-is-not-negated-at-the-call', + ), + pytest.param( + 'sum_back(p, over=snapshot, within=-spinup)', + r'which way a window reaches is the operator', + id='a-named-width-has-no-direction-to-negate', + ), + pytest.param( + "shift(p, over=snapshot, offset=bus_lead, edge='wrap')", + r"varies over \['bus'\], which that coordinate does not carry", + id='a-named-offset-is-read-where-the-expression-has-a-coordinate', + ), ], ) def test_an_ill_dimensioned_expression_is_rejected(expr, match):