diff --git a/src/lpspec/linopy/coverage.py b/src/lpspec/linopy/coverage.py index 8e5c615e6..4b077db3d 100644 --- a/src/lpspec/linopy/coverage.py +++ b/src/lpspec/linopy/coverage.py @@ -44,20 +44,56 @@ def check_constant_side_covers( ) -> None: """A comparison's constant side must have values wherever the row is built. - A missing row is read as 0, and on a side with no variable that zero *is* - the bound — `x <= cap` becomes `x <= 0`, which binds rather than vanishing. - Keyed to the rows the declaration builds, not to the coordinate product: a - `where` that removed the coordinate has already answered the question. + A missing row is read as 0, and where that zero is a constant piece it *is* + the bound — `x <= cap` becomes `x <= 0`, and `x + hi <= 100` becomes + `x <= 100`, both binding rather than vanishing. The question is asked of + every constant piece, a parameter in a variable-free additive position, + whichever side it was written on and beside whatever term: a coefficient, + which a variable stands with in a product, is a zero when it is missing (the + absence rules) and is not one. Keyed to the rows the declaration builds, not + to the coordinate product: a `where` that removed the coordinate has already + answered the question. """ - for side in (row.lhs, row.rhs): - if program.carries_variable(side): - continue - walk = _under_regions(side, ctx, mask) - found = [(node.name, where) for node, where in walk if isinstance(node, program.Parameter)] - for param, narrowed in sorted(found, key=itemgetter(0)): - missing = gaps_under(ctx.dataset[param], narrowed) - if missing: - raise DataError(uncovered_constant_message(param, missing, name)) + found = [pair for side in (row.lhs, row.rhs) for pair in _constant_leaves(side, ctx, mask, coefficient=False)] + for param, narrowed in sorted(found, key=itemgetter(0)): + missing = gaps_under(ctx.dataset[param], narrowed) + if missing: + raise DataError(uncovered_constant_message(param, missing, name)) + + +def _constant_leaves( + node: program.ExpressionNode, ctx: EvaluationContext, mask: Any, coefficient: bool +) -> Iterator[tuple[str, Any]]: + """Every parameter standing as a constant piece under *node*, with the rows it must cover. + + A constant piece is a parameter in a variable-free additive position — `hi` + in `x + hi`, or in `sum(x) + sum(hi)`. A parameter a variable stands with in + a product is a coefficient instead, and a sparse coefficient is a zero the + absence rules allow, so ``coefficient`` records having passed through such a + product on the way down and suppresses the parameters below it. Additive + structure, a reduction and a division by it leave the flag as it was; a + ``cases:`` region narrows the mask as it does for the whole walk. + """ + if isinstance(node, program.Variable): + return + if isinstance(node, program.Parameter): + if not coefficient: + yield node.name, mask + return + if isinstance(node, program.Cases): + for region in node.regions: + inside = evaluate_where(region.when, ctx) + yield from _constant_leaves(region.value, ctx, inside if mask is None else mask & inside, coefficient) + return + if isinstance(node, program.Multiply): + yield from _constant_leaves(node.left, ctx, mask, coefficient or program.carries_variable(node.right)) + yield from _constant_leaves(node.right, ctx, mask, coefficient or program.carries_variable(node.left)) + return + if isinstance(node, program.Divide): + yield from _constant_leaves(node.numerator, ctx, mask, coefficient) + return + for child in program.children(node): + yield from _constant_leaves(child, ctx, mask, coefficient) def _under_regions( diff --git a/src/lpspec/relational/engines/polars/assembly.py b/src/lpspec/relational/engines/polars/assembly.py index 2bba28855..34e71df3a 100644 --- a/src/lpspec/relational/engines/polars/assembly.py +++ b/src/lpspec/relational/engines/polars/assembly.py @@ -544,11 +544,7 @@ def _refuse_short_constant_parameters( short anywhere. """ found = [ - pair - for side in (c.lhs, c.rhs) - if not program.carries_variable(side) - for pair in _constant_parameters(side) - if pair[0] in self.measured.sparse + pair for side in (c.lhs, c.rhs) for pair in _constant_parameters(side) if pair[0] in self.measured.sparse ] for param, region in sorted(found, key=itemgetter(0)): missing = self._uncovered_coordinates(frame, param, c, region) @@ -728,9 +724,16 @@ def short_parameters(program: program.Program, attached: AttachedSources) -> dic def _constant_parameters( - node: program.ExpressionNode, region: program.Mask | None = None + node: program.ExpressionNode, region: program.Mask | None = None, coefficient: bool = False ) -> Iterator[tuple[str, program.Mask | None]]: - """Every parameter under *node*, each with the region it stands in. + """Every parameter standing as a constant piece under *node*, each with the region it stands in. + + A constant piece is a parameter in a variable-free additive position — `hi` + in `x + hi`, or in `sum(x) + sum(hi)`. A parameter a variable stands with in + a product is a coefficient instead, and a sparse coefficient is a zero the + absence rules allow, so ``coefficient`` records having passed through such a + product on the way down and suppresses the parameters below it. Additive + structure, a reduction and a division by it leave the flag as it was. A region narrows what the pieces under it owe, and regions compose by conjunction as the walk descends, which is what @@ -741,16 +744,28 @@ def _constant_parameters( node: The expression to walk. region: The region *node* already stands in — the recursion's own accumulator, ``None`` at the call a caller writes. + coefficient: Whether *node* stands in a product with a variable — the + recursion's own accumulator, ``False`` at the call a caller writes. """ + if isinstance(node, program.Variable): + return if isinstance(node, program.Parameter): - yield node.name, region + if not coefficient: + yield node.name, region return if isinstance(node, program.Cases): for r in node.regions: - yield from _constant_parameters(r.value, both_regions(region, r.when)) + yield from _constant_parameters(r.value, both_regions(region, r.when), coefficient) + return + if isinstance(node, program.Multiply): + yield from _constant_parameters(node.left, region, coefficient or program.carries_variable(node.right)) + yield from _constant_parameters(node.right, region, coefficient or program.carries_variable(node.left)) + return + if isinstance(node, program.Divide): + yield from _constant_parameters(node.numerator, region, coefficient) return for child in program.children(node): - yield from _constant_parameters(child, region) + yield from _constant_parameters(child, region, coefficient) def declares_quadratic(c: program.ConstraintDeclaration) -> bool: diff --git a/tests/test_absence.py b/tests/test_absence.py index f510b0586..6ffd97c2a 100644 --- a/tests/test_absence.py +++ b/tests/test_absence.py @@ -140,6 +140,50 @@ def test_a_where_is_the_escape_from_the_constant_side_check(): ) +def test_a_constant_piece_beside_a_term_is_refused_on_both_lanes(): + """A parameter added beside a variable term is a constant piece, not skipped (#1521). + + `w * x + c <= 100` reads `w * x <= 100` where the file says `w * x <= 100 - c`, + the missing `c` filled with the zero that is a bound. The eager check asked + its question of a side carrying no variable, so a piece beside a term went + unasked and the lane built the wrong row in silence; the relational lane, + asking of every constant fragment, refused alone. + """ + spec = override(SPARSE_COEFFICIENT_SPEC, **{'constraints.cap.expression': 'w * x + c <= 100'}) + both_lanes_refuse(spec, SPARSE_CONSTANT_DATA, match="parameter 'c' covers 1 fewer") + + +def test_a_constant_piece_beside_a_term_is_refused_through_a_reduction(): + """The same piece under a sum, where both lanes were blind (#1521). + + `sum(w * x, over=t) + sum(c, over=t) <= 100` sums the gap away before either + lane's per-coordinate check can see it — the eager lane skipped the side for + its variable, the relational lane could not see the hole through the sum + (#1465's mechanism) — so the parameter is asked directly, of both lanes now. + """ + spec = override( + SPARSE_COEFFICIENT_SPEC, + **{'constraints.cap': {'foreach': [], 'expression': 'sum(w * x, over=t) + sum(c, over=t) <= 100'}}, + ) + both_lanes_refuse(spec, SPARSE_CONSTANT_DATA, match="parameter 'c' covers 1 fewer") + + +def test_a_sparse_coefficient_beside_a_constant_piece_is_still_a_zero(): + """The widened check still reads a sparse coefficient as a zero (#1521). + + `w * x + c <= 100` with `w` short and `c` whole builds and both lanes agree: + `w` is a coefficient wherever a variable stands with it, so its missing row + is the zero the absence rules allow rather than an uncovered bound — only the + piece `c`, which no variable stands with, is owed its coordinates. + """ + data = {'t': [0, 1, 2], 'w': pd.Series({1: 1.0, 2: 1.0}), 'c': pd.Series({0: 5.0, 1: 4.0, 2: 5.0})} + spec = override(SPARSE_COEFFICIENT_SPEC, **{'constraints.cap.expression': 'w * x + c <= 100'}) + with differential(spec, data, lp=True) as run: + assert run.result.objective == pytest.approx(10.0 + 10.0 + 10.0, rel=RTOL), ( + 't=0: w absent, so its term is zero and x runs to its bound; elsewhere the bound is slack' + ) + + #: `south` is a load-only bus: both generators sit on `north`, so the group #: behind `south`'s constant side has no members at all. GROUPED_CONSTANT_SPEC = {