perf(engine): the read-back reads the label order rather than re-imposing it by FBumann · Pull Request #446 · fluxopt/specsolve

FBumann · 2026-07-31T19:47:25Z

#433's argument, at the third reader.

What

Every path in Labeller.frame hands back a label-ascending frame:

path	why
unmasked	`_in_label_order(...)` — verified, sort as the fallback
factored	`_in_label_order(...)` — same
counted	`with_row_index` after a sort — ascending by construction

_build_variable stores exactly that frame, and _read_back reads it — then
sorted it again. #433 removed this redundancy from cols; this is the third
reader.

It is not the seconds

Sorting moved a full copy of the coordinates, strings included, at the moment
the solver's own model is still resident — the worst point in the process to
allocate one. Reading every variable back:

case/l	time	extra RSS over the built model	process peak
dispatch	0.0624 → 0.0006 s	+681.0 → +0.2 MB	1.86 → 1.19 GB
fleet	0.0673 → 0.0029 s	+337.3 → +0.2 MB	2.20 → 1.87 GB
nodal	0.0216 → 0.0004 s	+225.6 → +0.1 MB	1.07 → 0.84 GB

Those peaks carry no solver — the vectors are synthetic. In a real solve the
saving lands on top of HiGHS's own copy of the model, which is resident the
whole time.

The read-back now allocates 0.2 MB total: select on an in-memory frame plus a
Series attach is zero-copy, so nothing is moved at all.

to_parquet pays this per variable — fleet has twelve.

Equivalence

Every declaration of every bench case at m and l — 66 variables and
constraints across six models, primal and dual — returns a frame identical to
the sorted one. Zero mismatches.

Tests

test_a_solution_is_read_back_in_label_order_without_sorting_for_it asserts the
contract, not just the mechanism: seeding the vector with an arange makes
every value its own label, so a read-back in label order comes out ascending.
That fails if the frames ever stop arriving ordered — which is the risk this
change takes on. The plan assertion is the other half, against the sort coming
back.

The model's mask puts the variable on the factored path and the constraint on
the counted one, so neither is ordered merely by being a scan.

The invariant

_in_label_order justified itself by cols alone. It now names both readers
that take the order on trust — a reason that lists one of two is how the
guarantee gets weakened for the other.

🤖 Generated with Claude Code

…sing it #433's argument, at the third reader. Every path in `Labeller.frame` hands back a label-ascending frame and two of them verify it, so `_read_back` sorting again re-established an order the frame never lost. It is not the seconds that matter. Sorting moved a full copy of the coordinates — strings included — at the moment the solver's own model is still resident, which is the worst point in the process to allocate one. Reading every variable of `dispatch/l` back: time over the built model process peak sort 0.0624s +681.0 MB 1.86 GB no sort 0.0006s +0.2 MB 1.19 GB fleet/l 0.0673s -> 0.0029s +337.3 MB -> +0.2 MB 2.20 -> 1.87 GB nodal/l 0.0216s -> 0.0004s +225.6 MB -> +0.1 MB 1.07 -> 0.84 GB Those peaks carry no solver: in a real solve the saving lands on top of HiGHS's own copy of the model, which is resident throughout. Every declaration of every bench case at `m` and `l` — 66 variables and constraints across six models, primal and dual — returns a frame identical to the sorted one. `_in_label_order` now names both readers that take the order on trust. It justified itself by `cols` alone, and a reason that lists one of two is how the guarantee gets weakened for the other. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-07-31T19:47:38Z

Warning

Review limit reached

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

Next review available in: 50 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: 5d45da27-5b36-4ed2-b05b-67c6795f331b

📥 Commits

Reviewing files that changed from the base of the PR and between a5825f2 and 5adb969.

📒 Files selected for processing (3)

src/lpspec/relational/engines/polars/executor.py
src/lpspec/relational/engines/polars/labels.py
tests/test_relational.py

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.}

#448 replaced the custom runner with a pytest suite — `bench/run.py`, `_run_case.py` and `compare.py` are gone. This branch's copy of `run.py` carried the duckdb arm; main's `conftest.py` already has it (`ENGINE = {'lpspec': None, 'duckdb': 'duckdb'}`), so the deletion is taken and nothing is lost. `report.py` keeps main's `.json` reader and this branch's `--arms`, which is what lets the engine comparison be tabulated at all. **#446 is the one that needed care.** It stopped `_read_back` sorting, on the grounds that every labelling path already produces a label-ascending frame and two of them verify it. That is true of polars. It is not true of a *SQL* relation: two of duckdb's three label paths are views over a cross join, and SQL promises no order at all — so inheriting the unsorted read-back would have attached primal values to the wrong coordinates, silently, on a solve. So the duckdb label registry now asks for the order on the way out (`ORDER BY var_label` / `ORDER BY row` in `_Labels.__getitem__`), where it is paid only by a caller that reads a solution back. `Engine._read_back` states that both engines owe it an ordered frame and by what means each one does. That is the same asymmetry #449 is about, arriving a second time: a change that is free on one engine because its data structure already has the property, and that the other has to buy. 781 passed on polars, 780 passed / 5 skipped on duckdb, ruff + pyrefly (0 errors) + mkdocs --strict clean.

Brings #399 up to `ce0fb3d` — #443, #446, #447, #444 and #448. **#448 replaced the custom runner with a pytest suite.** `bench/run.py`, `_run_case.py` and `compare.py` are gone; this branch's `run.py` carried the duckdb arm and main's `conftest.py` already has it, so the deletion is taken and nothing is lost. `report.py` keeps main's `.json` reader and this branch's `--arms`, without which the engine comparison cannot be tabulated at all. **#446 needed care.** It stopped `_read_back` sorting, because every labelling path already yields a label-ascending frame and two of them verify it. True of polars. Not true of a SQL relation: two of duckdb's three label paths are views over a cross join and SQL promises no order, so inheriting the unsorted read-back would attach primal values to the wrong coordinates — silently, on a solve. duckdb's label registry now asks for the order on the way out, where only a caller reading a solution pays it. That is the third time today a change free on polars had to be bought here: `cols` going positional (#433), the read-back (#446), and the registry. The asymmetry is the branch's most useful finding and is written up in `bench/duckdb-spike.md`. 781 passed on polars, 780 passed / 5 skipped on duckdb, ruff + pyrefly (0 errors) + mkdocs --strict clean.

…ges keep the math they print The pin was the stack head as it stood, which forked before math-spec's alpha.85 — so regenerating every gallery page with that typesetter reverted #446's GitHub math delimiters across 62 files, and rung 10's description to the prose #442 had rewritten. Neither was this change's to make. The stack's own #437 branch had already been rebased onto main; only #443's one commit was left behind it. Rebased onto that head, and the pin follows it. The rebase brings #444 with it: a grouped position carries the walk it counts within rather than naming its pieces, so the predicate reads `partition` straight and rebuilds no walk of its own — and the linopy lane now holds a grouped position to the same one-dimension rule as every other walk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SrEY2ZhFD1YXamRn4FzpMa

FBumann added area:engine Lowering, IR, relational executor, the sink contract perf Wall time, memory, or I/O cost — not a wrong answer engine:polars Specific to the polars engine — the default labels Jul 31, 2026

FBumann merged commit 53f8eb8 into main Jul 31, 2026
7 checks passed

fluxopt-release Bot mentioned this pull request Jul 31, 2026

chore(main): release 0.0.1-alpha.59 #447

Merged

Navigation Menu

Repository navigation

perf(engine): the read-back reads the label order rather than re-imposing it - #446

perf(engine): the read-back reads the label order rather than re-imposing it#446
FBumann merged 1 commit into
mainfrom
perf/polars-readback-order

FBumann commented Jul 31, 2026

Uh oh!

coderabbitai Bot commented Jul 31, 2026

Review limit reached

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

FBumann commented Jul 31, 2026

What

It is not the seconds

Equivalence

Tests

The invariant

Uh oh!

coderabbitai Bot commented Jul 31, 2026

Review limit reached

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant