Repository navigation
test(compat): a divisor written as a power is named and refused on both lanes - #1615
Merged
Merged
Conversation
…th lanes `p / (d ** 2) <= 1` with `d` missing a row used to split the lanes: the relational lane named the divisor `''` and the eager lane found no divisor to check, filled the gap with a zero coefficient and built `inf` into the matrix with no refusal. The cause was a walk in math-spec that had no branch for a power, so `d ** 2` hid `d`. That was fixed upstream (energy-models/mathspec#403, in the pinned alpha.85). What this tree owed is the guard that both lanes now name `d` and refuse in one sentence — the gap was never in the language's coverage rules but in what this package asks the language. Closes #1536. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JyxyehhHvXsBqf6NxeQgU1
Merging this PR will not alter performance
Comparing 
Footnotes
|
FBumann
added a commit
that referenced
this pull request
Sep 14, 2026
…ata is short (#1621) > **Prompt:** What about the last bug in lpspec? … Full fix, both shapes. > [!NOTE] > The following content was generated by AI. A constant piece added beside a variable term is now asked for its data on both lanes. `x + hi <= 100` with `hi` short of a coordinate read `x <= 100` on the eager lane — the missing `hi` filled with the zero that is a bound — while the relational lane refused; the two lanes disagreed, and the eager one built the wrong constraint in silence. Both lanes' constant-side check skipped any side that carried a variable, so a piece beside a term went unasked. They now ask of every **constant piece** — a parameter in a variable-free additive position — found by threading a `coefficient` flag through the walk: a parameter a variable stands with in a product is a coefficient, whose missing row is the zero the absence rules allow, and is left alone. Closes #1521. <details><summary>The shapes, and both lanes' verdict</summary> | constraint, `hi`/`c` short one coordinate | before | after | | --- | --- | --- | | `x + hi <= 100` | eager builds `x <= 100`, relational refuses | both refuse, one sentence | | `sum(x) + sum(hi) <= 100` | **both** build (the gap is summed away) | both refuse, one sentence | | `(x + hi) * 2 <= 100` | eager builds, relational refuses | both refuse | | `w * x <= 100`, `w` short | both build (`w=0` is a legal coefficient) | unchanged — still both build | The aggregated shape (`sum(x) + sum(hi)`) is the one #1465 left, missed on both lanes; the eager check asked of a variable-free side and the relational one could not see the hole through the sum. Both now ask the parameter directly. </details> <details><summary>What was verified</summary> - Three tests in `tests/test_absence.py`: both lanes refuse `w * x + c <= 100` and its summed form with one identical sentence (`both_lanes_refuse`), and a sparse coefficient beside a covered piece still builds and agrees (`differential(..., lp=True)`). The two refusal tests fail on the tree before the fix (the eager lane built in silence); confirmed. - Full suite: **3684 passed, 329 skipped**; the 10 failures are all `gurobi` / `xpress` cases, which need commercial solvers absent from this environment — 94 pass with HiGHS across the two affected files. `ruff check` / `ruff format --check` clean; the changed functions add no `pyrefly` error. - **Not run:** `pixi run check` — pixi is unavailable here, so gates ran through a local venv. </details> <details><summary>Mutation table</summary> Each guard deleted in turn, the affected tests run (`tools.mutate` needs a committed tree it did not have mid-work, so taken by hand): | guard removed | caught by | | --- | --- | | the `if not coefficient:` yield-suppression (both walks) | `test_a_sparse_coefficient_is_still_a_zero_coefficient`, `…_beside_a_constant_piece_is_still_a_zero`, `…_on_the_bound_side_still_pins_the_variable` fail | | the `coefficient or carries_variable(…)` propagation through a product | the same three coefficient tests fail | | the widening (restore the whole-side `carries_variable` skip) | `test_a_constant_piece_beside_a_term_is_refused_on_both_lanes`, `…_through_a_reduction` fail | </details> <details><summary>On the stack</summary> Meant as a PR stacked on #1536's branch, but [#1615](#1615) merged to `main` first, so this targets `main` directly — the base already carries #1536, and the diff is this fix alone. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JyxyehhHvXsBqf6NxeQgU1 --- _Generated by [Claude Code](https://claude.ai/code/session_01JyxyehhHvXsBqf6NxeQgU1)_ Co-authored-by: Claude <noreply@anthropic.com>
FBumann
added a commit
that referenced
this pull request
Sep 15, 2026
…ata is short (#1621) > **Prompt:** What about the last bug in lpspec? … Full fix, both shapes. > [!NOTE] > The following content was generated by AI. A constant piece added beside a variable term is now asked for its data on both lanes. `x + hi <= 100` with `hi` short of a coordinate read `x <= 100` on the eager lane — the missing `hi` filled with the zero that is a bound — while the relational lane refused; the two lanes disagreed, and the eager one built the wrong constraint in silence. Both lanes' constant-side check skipped any side that carried a variable, so a piece beside a term went unasked. They now ask of every **constant piece** — a parameter in a variable-free additive position — found by threading a `coefficient` flag through the walk: a parameter a variable stands with in a product is a coefficient, whose missing row is the zero the absence rules allow, and is left alone. Closes #1521. <details><summary>The shapes, and both lanes' verdict</summary> | constraint, `hi`/`c` short one coordinate | before | after | | --- | --- | --- | | `x + hi <= 100` | eager builds `x <= 100`, relational refuses | both refuse, one sentence | | `sum(x) + sum(hi) <= 100` | **both** build (the gap is summed away) | both refuse, one sentence | | `(x + hi) * 2 <= 100` | eager builds, relational refuses | both refuse | | `w * x <= 100`, `w` short | both build (`w=0` is a legal coefficient) | unchanged — still both build | The aggregated shape (`sum(x) + sum(hi)`) is the one #1465 left, missed on both lanes; the eager check asked of a variable-free side and the relational one could not see the hole through the sum. Both now ask the parameter directly. </details> <details><summary>What was verified</summary> - Three tests in `tests/test_absence.py`: both lanes refuse `w * x + c <= 100` and its summed form with one identical sentence (`both_lanes_refuse`), and a sparse coefficient beside a covered piece still builds and agrees (`differential(..., lp=True)`). The two refusal tests fail on the tree before the fix (the eager lane built in silence); confirmed. - Full suite: **3684 passed, 329 skipped**; the 10 failures are all `gurobi` / `xpress` cases, which need commercial solvers absent from this environment — 94 pass with HiGHS across the two affected files. `ruff check` / `ruff format --check` clean; the changed functions add no `pyrefly` error. - **Not run:** `pixi run check` — pixi is unavailable here, so gates ran through a local venv. </details> <details><summary>Mutation table</summary> Each guard deleted in turn, the affected tests run (`tools.mutate` needs a committed tree it did not have mid-work, so taken by hand): | guard removed | caught by | | --- | --- | | the `if not coefficient:` yield-suppression (both walks) | `test_a_sparse_coefficient_is_still_a_zero_coefficient`, `…_beside_a_constant_piece_is_still_a_zero`, `…_on_the_bound_side_still_pins_the_variable` fail | | the `coefficient or carries_variable(…)` propagation through a product | the same three coefficient tests fail | | the widening (restore the whole-side `carries_variable` skip) | `test_a_constant_piece_beside_a_term_is_refused_on_both_lanes`, `…_through_a_reduction` fail | </details> <details><summary>On the stack</summary> Meant as a PR stacked on #1536's branch, but [#1615](#1615) merged to `main` first, so this targets `main` directly — the base already carries #1536, and the diff is this fix alone. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JyxyehhHvXsBqf6NxeQgU1 --- _Generated by [Claude Code](https://claude.ai/code/session_01JyxyehhHvXsBqf6NxeQgU1)_ Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
The following content was generated by AI.
A test, on both lanes, pinning that a divisor written as a power is named and
refused.
p / (d ** 2) <= 1withdmissing a row used to split the lanes, andneither answer was usable.
The cause was upstream:
program.childrenin math-spec had no branch for apower, so
d ** 2hiddfrom every walk this package asks for a divisor'snames. The relational lane named the divisor
''; the eager lane found nodivisor to check, filled the gap with a zero coefficient and built
infintothe matrix with no refusal at all.
That was fixed upstream in energy-models/math-spec#403
(PR #404, in the pinned
alpha.85). What this tree owed is the guard — the gapwas never in the language's coverage rules, but in what this package asks the
language. Both lanes now name
dand refuse in one identical sentence.Closes #1536. The issue's
blocked:upstreamlabel is now stale — the blockermerged.
What was verified
New test
test_a_sparse_divisor_written_as_a_power_is_named_and_refused_on_both_lanesin
tests/test_arithmetic_laws.py, beside the existing divisor tests, usingboth_lanes_refuse(which builds the two lanes apart, so an eager lane thatsilently built
infwould fail the raise rather than pass unnoticed).Confirmed against a simulated pre-perf(duckdb): the factored label path, and one answer for which route to take #403
children(thePowerbranch dropped):the test fails, so it exercises the gap rather than passing on a broken tree.
Against the pinned math-spec it passes; both lanes return:
tests/test_arithmetic_laws.py: 26 passed (both lanes, linopy from gitmaster).
ruff checkandruff format --checkclean.Not run:
pixi run check— pixi is unavailable here, so the tests andlinters were run through a local venv. This is a test-only change, so
src/and the schema are untouched.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JyxyehhHvXsBqf6NxeQgU1
Generated by Claude Code