perf: close the build-time gap, and measure the mask the ladder never measured

[FBumann]

Stacked on #143 (the harness) — retarget to main when that lands.

Overlaps #152. That PR does a narrower version of the same idea and the two are complementary, not competing. Details and a per-case table at the bottom — worth reading before merging either.

The docs blamed the wrong phase

docs/benchmarks.md attributed the ~2× to "fixed work — re-scanning the vars table per section, ~100M printf calls, a 2.6 GB CSV write". Those are all emit-side. The committed phase timings say otherwise:

case/size	build: fk	build: linopy		emit: fk	emit: linopy
dispatch/l	3.89s	0.40s	9.7×	2.78s	2.79s	1.0×
transport/l	5.49s	0.90s	6.1×	3.76s	3.07s	1.2×

Emit was already at parity with the writer we're compared against. The whole gap was build. (The phases don't do the same work — linopy defers coefficient materialisation to emit — so the ratio isn't apples-to-apples, but it localises our cost: build was 58% of our wall.)

bench/profile_build.py narrows it further: _build_variable was 40% of all SQL time, nearly all of it the chunked INSERT INTO var_p that assigns labels, and the cost was the sort in ROW_NUMBER() OVER (ORDER BY t_snapshot.ord, t_generator.ord). Building the same 10M-row frame four ways: 8.3s as written, 2.4s without the ORDER BY, 4.4s storing ordinals instead of values. Packing both sort keys into one BIGINT saves 15% — so it's the sort, not the key width.

The fix keeps the labels

Dropping the ORDER BY changes which coordinate gets which solver index, and tests/test_walkthrough.py pins that as a golden file. So instead: when the mask doesn't read the leading dim, the surviving trailing coordinates are identical for every leading value — rank them once over a tiny table and the label is (lead.ord - lo) * width + rank. No window, no 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.

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

The sparse case is the bigger result

dispatch masks on p_max > 0 against a p_max that is always positive — so its where removes nothing and the ladder only ever measured a fully dense coord product: the eager lane's best case and ours worst. transport has no where at all.

bench/models/sparse.yaml keeps ~20% of a 2-D product. On the regenerated ladder:

case, top rung	wall	peak
dispatch, 10M	1.00×	0.28×
transport, 9.8M	1.21×	0.48×
sparse, 2.1M live	0.57×	0.20×

docs/benchmarks.md listed a where-density sweep under "not measured yet" and predicted the gap would be "qualitative rather than 2×". It is — in our favour. The case also reads the leading dim, so it exercises the general sorted path rather than the new arithmetic one; the ladder now covers both.

Two smaller items

• vtype → ENUM. 'continuous' was written as VARCHAR once per column, the widest thing on the row. 0.87s → 0.48s at 10M, and no sink changed because vtype = '...' comparisons still work.
• The objective's GROUP BY, skipped only where provably dead: one fragment, one variable, fragment dims == variable dims. 0.60s → 0.22s.

pyrefly caught a real bug in the walker that decides the second one: plan.Divide spells its operands numerator/divisor, not left/right, so an objective containing a division would have raised AttributeError out of the match statement. There's a regression case for it now.

Docs

bench/results/latest.jsonl and the Results tables are a fresh full ladder on Linux x86-64, replacing a macOS/M-series set the label change had invalidated; the doc says explicitly not to compare them. Two stale claims corrected: the emit-side attribution above, and the transport 256 MB OOM — which did not reproduce here, recorded as the failure being marginal rather than fixed, since both the machine and the label frame changed.

Relationship to #152

#152 ("unmasked label assignment needs no sort") computes the row-major label in closed form when where is None. Mine ranks survivors into a small table when the mask doesn't read the leading dim. Measured, per label frame in the ladder:

frame	this PR	#152
dispatch var_p — 2-D, mask on the trailing dim	arithmetic	no (has a where)
dispatch con_power_balance — 1-D, unmasked	sorted window	fires
sparse var_p — 2-D, mask reads the leading dim	sorted window	no
sparse con_power_balance — 1-D, unmasked	sorted window	fires
transport var_p / var_f / con_balance — unmasked	arithmetic	fires

Neither subsumes the other. #152 doesn't fire on dispatch at all — its where is a ParameterComparison, not None — which is the case that produced the headline "2× slower". This PR doesn't fire on 1-D frames, where #152 does. Where both fire, #152's closed form should win: it needs no join.

The best end state is all three tiers — closed form when unmasked, ranked survivors when the mask factors, window otherwise — but that is a merge of someone else's open PR, so I've left it. Whichever lands second will conflict in _label_frame; happy to do the combining.

Verification

uv run pytest — 316 passed, 1 xfailed · ruff check + format clean · pyrefly 0 errors · parity gate green on all three cases (bit-identical objectives).

🤖 Generated with Claude Code

https://claude.ai/code/session_0145rKirxsCRGodhkKozjaYW

---

Generated by Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d5f6ccf-441f-4365-a9dd-fb3db70bd733

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[FBumann]

Closing in favour of one PR per change. Nothing here was wrong — the opposite, the measurements are the best evidence we have on build cost — but five independent changes in one PR means one changelog line for five perf: entries, no way to attribute the end-to-end numbers to any single change, and no revert boundary between them.

The pieces now have issues, each carrying this PR's measurement:

commit	issue
387c5c1d stream columns through Arrow buffers	#171
07ed9a87 vtype ENUM	#170
07ed9a87 objective GROUP BY col	#172
ef3aa375 labels when the mask factors	#173
86849dc5 bench: attribute build time to the SQL	folds into #143
the sparse rung + ladder regeneration	#147

Two findings from this PR that were not code and should not be lost:

• The ladder never measured a mask that bites. dispatch guards on p_max > 0 against a p_max that is always positive, and transport has no where at all — so every published number is the fully dense coordinate product, our worst case and the eager lane's best. bench/models/sparse.yaml at ~20% density came out at 0.57x wall / 0.20x peak, i.e. the gap is in our favour once a mask does anything. Recorded on Benchmark coverage: solver_direct, storage/MILP cases, sparsity sweep, speed-of-light floor #147.
• docs/benchmarks.md blamed the wrong phase. It attributed the ~2x to emit-side work (printf, the CSV write); the phase split says emit was already at parity and the whole gap was build. That correction belongs with whichever PR lands the ladder regeneration.

The branch stays; #173 is where its label commit gets re-cut, on top of #152 rather than beside it.

[FBumann]

Rebased onto main (#143 is merged) and dropped sparse.yaml — #143 landed nodal instead, which is the same axis with a model that occurs in the wild: dispatch over (snapshot, node, tech) where a technology only generates at a node it is installed at. It also sweeps density (12 / 6 / 3 / 1 technologies per node) rather than fixing one value. Your executor and sink commits are unchanged; only the bench case and its docs were dropped.

Two conflicts worth knowing about, both resolved in favour of main:

• examples/walkthrough.py — feat!: a solve result tells you whether it has one #148 renamed Solution to Result and split status / termination_condition. Kept main's naming plus your explicit sort_values on primal(), since the reason for it (a label join has no inherent row order) is unaffected by the rename.
• tests/test_relational.py — your objective-aggregate test asserted status == 'Optimal', which is now termination_condition == 'optimal'.

That second one was not only yours: the parity gate on main has the same bug and would abort every run. Fixed here in fix(bench): the parity gate reads termination_condition, not status.

The 256 MB OOM is a coin flip, not a regression

I regenerated the ladder on this branch and initially read it as a regression — dispatch/10M and nodal/3M now OOM at 256 MB where they used to build. Bisecting across your four commits gave a non-monotonic answer (commit 2 OOMs, commit 3 succeeds, commit 4 OOMs), which is the signature of marginality rather than a cause.

Confirmed by repetition on identical code, dispatch/10M at 256 MB:

2 of 6 runs completed, 4 OOMed.

So that budget cell is non-deterministic, and has been all along. Three consequences:

1. It explains why you could not reproduce the transport OOM — it is a coin flip, not a platform difference. Constraint assembly OOMs instead of spilling when the group_sum fan-in is wide #145 should be re-characterised: the build at 10⁷ variables under 256 MB fails non-deterministically, which is worse than a deterministic failure, because it is a flaky failure in production rather than an honest error.
2. Your changes are not the cause and are not blamed for it.
3. Your changes do help at generous budgets — peak drops on every case at 1 GB (dispatch 0.88 → 0.74 GB, nodal 0.58 → 0.51, transport 1.35 → 1.19) — but not enough to clear the cliff.

The harness should report a failure rate for tight-budget cells rather than a single cell that happens to be whichever way the last run fell. That is a follow-up on the harness, not on this PR.

Verification here: 341 passed, 1 xfailed · ruff + format clean · parity gate bit-identical on all three cases.

[FBumann]

Correction to the comment above. It said the pieces here were fine and needed only splitting. That was true of the two I had looked at; it is not true of all four.

Re-deriving 07ed9a87's objective change for #172 (now PR #179) turned up a silent wrong answer in the condition it used to decide when the GROUP BY col can be skipped:

one fragment, one variable, and the fragment's dims equal the variable's declared dims

That reads a fragment's dim tuple as if it described its row count. It does not. sum(…, over=d) in this engine is not a SQL aggregate — _sum_fragment drops d from the select list and leaves the rows alone, deferring the actual addition to the terminal aggregate this condition is trying to remove. So a fragment can report exactly the variable's dims while still holding many rows per var_label.

Reproduced against the shipped compiler, with q indexed by snapshot alone and price by (snapshot, generator):

objective      : sum(q * price, over=generator)
fragment dims  : ('snapshot',)
declared dims  : ('snapshot',)
condition fires: True
obj rows       : [(0,1.0), (0,2.0), (0,3.0), (1,1.0), (1,2.0), (1,3.0)]
expected       : [(0,6.0), (1,6.0)]

Downstream this splits: lp_file writes the duplicates as separate text terms and the LP format re-sums them, so it is accidentally right; solver_direct does cols LEFT JOIN obj USING (col), and a join against duplicate keys multiplies rows, so addCols appends more columns than the model has and every later column index shifts. The solve runs and returns a meaningless number.

#179 keeps the optimisation with a condition that holds, and pins this case as a test — one of its mutations is reverting to the condition above.

Status of the other three pieces, so nobody assumes more verification than happened:

piece	issue	state
vtype ENUM	#170	re-derived and tested (PR #174)
objective GROUP BY	#172	re-derived — bug found, fixed in #179
Arrow column ingest	#171	not re-derived
labels when the mask factors	#173	not re-derived — and it changes label assignment, so it is the one where a wrong condition costs most

Nothing here reflects badly on the measurements, which were good and are the reason these numbers exist at all. The lesson is narrower: a condition that decides when to skip a correctness-preserving step needs its counterexample hunted deliberately, not just a passing test suite — the differential tests did not catch this, because no model in them has a sum over a dim its variable lacks.