Backend router, vtype, shift(), and piecewise: blocks - #19
Merged
Conversation
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThe relational backend gains ChangesRelational backend expansion
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MathSchema
participant select_backend
participant lower_program
participant DuckdbExecutor
participant HiGHS
MathSchema->>select_backend: provide schema
select_backend->>lower_program: check relational eligibility
lower_program-->>select_backend: eligibility or fallback reason
select_backend->>DuckdbExecutor: lower eligible schema
DuckdbExecutor->>HiGHS: solve typed columns and constraints
HiGHS-->>DuckdbExecutor: objective and variable values
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@linopy_yaml/helpers.py`:
- Around line 131-132: Update the shift-amount handling around dim and n to
validate that n is integral before converting it with int(). Reject fractional
values such as 1.5 consistently with relational lowering, while preserving valid
integer shift behavior and preventing eager fallback from truncating
expressions.
In `@linopy_yaml/relational/executor.py`:
- Around line 538-546: Update the dimension-table construction used by the
relational executor so each dim_* table assigns ordinals in declared/coords
order rather than ORDER BY val, preserving xarray positional shift semantics for
non-monotonic and string coordinates. Keep the shift join logic in the affected
executor path unchanged, and add a differential test comparing eager and
relational results for unsorted coordinates.
In `@SPEC.md`:
- Line 1041: Synchronize the relational capability documentation in SPEC.md: add
shift to the supported affine subset near lines 1041-1041, update the
vtype/integrality description near lines 1108-1115 to reflect current support
beyond continuous-only columns, and remove binary/integer variables from the
eager-fallback list near lines 1138-1141.
In `@tests/test_piecewise_convex.py`:
- Line 78: Update the piecewise cost oracle’s max computation to use strict
zipping for slopes and intercepts, ensuring mismatched segment lengths raise an
error instead of being silently truncated. Preserve the existing cost
calculation and max behavior.
In `@tests/test_roll.py`:
- Around line 127-131: Update the YAML conversion setup around STORAGE_YAML to
verify that the original roll(soc, snapshot=1) token exists before replacing it,
and assert the resulting yaml_text differs or contains the expected shift(soc,
snapshot=1) expression so the test cannot silently continue using roll().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 11545865-d2e4-47b4-a710-23f36d0c75ca
📒 Files selected for processing (12)
SPEC.mdexamples/piecewise_convex.yamllinopy_yaml/helpers.pylinopy_yaml/lowering.pylinopy_yaml/relational/executor.pylinopy_yaml/relational/ir.pylinopy_yaml/router.pytests/test_lowering.pytests/test_milp.pytests/test_piecewise_convex.pytests/test_roll.pytests/test_router.py
Owner
Author
|
Added the |
FBumann
changed the title
…st lane) Record the scoping decision prompted by linopy 0.9's trajectory (piecewise/SOS/indicator/dualize/updates are model transformations the eager builder inherits for free and a flat matrix streamer cannot): - §12.1: the relational backend is a streaming compiler for large pure-affine models — an optimization lane with automatic fallback, not a general replacement for the eager builder. - §12.4: the IR is affine-by-design, decided. Formulations are eager-only; if ever streamed, they enter as an expansion stage that emits declarations, never as expression nodes. Reimplementing linopy's reformulation passes is explicitly rejected. Semi-continuous (a vtype, not a formulation) is the planned extension. - §12.6: the solver_direct sink cap is stated (three streams today; five-stream upgrade path documented). - §12.8 (new): backend eligibility and automatic fallback. linopy_yaml/router.py implements §12.8: relational_eligibility(schema) decides by attempting the lowering — so eligibility can never drift from what the backend actually supports — and select_backend() returns an explicit choice with the verbatim rejection reason on fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The route back to piecewise, per discussion — at the right layers: - vtype: binary/integer variables on the relational backend. cols gains a vtype column; the LP sink writes binary/general sections; the solver_direct sink sets HiGHS integrality per column batch. Lowering maps binary to vtype='binary' with fixed 0/1 bounds (linopy parity). Basic MILP is now relational-eligible — verified by a unit-commitment differential (objective, 0/1 integrality, both sinks). - shift(expr, dim=n): non-cyclic counterpart of roll — vacated positions contribute zero. Eager: linopy/xarray .shift(); relational: ir.Shift(wrap=False), the same ord-join without the modulo. This is the ordering primitive the nonconvex piecewise expansion needs, and useful directly for acyclic storage recurrences. The differential test also caught that a start-empty battery makes the original test data infeasible — on which both backends agreed exactly. - Convex piecewise needs no machinery: examples/piecewise_convex.yaml shows the epigraph formulation in ordinary affine YAML — pure LP, relational-eligible today, verified pointwise against a numpy evaluation of the piecewise cost. SPEC: §7.3 shift docs, §12.4 updated (vtype landed; nonconvex piecewise planned as schema-level expansion so both backends receive identical affine declarations; formulations never enter as IR expression nodes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ends Add the piecewise: schema section, mirroring linopy.Model.add_piecewise_formulation: N symmetric links [expression, values-parameter, sign?] jointly pinned to a breakpoint-indexed curve (2 links = y=f(x); 3+ = CHP-style joint operating curves). Links accept any affine expression string — a bare variable name is just the simplest case — and one link may carry <=/>= to be bounded by the curve instead of pinned (linopy's restriction mirrored: only with exactly two links). Breakpoints are parameters, not literals, so curves can vary along other dims. Expansion (linopy_yaml/piecewise.py) runs before either backend and emits only existing language constructs — λ weights with a convexity row, one link row per tuple, and segment binaries with adjacency lam <= seg + shift(seg, bp=1) — so eager and relational receive identical affine declarations and stay differential-testable. convex: true drops the binaries (pure-LP convex hull), keeping the model relational-eligible as an LP. The λ method is expansion-pure: no derived data (slopes/intercepts/segment lengths) needed. Wired into both entry points: Model.from_yaml expands after schema validation; lower_program expands before lowering, so the router's eligibility answer accounts for expansion automatically. Tests: nonconvex objective lands exactly ON the curve vs a numpy interpolation (adjacency binaries load-bearing) while convex: true provably drops to the chord; CHP 3-link joint curve matches interpolation on all links; inline-expression links; reference and sign validation. SPEC §3.6 + §12.4 updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
active gating (linopy's active= parameter): the λ method makes this a one-line formulation change — sum(lam, over=bp) == active (and same for the segment pick row), which reproduces linopy's semantics exactly: when the gate is 0 all λ vanish and every ==-link pins its expression to zero. active accepts an expression (bare binary variable being the common case); a bare non-binary variable is rejected at expansion. Not supported with convex: true (mirroring method='lp'). Curvature guard for convex: true, closing the silent-wrong-answer gap: - schema-time: exactly two links (the hull relaxation is only well-defined for a single y=f(x) curve — mirrors linopy's lp method) - data-time (validate_piecewise_data, wired into Model.from_yaml and tidy_sources): x-breakpoints strictly increasing, and mixed-curvature y-curves rejected with a pointer to the exact MILP form. Consistent curvature passes — the hull semantics remain documented behavior. Checked per curve slice when breakpoints vary along other dims; parquet-path sources bypass the guard (data never enters Python). Tests: gated UC-style model differential on both backends (cost ON the curve when committed, pinned to zero when off), binary-check, mixed- curvature and monotonicity rejections, convex two-link restriction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ample examples/piecewise.yaml showcases the block with per-generator curves (breakpoint parameters carrying [generator, bp] — inexpressible with flat breakpoint lists), convex: true, relational-eligible LP; new differential test checks each generator's cost sits on its own curve. The epigraph pattern stays as an inlined tested pattern in test_piecewise_convex.py (automating it is issue #23's method: lp). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FBumann
force-pushed
the
relational/vtype-shift-router
branch
from
July 24, 2026 15:01
b6c12eb to
c127f00
Compare
Per the update-in-same-PR rule: the schema-level piecewise expansion and the backend router are structural additions from this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, doc sync - relational executor: explicit dimension indexes now assign ordinals in declared/coords order (pandas first-occurrence; parquet via file_row_number) instead of ORDER BY val, so Shift's positional semantics match xarray/linopy for non-monotonic and string coordinates. Derived dims keep the sorted deterministic fallback, documented. New differential test with lexicographically-scrambling string labels (t0..t47) fails on the old code and passes now. - helpers: shift()/roll() reject fractional amounts (TypeError) instead of silently truncating via int(), consistent with relational lowering. - SPEC: v0 subset gains shift; sink-cap paragraph reflects vtype integrality; binary/integer removed from the §12.8 fallback list; the stale "piecewise ... eager-only" sentence in §12.4 now points at the schema-level expansion. - tests: strict zip in the piecewise cost oracle; the roll→shift YAML replacement in the acyclic test is now asserted so it cannot silently keep testing roll(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FBumann
added a commit
that referenced
this pull request
Jul 24, 2026
* docs: architecture pipeline as mermaid; linopy-free rule; #19 content folded in Ports the doc improvements stranded on the feat/expression-macros branch after #20's squash-merge: the pipeline as one validated mermaid flowchart (renders on GitHub, stays diffable text), the promoted hard rule that the relational lane is linopy-free (duckdb -> highspy, linopy's semantics as a spec to match, not code to share), and the completed module map. Folds in #19's additions: piecewise expansion in the front end, piecewise.py and router.py rows, and roll/shift as the dim-as-key counterexample in the macro-friendliness rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: separate the two lanes visually; add language-tiers diagram Pipeline redesign after actually rendering it: each lane gets its own tinted subgraph (green relational, blue eager) with its own data entry, which removes every crossing edge — the router's two labeled edges are now the only lines entering the lanes. New small mermaid for the two-tier language economy (free composition -> taxed primitives -> both backends; @register as a dashed escape hatch that reaches the eager backend only). Both diagrams validated with mermaid-cli. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: README catches up with the two-backend era The README predated the relational backend entirely. Adds: the two-backend framing in the intro, a 'models that don't fit in memory' use case with the automatic-routing story and the 107M-var/0.6GB headline, the three new schema sections (expressions, macros, piecewise), updated helpers (group_sum, shift) and features list, the [relational] install extra, honest scoping of the pure-consumer goal to the eager path, fixed introspection API names, and links to ARCHITECTURE.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: shrink README; frame linopy as compat layer + validation oracle README drops from ~220 to ~140 lines keeping headlines and use cases: callback comparison condensed to one paragraph plus example, open- questions section folded into Status, features/schema merged into one compact language section. Framing updated per the scope decision: models build on the relational/streaming engine; linopy is kept for exactly two roles — the compatibility layer (Python-built models, .yaml.extend, out-of-subset fallback with stated reason) and the validation oracle. ARCHITECTURE.md lane title and hard rule 3 aligned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: miniature two-lane diagram in the README A 7-node, module-name-free version of the pipeline — different altitude than ARCHITECTURE.md's full diagram, so it only goes stale on true topology changes. Validated with mermaid-cli. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 24, 2026
FBumann
added a commit
that referenced
this pull request
Jul 31, 2026
The benchmarks page was 110 table rows in twelve tables, all weighted the
same, with no shape and no headline — and the one chart page we had was a
separate HTML file linked from a mid-paragraph, absent from the nav. So
the numbers were published and the *finding* was not.
Four figures now lead the Results section, written by `bench.plot` from
the same jsonl the tables come from:
wall time to a loaded solver, faceted by sink, log-log
peak the same builds' resident memory, its own chart, never a
second axis on the first
cases all six models as small multiples
phases where the time goes at `l` — the chart the tables cannot be
read for at all, since they publish one total per row
`bench/svg.py` draws them: no plotting library, because these pages are
read on GitHub as often as on the site and a chart has to be a committed
file rather than something a renderer produces at view time. Two files
per figure, light and dark, because mkdocs-material's toggle stamps the
host page and an `<img>`-referenced SVG cannot see it; the `#only-light`
/ `#only-dark` suffixes choose, and GitHub takes the first of the pair.
The palette is validated rather than chosen — slots 1 and 2 of a
reference categorical palette, run through the six checks against both
surfaces (lightness band, chroma floor, CVD separation, normal-vision
floor, contrast). Every series is direct-labelled as well as coloured, so
identity survives a reader who sees neither hue.
The twelve tables stay, one `<details>` per case, and their headings
became bold text: a heading inside a fold still lands in the table of
contents, which is twelve rail entries for the appendix.
`docs/bench.svg` deleted — drawn for #19, hardcoded light-mode colours,
referenced from nowhere since. `tests/test_docs_site.py` now checks both
directions, so the next one fails instead of rotting.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FBumann
added a commit
that referenced
this pull request
Aug 1, 2026
The benchmarks page was 110 table rows in twelve tables, all weighted the
same, with no shape and no headline — and the one chart page we had was a
separate HTML file linked from a mid-paragraph, absent from the nav. So
the numbers were published and the *finding* was not.
Four figures now lead the Results section, written by `bench.plot` from
the same jsonl the tables come from:
wall time to a loaded solver, faceted by sink, log-log
peak the same builds' resident memory, its own chart, never a
second axis on the first
cases all six models as small multiples
phases where the time goes at `l` — the chart the tables cannot be
read for at all, since they publish one total per row
`bench/svg.py` draws them: no plotting library, because these pages are
read on GitHub as often as on the site and a chart has to be a committed
file rather than something a renderer produces at view time. Two files
per figure, light and dark, because mkdocs-material's toggle stamps the
host page and an `<img>`-referenced SVG cannot see it; the `#only-light`
/ `#only-dark` suffixes choose, and GitHub takes the first of the pair.
The palette is validated rather than chosen — slots 1 and 2 of a
reference categorical palette, run through the six checks against both
surfaces (lightness band, chroma floor, CVD separation, normal-vision
floor, contrast). Every series is direct-labelled as well as coloured, so
identity survives a reader who sees neither hue.
The twelve tables stay, one `<details>` per case, and their headings
became bold text: a heading inside a fold still lands in the table of
contents, which is twelve rail entries for the appendix.
`docs/bench.svg` deleted — drawn for #19, hardcoded light-mode colours,
referenced from nowhere since. `tests/test_docs_site.py` now checks both
directions, so the next one fails instead of rotting.
Co-Authored-By: Claude Opus 5 (1M context) <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
1 participant
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.
Follow-up to #18: the scope decision and the full route back to piecewise. Five commits, in order:
linopy_yaml.router: a schema is relational-eligible iff it lowers;select_backend()falls back to the eager builder with the verbatim rejection reason. Eligibility is decided by attempting the lowering, so it can never drift. SPEC §12 records the scope decision: relational = streaming compiler for large pure-affine/basic-MILP models; eager = feature-complete default; the IR stays affine-by-design.cols.vtype, LPbinary/generalsections, HiGHS integrality per batch). Basic MILP is relational-eligible; unit-commitment differential verifies objective to 1e-9 with true 0/1 integrality on both sinks.shift(expr, dim=n)— non-cyclicroll(vacated positions contribute zero) on both backends; eager = linopy.shift(), relational = the roll ord-join without the modulo. Used by acyclic storage recurrences and the piecewise adjacency row.piecewise:blocks — mirroringlinopy.Model.add_piecewise_formulation: N symmetric tuple links[expression, values-parameter, sign?]jointly pinned to a breakpoint-indexed curve (2 links = y=f(x); 3+ = CHP joint operating curves). Links accept inline affine expressions; breakpoints are parameters, so curves may vary along other dims. Expanded at the schema level (λ convex-combination + adjacency binaries) before either backend runs, so eager and relational receive identical affine declarations and stay differential-testable.convex: truedrops the binaries (pure-LP hull). Tests prove the nonconvex objective lands exactly ON the curve vs numpy interpolation whileconvex: trueprovably yields the chord.active:(linopy parity: formulation pinned to 0 when the gate is 0; in λ form it is justsum(lam) == active), and a two-level guard forconvex: true: exactly two links at schema time, strictly-monotone breakpoints and no mixed curvature at data time — closing the silent-hull-relaxation wrong-answer gap.Remaining piecewise
methodoptions tracked as issues: #22 (incremental) and #23 (sos2 / lp).110 tests green; mypy/ruff clean on all touched code.
🤖 Generated with Claude Code