Skip to content

perf: compute variable labels arithmetically when the mask factors - #178

Merged
FBumann merged 1 commit into
mainfrom
claude/investigate-build-throughput
Jul 28, 2026
Merged

perf: compute variable labels arithmetically when the mask factors#178
FBumann merged 1 commit into
mainfrom
claude/investigate-build-throughput

Conversation

@FBumann

@FBumann FBumann commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Reduced to one commit. When I split the harness out (#182) I found that most of what this PR carried had already landed as focused PRs — #174 (vtype ENUM), #179 (objective GROUP BY skip) and #181 (the HiGHS column ingest, framed better there as a bug: the ORDER BY c.col was an unbounded global sort). Those implementations are the same as the ones I had; theirs are narrower and better tested, so I dropped mine rather than duplicate them.

What is left is the piece nothing else covers.

The change

_label_frame numbered every coordinate with ROW_NUMBER() OVER (ORDER BY ord, …) — a full sort of every chunk, and 71% of build at 10M variables by the profiler in #182.

When a where does not read the leading dim, the surviving trailing coordinates are identical for every leading value. Rank them once over a table of size other and the label is (lead.ord - lo) * width + rank: no window, no per-chunk sort.

Verified rather than assumed — built both frames and diffed them: 0 differing rows at 1M and at 10M, and the same on the walkthrough model. The labels are the same integers, not merely valid ones, which is what makes this an optimisation rather than a decision. Labels decide which solver index a coordinate gets, so a fast path that renumbered them would be a different model on the wire.

Relationship to #152

#152 does the same idea for the narrower case: where is None, computed in closed form. This subsumes it on every case in the ladder — #152 does not fire on dispatch at all, because its where is a ParameterComparison rather than None, and that is the case that produced the "2x slower" headline.

Neither strictly dominates: #152 fires on 1-D frames, where this one requires a non-empty rest. On the ladder that is con_power_balance — 100k rows against 10M, so noise. The end state is three tiers — closed form when unmasked, ranked survivors when the mask factors, window otherwise — and whoever lands second should do the merge; they conflict in _label_frame.

Measured

Interleaved against the base, best of 3, memory_limit=1GB, top rung — build time:

case before after
dispatch 10M 3.54 s 2.08 s −41%
transport 9.8M 4.78 s 2.83 s −41%

(Those numbers were taken with #174/#179/#181 also applied; this commit is the dominant term but not the only one. bench/results/latest.jsonl is deliberately not regenerated here — the published ladder should be re-run once the stack has landed, not per-PR against a moving engine.)

Also

examples/walkthrough.py gains an explicit sort_values on primal(). A label join has no inherent row order, so the golden file was pinning storage layout rather than a contract — true before this change, but this is what surfaced it.

Verification: 342 passed, 1 xfailed · ruff check + format clean · pyrefly 0 errors.

🤖 Generated with Claude Code

@FBumann FBumann added the area:engine Lowering, IR, relational executor, sinks label Jul 27, 2026
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@FBumann, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 24 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a17a2e0-dd6a-4b37-b17d-3e0df82430f4

📥 Commits

Reviewing files that changed from the base of the PR and between a2e240e and edfc5f4.

⛔ Files ignored due to path filters (1)
  • examples/walkthrough.out is excluded by !**/*.out
📒 Files selected for processing (2)
  • examples/walkthrough.py
  • src/farkas/relational/executor.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/investigate-build-throughput

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@FBumann

FBumann commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Added a second harness — bench/regressions/, on pytest-benchmem — because the perf work this PR starts needs a metric that bench/run.py cannot provide.

Why a second one

bench/run.py publishes how farkas compares to linopy, so it must measure peak RSS: that is the number a reader can check with /usr/bin/time. It is a poor regression detector, though — RSS moves with machine load, and I mis-read a coin-flip OOM as a regression twice this week before bisecting settled it.

pytest-benchmem is pytest-benchmark's timing plus a memray peak pass, with isolate=True so every pass runs in a fresh process. That matters twice: duckdb would otherwise be measured with a warm buffer pool, and isolation is what makes whole-process rss available beside the memray peak, so the two can be watched for divergence.

Why memray is right there and wrong in the ladder

Measured, dispatch/m, same runs reporting both:

arm ru_maxrss memray peak
farkas 309 MB 211 MB
linopy 604 MB 2967 MB

memray counts polars' reserved arenas as allocated and does not count the interpreter or mapped libraries at all, so the bias points in opposite directions in the two lanes: the peak ratio is 0.51x by RSS and 0.07x by memray. Publishing that would turn a defensible 2x memory win into a fictitious 14x one — false to anyone who checked. Within one lane the same bias sits on both sides of a diff and cancels, leaving a metric that is deterministic and attributable to a call stack.

So: RSS for the comparison we publish, memray for the regressions we chase, and --benchmark-memory-compare-fail=mean:10% to make the second a gate.

Using it

uv sync --group bench
uv run pytest bench/regressions --benchmark-memory

Four workloads, ~74 s, reporting time and both memory metrics side by side:

  Name (time in ms)                     Min        Mean   │  peak·min (MiB)   rss·min (MiB)
  test_build_and_write[dispatch-s]   143.61      145.40   │           34.58          192.75
  test_build_and_write[nodal-m]      356.37      407.45   │           69.10          230.52
  test_build_and_write[dispatch-m]   526.52      537.78   │          162.05          312.64
  test_build_and_write[transport-m]  691.52      751.12   │          196.14          393.75

Its own dependency group (--group bench), not dev — nothing in the test suite or the published ladder needs memray. testpaths is still tests, so uv run pytest does not collect it. The tests assert a column count as a cheap correctness gate: the published ladder has a parity gate against linopy, this one at least refuses to time a model it did not build.

@FBumann
FBumann force-pushed the claude/investigate-build-throughput branch from e419723 to 3a4aea6 Compare July 27, 2026 07:54
The label frame was the single most expensive statement in build, and the cost
was the sort inside

    ROW_NUMBER() OVER (ORDER BY t_snapshot.ord, t_generator.ord)

When the mask does not read the leading dim, that sort is avoidable without
changing a single label. `where: "p_max > 0"` depends only on generator, so
the surviving trailing coordinates are identical for every snapshot: rank them
once over a table of size prod(rest), and the label is
`(lead.ord - lo) * width + rank` — no window function, no per-chunk sort. The
general path stays for masks that do read the leading dim.

Measured with bench/run.py, best of the repeats, on this machine:

  case/size    build before  build after          vs linopy
  dispatch/m       1.27s        0.88s        2.20x -> 1.67x
  dispatch/l       8.66s        5.01s        1.35x -> 0.90x
  transport/m      1.56s        1.10s        2.03x -> 1.65x
  transport/l     10.67s        7.27s        1.65x -> 1.31x

dispatch/l crosses over: farkas is now faster than linopy end to end at 10M
variables. The absolute ratios are machine-specific and differ from the
committed macOS numbers, so the delta is the reliable part; docs/benchmarks.md
is left for a run on the machine its ladder was measured on.

Labels are unchanged, and that is checked rather than assumed: building the
same 10M-row frame both ways and diffing gives 0 differing rows, and the same
holds on the walkthrough model.

One test moved. examples/walkthrough.py printed `sol.primal('p').head(6)`
unsorted, so its golden file pinned the physical storage order of var_p —
which this change alters even though the labels do not. primal() is a label
join and a join has no inherent row order, so the example now sorts
explicitly. That makes the golden stable under any future storage change
(chunking, parallelism, a duckdb upgrade), which it was not before.

Left alone deliberately: `primal()` itself still returns rows in whatever
order the join produces. Adding an ORDER BY there would also hit
_solution_to_parquet, which streams, and a global sort is exactly what that
path exists to avoid.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0145rKirxsCRGodhkKozjaYW
@FBumann
FBumann force-pushed the claude/investigate-build-throughput branch from 3a4aea6 to 53fde19 Compare July 27, 2026 07:59
@FBumann FBumann changed the title perf: cheaper labels, cheaper columns, and a profiler that says where the time goes perf: cheaper labels, cheaper columns, cheaper objective Jul 27, 2026
@FBumann
FBumann force-pushed the claude/investigate-build-throughput branch 2 times, most recently from 22cbc80 to edfc5f4 Compare July 27, 2026 08:01
@FBumann FBumann changed the title perf: cheaper labels, cheaper columns, cheaper objective perf: compute variable labels arithmetically when the mask factors Jul 27, 2026
@FBumann FBumann added the perf Wall time, memory, or I/O cost — not a wrong answer label Jul 27, 2026
FBumann added a commit that referenced this pull request Jul 28, 2026
…ically (#212)

Carried over from #178, which measured this on duckdb and cannot merge — that
PR is `ROW_NUMBER() OVER` and `CREATE TABLE surv_*` against an engine #189
deletes. The reasoning survives; the SQL does not.

A mask that reads none of the leading dims removes the same coordinates under
every one of their values, so the survivors are a rectangle: the full product
of the leading dims against one surviving set. Ranking that set sorts the
*set*, not the product — on `dispatch` it sorts the generators rather than 10M
`(snapshot, generator)` pairs — and the label is then arithmetic, exactly as
the unmasked path already is.

Masked label assignment at the `l` rung, and its share of build:

  dispatch   0.248 s (66%) -> 0.016 s (7%)    build 0.38 -> 0.24 s
  nodal      0.170 s (46%) -> 0.010 s (4%)    build 0.37 -> 0.24 s
  sector     0.102 s       -> 0.003 s         build 0.33 -> 0.27 s

`sector`'s *rows* keep the counted path and should: `where: demand` reads a
parameter dimensioned `[snapshot, node, carrier]`, so the mask sees the
leading dim and the survivors are not a rectangle. The prefix has to be
leading rather than merely absent from the mask — a label follows declaration
order, and only a prefix leaves the surviving set contiguous under each of its
coordinates.

The new test runs both paths over one model and compares them outright,
because an off-by-one here is a different model rather than a slower one.
Swapping the stride for the rank in the label arithmetic fails it and three
others.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@FBumann FBumann added the engine:duckdb Specific to the duckdb executor — invalidated if the polars path wins label Jul 28, 2026
@FBumann
FBumann merged commit 5d4745f into main Jul 28, 2026
3 checks passed
FBumann added a commit that referenced this pull request Jul 28, 2026
…ract

`perf: compute variable labels arithmetically when the mask factors` (#178)
shipped the same closed form this branch proposed, and went further — it
also factors a mask that misses the leading dim. `_row_major_label` is
superseded by `SqlCompiler.positional_label`, so the executor takes main's
side wholesale and this branch's code change is gone.

What survives is what #178 did not bring:

- the row-major *contract* in ARCHITECTURE.md, reworded for the three paths
  main now has (arithmetic / factored / counted) rather than the two this
  branch wrote it for
- labels asserted against a spelled-out row-major expectation over three dims
  of distinct cardinality, so a swapped stride cannot pass and the test cannot
  agree with the implementation by sharing its arithmetic

`test_masked_labels_stay_dense` was written when any mask meant the window.
Under #178 its predicate (`cap` on the trailing dim) factors instead, so it
would no longer have covered the path it names. Split in two: it now masks on
the *leading* dim, which cannot factor, and is the only test that fails when
the counted path loses its running offset; the factored path gets its own
test against the same spelled-out expectation.

Mutation-checked both ways: reversing the factored stride fails the factored
test, dropping the chunk offset fails the counted one.

406 passed, 1 xfailed; ruff and pyrefly clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FBumann added a commit that referenced this pull request Jul 28, 2026
release-please took its changelog entry from #152's squash subject, which
was the PR's *old* title: the PR was retitled after the merge button had
already been pressed, and the release was cut 22 seconds later. So alpha.30
announces `unmasked label assignment needs no sort` under Performance,
crediting the same optimisation alpha.29 already shipped as #178 and
attributing measurements to a commit that changes no source file.

Recategorised as Documentation under the title the PR actually merged with,
with a note that alpha.30 is alpha.29 plus tests. release-please is
manifest-driven and only prepends, so editing a released section is safe.

The commit subject on main stays wrong -- a tag, a release and two published
assets point at that SHA, and rewriting main to fix a message is not worth
that. The changelog and the release notes are where anyone actually looks.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
FBumann added a commit that referenced this pull request Jul 28, 2026
main gained the three duckdb perf PRs (#152, #174, #178), two language-level
fixes (#201, #223) and the bench harness from #205. Resolved by lane:

**Engine files take the polars side.** #152, #174 and #178 are `ROW_NUMBER()
OVER`, `CREATE TYPE ... AS ENUM` and `CREATE TABLE surv_*` against the engine
this branch deletes. Their *ideas* are already here in polars form — #152's as
`_positional`, #174's as #211, #178's as #212 — so taking main's side would
have re-added SQL against a connection that no longer exists.

**Language and validation take main's.** #223's `lowering.py`,
`validation.py` and `linopy/builder.py` changes merged clean and are engine
independent; its executor half and #201's are already here (as #225 and
`_check_one_row_per_coordinate`). `test_objective.py` and `test_validation.py`
arrived whole and pass on this lane unmodified — 404 tests to 414.

**bench takes ours**, which carries #205's two ideas ported rather than
merged (#215): the duckdb commit also brings `--memory-limits`, `--chunk-rows`
and `workdir_bytes`, the budget apparatus this branch removed.

Two tests were combined rather than picked. `test_group_sum`'s docstring says
both true things now — the divergence between lanes, and the aggregate-skip
premise that rests on it. `test_milp` keeps the polars Enum test and gains
main's ragged-chunk test, which is API-level and applies to this sink too.

Main's duckdb label tests are dropped, not ported: they drive `DuckdbExecutor`
and `ex._con` directly. The properties they pin are covered here by
`test_a_mask_that_removes_nothing_labels_exactly_like_no_mask` and
`test_a_factored_mask_labels_exactly_like_the_counted_path`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FBumann added a commit that referenced this pull request Jul 28, 2026
Full ladder after merging main: five cases, two sinks, both arms, best of
three, parity gate green on all five. 288 records, no failures.

**Every ratio is unchanged within noise, and that is the expected result.**
main's three duckdb perf PRs cannot move this comparison: neither arm here is
duckdb. The farkas arm is the polars engine and the linopy arm is
`farkas.linopy.build`, so the work in #152, #174 and #178 has nothing to act
on — their ideas already arrived on this branch as `_positional`, #211 and
#212, and it is those that the numbers already reflected.

`transport` reads 1.59x / 0.81x where it read 1.62x / 0.78x, `sector` 0.31x
where it read 0.32x, and so on. The tables and every figure quoted in the
prose are refreshed off the new file so the doc and its provenance agree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FBumann added a commit that referenced this pull request Jul 28, 2026
…sult (#231)

* bench: a case where I/O is not noise, and a sink axis to measure it on (#205)

Two changes to the harness, both about measuring what users actually do.

**`profiled`, the case whose input is the same size as its model.** Every
existing case builds its coordinate product from parameters far smaller than
the product they explode into — 2-15% of it — so reading the parquet is 2-9%
of the build and the read path is lost in the margin. `profiled` is `nodal`'s
cardinalities with the mask removed and `availability` dense over
`(snapshot, node, tech)`: at the `l` rung, a 12M-row table against a 12M
coordinate product. That makes the read path, the join, and the
in-memory-vs-parquet question measurable rather than noise.

It is also the shape the eager lane handles best — a parameter already dense
over the variable product is the array xarray wants, no broadcast and no
alignment — while we join a full-size frame against a full-size coordinate
product. `nodal` is the mirror image and our clearest win. Carrying both is
what keeps the ladder from holding only shapes that suit one engine. Closes #202.

**Both sinks, not just the LP file.** `--sinks lp highs`, defaulting to both.
The LP file is the artifact fewest callers want, and it is not the same
comparison: at `profiled/m` the peak ratio is 0.60x through the LP writer and
0.94x through the handoff, because HiGHS's own dense model is resident in both
arms and swamps the difference between them.

The `highs` sink stops at the handoff — `run()` is never called. The simplex is
the same work whoever filled the model, so including it would swamp the phase
this harness exists to measure. `solve_direct` is split into `build_highs` +
the solve so there is a seam to stop at; linopy's `Model.to_highspy()` is the
same seam on the other side, which is what makes the two arms comparable.
A rung the sink is capped out of is written to the JSONL as a `skipped` record
and footnoted in the report, so a coverage hole cannot read as a result.

Measured, `profiled` at the `l` rung (12M variables): LP file, farkas 8.42 s /
1.49 GB against linopy 2.44 s / 3.26 GB; HiGHS handoff, farkas 7.24 s / 2.60 GB
against linopy 2.93 s / 3.23 GB. The parity gate passes on the new case.

No published numbers change here: `bench/results/latest.jsonl` is left as it
was, since re-running the ladder is its own act.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: refuse a parameter source that carries a coordinate twice (#201)

A parameter is a function of its dims, so two rows for one coordinate has no
defined answer. The eager lane already refuses such a source — `xarray` will
not lay out a duplicated index — while the relational lane quietly resolved it
into a sum. Two lanes that accept the same language were accepting different
data, and the one that accepted more gave an answer nobody asked for.

Now it raises a `DataError` naming the parameter and up to three offending
coordinates. One `GROUP BY … HAVING count(*) > 1` per parameter, over a source
orders of magnitude smaller than the model built from it.

`test_a_coordinate_must_be_single_valued` had to be narrowed: it doubled the
whole generator frame, which feeds both the coordinate index *and* two
parameters, so the new check now fires first and correctly. It doubles only the
index; the parameter case is its own test beside it.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(main): release 0.0.0-alpha.27 (#210)

Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com>

* fix: three ways an objective or a scalar parameter answered quietly (#223)

* fix: an objective sums the terms it names, once each

Two ways an objective quietly answered a question nobody asked.

`x * a + y * b`, with `x, a` on `i` and `y, b` on `j`, is `|i| + |j|` summands.
The relational lane knew that — an expression there is a set of term
fragments, each keeping its own dims until the objective sums it. The eager
lane added the two operands first, which is linopy's `+`, which broadcasts:
every term counted once per coordinate of its sibling, `|i| * |j|` summands,
8.8x apart on the mixed-density case that found it (#197). Hard rule 3 says
the lanes mean the same thing, and here the relational one was right.

So the eager lane distributes the sum over addition and sums each product on
its own. The distribution has to survive an operator applied to the group —
`(x * a + y * b) * c` was wrong by the same factor — so `*`, `/` and unary
minus are walked too, and everything else stays one opaque term.

Separately, `objectives.<name>.equations` is a list of which only the first
entry was ever read. It reads as "several terms in one objective", because
that is what the key means under a constraint, and the answer was wrong by
whatever the ignored entries were worth with nothing to see (#198). It is now
a load error naming the rewrite, spelled from what was written.

SPEC said "only equations[0] is used" in §2 and "the last defined wins" for
multi-objective in §11 — the second was already false, since a second
objective has been a load error for a while.

Closes #197
Closes #198

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: a dimensionless parameter is one row, and now has to be

`_check_one_row_per_coordinate` returned early when a parameter had no dims,
on the reading that there was no coordinate to group by. But a parameter with
no dims has exactly one coordinate — the empty one — so the rule it was
skipping is the rule that matters most there.

The join that broadcasts a dimensionless parameter is `ON TRUE`, which is
correct broadcast semantics for one row and a silent row multiplication for
two: duplicate `cols` entries for one variable in a bound, duplicate mask rows
in a where. Both built and solved without a word. A user who gets here has
usually passed a column they thought was indexed, so the message says what a
dimensionless parameter means rather than only what is wrong with the source.

Zero rows is caught by the same count, and was the other half of the hole.

One `count(*)` per scalar parameter, at metadata level.

Closes #166

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(main): release 0.0.0-alpha.28 (#226)

Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com>

* perf: compute variable labels arithmetically when the mask factors (#178)

The label frame was the single most expensive statement in build, and the cost
was the sort inside

    ROW_NUMBER() OVER (ORDER BY t_snapshot.ord, t_generator.ord)

When the mask does not read the leading dim, that sort is avoidable without
changing a single label. `where: "p_max > 0"` depends only on generator, so
the surviving trailing coordinates are identical for every snapshot: rank them
once over a table of size prod(rest), and the label is
`(lead.ord - lo) * width + rank` — no window function, no per-chunk sort. The
general path stays for masks that do read the leading dim.

Measured with bench/run.py, best of the repeats, on this machine:

  case/size    build before  build after          vs linopy
  dispatch/m       1.27s        0.88s        2.20x -> 1.67x
  dispatch/l       8.66s        5.01s        1.35x -> 0.90x
  transport/m      1.56s        1.10s        2.03x -> 1.65x
  transport/l     10.67s        7.27s        1.65x -> 1.31x

dispatch/l crosses over: farkas is now faster than linopy end to end at 10M
variables. The absolute ratios are machine-specific and differ from the
committed macOS numbers, so the delta is the reliable part; docs/benchmarks.md
is left for a run on the machine its ladder was measured on.

Labels are unchanged, and that is checked rather than assumed: building the
same 10M-row frame both ways and diffing gives 0 differing rows, and the same
holds on the walkthrough model.

One test moved. examples/walkthrough.py printed `sol.primal('p').head(6)`
unsorted, so its golden file pinned the physical storage order of var_p —
which this change alters even though the labels do not. primal() is a label
join and a join has no inherent row order, so the example now sorts
explicitly. That makes the golden stable under any future storage change
(chunking, parallelism, a duckdb upgrade), which it was not before.

Left alone deliberately: `primal()` itself still returns rows in whatever
order the join produces. Adding an ORDER BY there would also hit
_solution_to_parquet, which streams, and a global sort is exactly what that
path exists to avoid.


Claude-Session: https://claude.ai/code/session_0145rKirxsCRGodhkKozjaYW

Co-authored-by: Claude <noreply@anthropic.com>

* perf: cols.vtype is an ENUM, not a VARCHAR per column (#174)

`vtype` is one of three literals written once per column, which as VARCHAR
made it the widest thing on the row — and the row count is the model's
column count. An ENUM stores the tag instead.

Nothing downstream moves: `vtype = '...'` comparisons in both sinks keep
working against an ENUM unchanged.

Measured on the branch this was split out of (#153): 0.87s -> 0.48s writing
`cols` at 10M columns.

The members come off `plan.VariableType` rather than being typed out, so the
Literal stays the single declaration. The test pins that: an ENUM rejects a
value it does not know, so a fourth variable type added to the plan and not
here would otherwise fail at insert with a duckdb cast error a long way from
its cause.

Closes #170.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore(main): release 0.0.0-alpha.29 (#227)

Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com>

* perf: unmasked label assignment needs no sort (#152)

`_label_frame` numbered every coordinate with
`ROW_NUMBER() OVER (ORDER BY ord, ...)`, a full sort of every chunk. It was
71% of build at 10M variables (#146), and the sort is the whole cost: the
same statement without the ORDER BY runs at the join floor.

When nothing is masked out, that rank has a closed form -- it is the
mixed-radix value of the `ord` tuple -- so the label can be computed instead
of sorted for. Isolated at 10M rows: 4.37s -> 0.56s. End to end, interleaved
best-of-three, `transport` at 9.8M variables: build 10.19s -> 6.52s (-36%).

The labels are *the same integers*, not merely valid ones, which is what
makes this a pure optimisation rather than a decision. #151 weighed dropping
the ordering guarantee instead and had to leave three questions open --
`solver_direct`'s batching, reproducibility (#109), and the locality of `A`.
None of them arise here: nothing downstream can tell the two apart, and the
walkthrough golden that a renumbering would have forced to regenerate is
untouched.

Masked frames keep the window. Denseness there depends on which rows survive
the predicate, and no closed form knows that -- so `dispatch`, whose variable
is masked by `where: p_max > 0`, is unchanged by this commit.

Verified: HiGHS `addRows` is insensitive to column-index order within a row
(bit-identical objective and primal under reversed and shuffled indices), so
the `ORDER BY` was not load-bearing for the shipped path either.

332 passed; ruff and pyrefly clean.


Claude-Session: https://claude.ai/code/session_013DtGqM3z2DTtT556178iXi

Co-authored-by: Claude <noreply@anthropic.com>

* chore(main): release 0.0.0-alpha.30 (#228)

Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com>

* docs(bench): re-run the ladder on the merged tree

Full ladder after merging main: five cases, two sinks, both arms, best of
three, parity gate green on all five. 288 records, no failures.

**Every ratio is unchanged within noise, and that is the expected result.**
main's three duckdb perf PRs cannot move this comparison: neither arm here is
duckdb. The farkas arm is the polars engine and the linopy arm is
`farkas.linopy.build`, so the work in #152, #174 and #178 has nothing to act
on — their ideas already arrived on this branch as `_positional`, #211 and
#212, and it is those that the numbers already reflected.

`transport` reads 1.59x / 0.81x where it read 1.62x / 0.78x, `sector` 0.31x
where it read 0.32x, and so on. The tables and every figure quoted in the
prose are refreshed off the new file so the doc and its provenance agree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test: route the objective tests' pandas through the oracle guard

`test_objective.py` arrived from main with a bare `import pandas as pd`. It
merged without a conflict, so nothing flagged it — and on this branch pandas
is not a core dependency, so the bare-install CI job died collecting it.

Every other test module on this branch takes pandas from `tests.oracle`, which
is a `pytest.importorskip` behind the `[linopy]` extra. `test_duals.py` even
carries the comment this file needed: "through the guard: a bare import would
beat it".

Reproduced against an install with no pandas, linopy or xarray: 11 skipped
and a collection error before, 214 passed and 21 skipped after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: fluxopt-release[bot] <267260463+fluxopt-release[bot]@users.noreply.github.com>
FBumann added a commit that referenced this pull request Jul 28, 2026
Adopts linopy's v1 reading of absence as the language's own, and makes v1 the
oracle rather than a mode we happen to survive.

**What changes.** A term whose variable is masked out no longer contributes
zero — it makes the row absent, so `x + y >= 10` is *no constraint* where `y`
is masked rather than `x >= 10`. The old reading is how

    x - rel_max * size <= 0

silently became `x <= 0` on an unsized component: feasible model, plausible
answer, no error. That is goal 1 of the v1 convention ("no silent wrong
answers") and the whole of PyPSA/linopy#712, and it was reachable here.

**Two things deliberately do not propagate.** A *reduction* skips absent slots
(§13), so `sum(x, over=d)` stays defined when only some of `d` exists — without
that, one masked component would delete a system-wide accounting row. A
*parameter* covering only some coordinates is sparse encoding, not absence: its
missing rows mean a zero coefficient (SPEC §8), which is what lets a coefficient
table hold live entries only. Absence is a property of variables.

**How the engine tells them apart.** `TermFragment.presence` carries the
variable's own coordinates beside the term stream, because once `coeff x var`
are multiplied the frame cannot say which side removed a row. It is set only
for a variable whose declaration has a `where` — decided off the plan, before
data — so an unmasked variable never imposes the cost, and `_label_frame` keeps
both of its arithmetic paths (#152, #178) for every equation that does not turn
on the difference.

**The oracle is v1, and it raises rather than skips.** A skip would be the worst
outcome available: the suite would go green having stopped comparing the lanes
on exactly the cases the convention changed. No release carries the option yet,
so `[tool.uv.sources]` pins PyPSA/linopy#717 by branch — by branch and not by
rev on purpose, since a stale rev would measure us against a spec that has moved.

Not included, and it is the follow-up this needs: `defined(v)` (#219). Dropping
the row is now the only reading available, and the way to ask for the other one
is complementary `where` clauses over a variable's existence — which is not yet
sayable. Until it lands, a model wanting "keep the row, treat the term as zero"
has to carry a parameter mirroring the variable's mask.

`test_a_constraint_row_left_with_no_variables` stays xfailed and is *not* this:
raw linopy builds a term-less row under both conventions (`labels=[0,1]`,
`vars=[-1]`), so that divergence lives in our own eager lane and wants its own
diagnosis.

Refs #8, #219
FBumann added a commit that referenced this pull request Jul 28, 2026
…ow (#234)

* feat(compat): answer linopy's v1 convention where the position knows the answer

The oracle lane runs clean under `linopy.options["semantics"] = "v1"`, which
issue #8 has been parked on since the tracked PR (PyPSA/linopy#591) was closed
unmerged. The live work is PyPSA/linopy#717; its spec is `doc/design/
convention.rst` on `feat/arithmetic-convention`.

Two things the convention changes for us, and one it turns out it cannot:

**§8 never fires.** Its "shared dimensions must carry identical labels" exists
because linopy operands are arbitrary xarray objects with independent indexes,
so a mismatch is ambiguous between "different data" and "subset". We resolve a
master coordinate per dimension before any data binds (SPEC §8) and reindex
every operand to it, so a superset already raises, a subset is filled and a
reorder is fixed by label. Measured: four orderings of `a + factor + b` with a
sparse factor and a masked variable produce byte-identical rows, so the
associativity break v1 §8 exists to stop (PyPSA/linopy#711 — chained left-joins
dropping coordinates) has no mechanism here.

**§5 does fire, on our central idiom.** `load_parameters` reindexes to the
master coordinates, so a parameter covering a subset of its dims arrives at
linopy as NaN — and v1 refuses a NaN in a user-supplied constant, because from
inside linopy a deliberate absence and a data error are indistinguishable. That
is every sparse parameter, which is what SPEC §8 means by "sparse data gives
sparse variables" and what a generated binder emits by the tableful. The 414
tests passing beforehand were not evidence otherwise; the suite had no sparse
float parameter. It has one now, and it fails without this change.

**So the answers are given per position, not once in the loader.** The same
missing row means three different things: zero in a coefficient, an error in
`bounds:` (unbounded is not bounded-at-zero), false in a `where` (SPEC §6's
bare name is "non-null and finite"). A single fill in the loader would have to
pick one and be wrong for the other two.

`linopy/semantics.py` is where those answers live, mirroring linopy's own
module of that name and for its reason: the evaluator stays about evaluating,
and a later change to the convention is a single-file diff. All three functions
are correct under the legacy convention too — they resolve absence to the value
legacy reached implicitly — so none is conditional on the option. What v1
changed is that staying silent stopped being available.

Verified on three configurations: released linopy 0.9.0 (415 passed), the v1
branch under `semantics="v1"` (415 passed), and the same branch under
`semantics="legacy"` (413 passed, 2 failing only on `LinopySemanticsWarning`
promoted to an error by our own filterwarnings).

Refs #8

* refactor(engine): a fragment loses rows for two reasons; carry them apart

Prerequisite for adopting v1's absence semantics, and inert on its own — no
behaviour changes, 415 tests unmoved.

A term fragment can lose rows at a coordinate for two unrelated reasons, and a
constraint row has to react to exactly one of them. A **masked variable** is
genuinely absent there. A **sparse parameter** is a compressed dense array whose
missing rows mean a zero coefficient — SPEC §8's "sparse data gives sparse
variables", and what a generated binder emits by the tableful. Once the two are
multiplied into one frame the distinction is gone, which is why "drop the row
where a variable is absent" could not be written as an anti-join against the
term stream.

So the variable's own coordinates ride alongside as `TermFragment.presence`,
rewritten by the same shape operators that rewrite the term. The propagation
rule follows from the convention rather than from convenience:

  variable leaf        the variable's frame
  parameter, constant  none — sparsity is an encoding, not absence
  a * b, a / b, -a     the variable side's, unchanged: a sparse coefficient
                       zeroes a term, it does not unmake the variable under it
  sum, group_sum       cleared — §13 has reductions *skip* absent slots rather
                       than propagate them, which is what keeps a cross-module
                       accounting equation summing over a partly-masked dim
  roll, shift          still to do: the coordinate map has to be applied to
                       presence too, and shift's vacated edge unioned back in,
                       since SPEC §7 declares it contributes zero

Refs #8

* feat!: absence propagates and drops the row, on both lanes

Adopts linopy's v1 reading of absence as the language's own, and makes v1 the
oracle rather than a mode we happen to survive.

**What changes.** A term whose variable is masked out no longer contributes
zero — it makes the row absent, so `x + y >= 10` is *no constraint* where `y`
is masked rather than `x >= 10`. The old reading is how

    x - rel_max * size <= 0

silently became `x <= 0` on an unsized component: feasible model, plausible
answer, no error. That is goal 1 of the v1 convention ("no silent wrong
answers") and the whole of PyPSA/linopy#712, and it was reachable here.

**Two things deliberately do not propagate.** A *reduction* skips absent slots
(§13), so `sum(x, over=d)` stays defined when only some of `d` exists — without
that, one masked component would delete a system-wide accounting row. A
*parameter* covering only some coordinates is sparse encoding, not absence: its
missing rows mean a zero coefficient (SPEC §8), which is what lets a coefficient
table hold live entries only. Absence is a property of variables.

**How the engine tells them apart.** `TermFragment.presence` carries the
variable's own coordinates beside the term stream, because once `coeff x var`
are multiplied the frame cannot say which side removed a row. It is set only
for a variable whose declaration has a `where` — decided off the plan, before
data — so an unmasked variable never imposes the cost, and `_label_frame` keeps
both of its arithmetic paths (#152, #178) for every equation that does not turn
on the difference.

**The oracle is v1, and it raises rather than skips.** A skip would be the worst
outcome available: the suite would go green having stopped comparing the lanes
on exactly the cases the convention changed. No release carries the option yet,
so `[tool.uv.sources]` pins PyPSA/linopy#717 by branch — by branch and not by
rev on purpose, since a stale rev would measure us against a spec that has moved.

Not included, and it is the follow-up this needs: `defined(v)` (#219). Dropping
the row is now the only reading available, and the way to ask for the other one
is complementary `where` clauses over a variable's existence — which is not yet
sayable. Until it lands, a model wanting "keep the row, treat the term as zero"
has to carry a parameter mirroring the variable's mask.

`test_a_constraint_row_left_with_no_variables` stays xfailed and is *not* this:
raw linopy builds a term-less row under both conventions (`labels=[0,1]`,
`vars=[-1]`), so that divergence lives in our own eager lane and wants its own
diagnosis.

Refs #8, #219

* feat: a bare variable name in a where asks whether it exists

The escape hatch the previous commit made necessary. Absence now takes the row
with it, so a model wanting the other reading — keep the row, treat the term as
zero — needs a way to name those coordinates. Without it the only spelling is a
parameter mirroring the variable's own mask: two sources for one fact, and when
they drift the failure is not an error but a row quietly pinned to zero.

Surface is the one the language already implies rather than a new one. A bare
*parameter* name in a where asks "does this have a value here"; a bare
*variable* name asks "does this exist here". Same grammar, same shape, and
resolution already had a branch for the case — it just raised.

    - expression: x - rel_max * size <= 0
      where: "size"
    - expression: x <= 0
      where: "NOT size"

Pointwise on both lanes: a semi-join against the variable's frame relationally,
`labels != -1` eagerly. `_predicate_dims` reads it through the variable's
`foreach` exactly as a parameter is read through its dims, so `_free_prefix`
keeps its arithmetic path for the leading dims a mask cannot see.

**A self-reference is now a load error**, found by the parity sweep, which masks
`variables.p.where` and so asked `p` whether `p` exists. It died in linopy with
`KeyError: 'p'` — nothing in this package's voice. A variable's own where cannot
ask whether it exists, because that mask is what decides it.

Two limits, both deliberate and both visible in the tests. The dim rule refuses
a variable whose foreach exceeds the frame — masking `balance` over `[snapshot]`
by `p` over `[snapshot, generator]` would silently widen the mask, and saying
which reduction is meant needs ROADMAP Track 1 item 6 (`all(x, over=d)`). And
the predicate has no home in the dispatch-model parity sweep for that same
reason, so `COVERED_ELSEWHERE` maps it to the test that does exercise it rather
than letting the coverage guard be quietly weakened.

Closes #219. Refs #8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:engine Lowering, IR, relational executor, sinks engine:duckdb Specific to the duckdb executor — invalidated if the polars path wins perf Wall time, memory, or I/O cost — not a wrong answer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants