Repository navigation
build(deps): a rule the language now enforces has one home again - #1258
Merged
Merged
Conversation
math-spec v0.0.0-alpha.11 enforces the two rules a named `offset=` or `within=` was always documented to obey — integral, and constant along the axis its operator walks — plus the stray-dim rule and the refusal of a negation at the call site. All four had a second implementation here, reached later and worded differently; the language speaks first through `dims_of`, so those arms were unreachable the moment the pin moved. `_named_width`, three quarters of `_named_offset` and `_within_reach` go with them. What stays is the one rule the language does not hold: a named offset must say what the vacated positions contribute, which is about a presence frame this repository owns. The same release gives `sum_back` a `by=`, and neither lane builds a window that stops at each group's edge. It is refused rather than ignored: unread, the window spans the seam between two groups and sums rows that are not neighbours — a model that builds, solves and reports optimal. One refusal serves both lanes, `lpspec.linopy.build` lowering before it evaluates. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merging this PR will not alter performance
Comparing Footnotes
|
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.
math-specmoves fromv0.0.0-alpha.10tov0.0.0-alpha.11. The release doestwo things to this repository: it takes over four rules that had a second
implementation here, and it declares a keyword neither lane builds.
The four rules have one home again
alpha.11 enforces, for both
shift(offset=…)andsum_back(within=…), whatthe reference always said a named amount obeys — integral, constant along the
axis its operator walks, read at a coordinate the expression carries, and never
negated at the call site. Every one of them was also implemented here, in
_named_widthand_named_offset, and the language now speaks first: loweringcalls
dims_ofbefore it reads the amount, so those arms became unreachablethe moment the pin moved. They are deleted, with
_within_reachand the twomessages that served only them.
What stays is the one rule the language does not hold: a named offset must
say what the vacated positions contribute (#850). That is about a presence
frame keyed by the translated dimension alone, which is this repository's, not
the language's.
Coverage moved rather than went:
test_a_named_offset_that_cannot_mean_a_lag_is_refused[not-an-integer]a-named-offset-is-integral…[along-the-shifted-dim]a-named-offset-does-not-span-the-axis-it-walks…[not-a-parameter]test_a_named_width_that_cannot_mean_a_window_is_refused[not-an-integer]a-named-width-is-integral…[along-the-summed-dim]a-named-width-does-not-span-the-summed-axistest_a_named_offset_carries_its_sign_in_the_dataa-named-offset-is-not-negated-at-the-calltest_an_offset_over_a_dim_nothing_puts_in_reach_is_refused[nothing-puts-it-in-reach]a-named-offset-is-read-where-the-expression-has-a-coordinate…[not-what-the-partition-groups-into]sum_back(by=…)is refusedalpha.11 gives the window a partition, so it stops at each group's edge
(energy-models/mathspec#65). Neither lane builds it, and ignoring the keyword
is a wrong answer rather than a missing one — measured on this tree at the old
pin, four hours split
a a b b,within=3:What the unread keyword built
The eager lane answered the same file with
TypeError: _operator_sum_back() got an unexpected keyword argument 'by'.Now both refuse, in one sentence from one place —
lpspec.linopy.buildrunsthe lowering pass before it evaluates anything, so the eager operators never
see the call. That is also why the refusal is not duplicated into
linopy/operators.py: a guard there is unreachable through either entry point,which I checked by deleting the lowering guard and reading the traceback
(
linopy/__init__.py:110).Verified
suite:
3103 passed, 249 skipped, 1 xfailed. The 35 failures arexpress-parametrised and pre-existing on this machine (InterfaceError, thelicence) — red at the old pin too, checked case by case.
ruff check·ruff format --check .·pyrefly0 errors (through theworktree's own interpreter — the project-mode run resolves the primary
checkout's
.venvand reports 18 phantommissing-imports).mutation table:
lowering.py:364-365)lowering.py:410-411)Deliberately not done
offsetover a dim that is not whatby=groups into) has no twin in math-spec's own tests, though math-spec nowowns the rule. Deleted here rather than kept as a second home; worth filing
upstream.
test_both_lanes_read_every_keyword_the_language_declareskeeps itsblanket interception of
by=on the eager side, which is why it stays greenhere. It is a real hole — it would credit
sum_backa keyword onlyatandsumare dispatched by name — but narrowing it demands the eager lane readby=itself, which is only true once the window is built. The narrowingbelongs to the follow-up that implements the partition, and lands with it.
this one.