From 5adb969ee6286955eb4e9c093b1b28696d7de718 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Fri, 31 Jul 2026 21:47:22 +0200 Subject: [PATCH] perf(engine): the read-back reads the label order rather than re-imposing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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) --- .../relational/engines/polars/executor.py | 18 +++++++--- .../relational/engines/polars/labels.py | 4 ++- tests/test_relational.py | 33 +++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/lpspec/relational/engines/polars/executor.py b/src/lpspec/relational/engines/polars/executor.py index e24f76eaa..6ed453186 100644 --- a/src/lpspec/relational/engines/polars/executor.py +++ b/src/lpspec/relational/engines/polars/executor.py @@ -382,15 +382,23 @@ def _read_back( ) -> pl.LazyFrame: """One declaration's coordinates in label order, beside its values. + **The order is not re-established here, because it was never lost.** + Every path in :meth:`~lpspec.relational.engines.polars.labels.Labeller.frame` + hands back a label-ascending frame and two of them verify it + (:func:`~lpspec.relational.engines.polars.labels._in_label_order`), so + this reads the ordering rather than imposing it. Sorting again 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. + The slice is attached as a column rather than concatenated as a frame so that a length that does not match the coordinates raises instead of - padding with nulls — the block bookkeeping is an invariant, and the one - way it could go wrong is the one a silently short vector would hide. - :func:`_spanning` has already refused a vector that does not span the - model, so what is left here is the block bookkeeping alone. + padding with nulls — though :func:`_spanning` has already refused a + vector that does not span the model, so what is left here is the block + bookkeeping alone. """ start, height = self._blocks[name] - return labels.sort(label).select(*dims).with_columns(values.slice(start, height)) + return labels.select(*dims).with_columns(values.slice(start, height)) def _primal(self, name: str, values: pl.Series | None) -> pl.DataFrame: return self._solution_frame(name, values).collect(engine='streaming') diff --git a/src/lpspec/relational/engines/polars/labels.py b/src/lpspec/relational/engines/polars/labels.py index 0f5424364..20c2cf882 100644 --- a/src/lpspec/relational/engines/polars/labels.py +++ b/src/lpspec/relational/engines/polars/labels.py @@ -206,7 +206,9 @@ def _in_label_order(frame: pl.DataFrame, label: str) -> pl.DataFrame: (`compiler._coordinate_product`). That is an implementation detail of a dependency, and a label is arithmetic on ordinals rather than a position — so a change there would not corrupt a *label*. It would silently stop the - frame being sorted, and `cols` is read positionally. + frame being sorted, and **two readers take that order on trust**: `cols` is + handed to a solver positionally, and `executor._read_back` reads a solution + back against these coordinates without sorting them again. So the claim is verified. `is_sorted` is a linear scan over a column the frame already holds; the sort behind it is the correctness floor and is diff --git a/tests/test_relational.py b/tests/test_relational.py index e918d6809..685e5eab7 100644 --- a/tests/test_relational.py +++ b/tests/test_relational.py @@ -716,6 +716,39 @@ def test_infinite_bounds_survive_the_handoff(dispatch_data): assert ex.solve().is_ok +def test_a_solution_is_read_back_in_label_order_without_sorting_for_it(): + """The order is produced by the labeller, so the read-back only reads it. + + Seeding the vector with an ``arange`` makes every value its own label, so a + read-back in label order comes out ascending — which is the contract + `sol.primal` states, asserted directly rather than through the sort that + used to establish it. The mask puts the variable on the factored path and + the constraint on the counted one, so neither is ordered by being a scan. + + The plan assertion is the other half: re-imposing the order is not wrong, + it moved a full copy of the coordinates — 681 MB reading `dispatch/l` back + — at the moment the solver's own model is still resident. + """ + model = { + 'dimensions': {'t': {'dtype': 'int', 'values': [0, 1, 2, 3]}, 'g': {'values': ['a', 'b', 'c']}}, + 'parameters': {'cap': {'dims': ['g']}, 'load': {'dims': ['t']}}, + 'variables': {'p': {'foreach': ['t', 'g'], 'where': 'cap > 0', 'bounds': {'lower': 0, 'upper': 'cap'}}}, + 'constraints': {'meet': {'foreach': ['t'], 'where': 'load > 0', 'expression': 'sum(p, over=g) >= load'}}, + 'objectives': {'o': {'sense': 'minimize', 'expression': 'sum(sum(p, over=g), over=t)'}}, + } + sources = { + 'cap': pl.DataFrame({'g': ['a', 'b', 'c'], 'value': [5.0, 0.0, 7.0]}), + 'load': pl.DataFrame({'t': [0, 1, 2, 3], 'value': [1.0, 0.0, 2.0, 3.0]}), + } + with lps.build(model, sources) as ex: + primal = pl.Series('value', np.arange(ex._n_cols, dtype=np.float64)) + dual = pl.Series('value', np.arange(ex._n_rows, dtype=np.float64)) + variable = ex._solution_frame('p', primal) + assert 'SORT' not in variable.explain(optimized=False), 'the labeller already ordered this' + assert variable.collect()['value'].to_list() == list(range(len(primal))), 'primal not in label order' + assert ex._dual('meet', dual)['value'].to_list() == list(range(len(dual))), 'dual not in label order' + + @pytest.mark.parametrize('length', [2, 5], ids=['short', 'long']) def test_a_solver_vector_that_does_not_span_the_model_is_refused(monkeypatch, length): """A wrong length is a different model's answer, not a short one.