From fdc6e6c91cf340b440a7574d6acd803cc17af816 Mon Sep 17 00:00:00 2001 From: FBumann <117816358+FBumann@users.noreply.github.com> Date: Fri, 31 Jul 2026 20:14:24 +0200 Subject: [PATCH] fix(bench): compare like with like, and add a gurobi arm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `--sinks gurobi`: lpspec through `build_gurobi`, linopy through `to_gurobipy`, neither starting the search. Opt-in, because it needs the [gurobi] extra where the other two need nothing a contributor lacks. The size-limited wheel licence is no obstacle — it caps `optimize()`, which this harness never calls. Adding it surfaced a comparability bug in the arm that already shipped. **linopy names every variable and constraint by default and neither of our solver sinks names anything**, so `to_highspy()` was being timed doing work our arm does not do — and it is not a rounding error: naming is 82% of linopy's HiGHS hand-off (0.11s against 0.02s at 200k variables) and 35% of its Gurobi one. Both seams now pass `set_names=False`, so the two arms end holding the same artifact. That correction runs against us: published `emit` numbers for the highs sink will narrow when the ladder is next regenerated, and on `dispatch s` the two arms now emit in the same time. The claim that survives is the one this harness exists for — `build`, where lpspec is 0.01s against 0.17s. Co-Authored-By: Claude Opus 5 (1M context) --- bench/README.md | 26 ++++++++++++++++++++------ bench/_run_case.py | 37 +++++++++++++++++++++++++------------ bench/report.py | 13 ++++++++++--- bench/run.py | 10 ++++++---- 4 files changed, 61 insertions(+), 25 deletions(-) diff --git a/bench/README.md b/bench/README.md index 245b6ab5f..233111567 100644 --- a/bench/README.md +++ b/bench/README.md @@ -42,12 +42,26 @@ is the hard part. **Peak RSS and wall time**, per phase, for the same model built two ways: -| | `lp` | `highs` | -|---|---|---| -| `lpspec` | `lps.build(...)` then `ex.write(...)` | `lps.build(...)` then `build_highs(...)` | -| `linopy` | `lpspec.linopy.build(...)` then `Model.to_file(io_api='lp-polars')` | `lpspec.linopy.build(...)` then `Model.to_highspy()` | - -**The `highs` sink stops at the handoff — `run()` is never called.** That is the +| | `lp` | `highs` | `gurobi` | +|---|---|---|---| +| `lpspec` | `lps.build(...)` then `ex.write(...)` | `lps.build(...)` then `build_highs(...)` | `lps.build(...)` then `build_gurobi(...)` | +| `linopy` | `lpspec.linopy.build(...)` then `Model.to_file(io_api='lp-polars')` | … then `Model.to_highspy(set_names=False)` | … then `Model.to_gurobipy(set_names=False)` | + +`gurobi` is opt-in (`--sinks gurobi`): it needs the `[gurobi]` extra, where the +other two need nothing a contributor does not already have. It measures the +same seam — `build_gurobi` never calls `optimize()`, `to_gurobipy` never +returns a solved model. + +**`set_names=False` on the linopy side is load-bearing.** linopy names every +variable and constraint by default and neither of our solver sinks names +anything, so the default call would time a feature only one arm's model +carries. It is not a rounding error: naming is **82% of linopy's HiGHS +hand-off** (0.11s against 0.02s at 200k variables) and 35% of its Gurobi one. +The two arms have to end holding the same artifact or the number means +nothing — and the correction runs against us, which is the direction an +honest harness should err. + +**The solver sinks stop at the handoff — `run()` / `optimize()` is never called.** That is the whole discipline of it. HiGHS's simplex is the same work whoever filled the model, so including it would swamp the phase this harness exists to measure and publish a number about HiGHS under our name. Both arms end holding a populated diff --git a/bench/_run_case.py b/bench/_run_case.py index 11fec376f..f8ea1488e 100644 --- a/bench/_run_case.py +++ b/bench/_run_case.py @@ -60,7 +60,11 @@ def _run_lpspec( case: Case, paths: dict[str, str], lp: Path, phases: Phases, opts: argparse.Namespace ) -> dict[str, Any]: import lpspec as lps - from lpspec.relational.sinks.solvers.highs import build_highs + + if opts.sink == 'gurobi': + from lpspec.relational.sinks.solvers.gurobi import build_gurobi as build_solver + else: + from lpspec.relational.sinks.solvers.highs import build_highs as build_solver # The parameter/dimension split is harness bookkeeping — it re-parses the # YAML only because the runner, not lpspec, decides which parquet file is @@ -74,12 +78,13 @@ def _run_lpspec( if opts.sink == 'lp': ex.write(lp) else: - # the hand-off, and nothing past it. `run()` is never called: the - # simplex is the same work whoever filled the model, so timing it would - # swamp the phase this harness exists to measure and publish a number - # about HiGHS under our name. `build_highs` is the seam that stops - # there — linopy's `to_highspy()` is the same seam on the other side. - _handle = build_highs(ex._tables()) + # the hand-off, and nothing past it. `run()` / `optimize()` is never + # called: the search is the same work whoever filled the model, so + # timing it would swamp the phase this harness exists to measure and + # publish a number about the solver under our name. `build_highs` and + # `build_gurobi` are the seams that stop there — `to_highspy()` and + # `to_gurobipy()` are the same seams on linopy's side. + _handle = build_solver(ex._tables()) phases.mark('emit') # Read after the clock stops: counts are the harness's, not the engine's. @@ -116,9 +121,17 @@ def _run_linopy( # stdout m.to_file(lp, io_api=opts.io_api, progress=False) else: - # the same seam as the lpspec arm: both end holding a populated - # `highspy.Highs` with `run()` never called - _handle = m.to_highspy() + # The same seam as the lpspec arm: both end holding a populated solver + # model with the search never started. + # + # `set_names=False` is what makes them the same artifact. linopy names + # every variable and constraint by default and this sink names none, so + # the default call would time a *feature* neither arm's model needs to + # be solved — and it is not a rounding error: naming is 82% of the + # HiGHS hand-off (0.11s against 0.02s at 200k variables) and 35% of the + # Gurobi one. Leaving it in would report our own lead as several times + # what it is. + _handle = m.to_gurobipy(set_names=False) if opts.sink == 'gurobi' else m.to_highspy(set_names=False) phases.mark('emit') counts = {'columns': int(m.nvars), 'rows': int(m.ncons), 'nonzeros': None} return {'phases': phases.times, 'counts': counts} @@ -225,8 +238,8 @@ def main(argv: list[str] | None = None) -> int: ap.add_argument( '--sink', default='lp', - choices=('lp', 'highs'), - help='where the built model goes: an LP file, or straight into HiGHS', + choices=('lp', 'highs', 'gurobi'), + help='where the built model goes: an LP file, or straight into a solver', ) ap.add_argument('--cache', type=Path, default=None) opts = ap.parse_args(argv) diff --git a/bench/report.py b/bench/report.py index 8c9b422e0..0f3fae9f2 100644 --- a/bench/report.py +++ b/bench/report.py @@ -99,9 +99,16 @@ def sizes_of(case: str, rows: dict[Key, Row], sink: str = 'lp', *, density: bool 'lp': 'lpspec writes the LP file, linopy through its `lp-polars` writer.', 'highs': ( 'Both arms end holding a populated `highspy.Highs` with `run()` never ' - 'called: lpspec through `build_highs`, linopy through `to_highspy()`. ' - 'The simplex is the same work whoever filled the model, so timing it ' - 'would say nothing about the lane that filled it.' + 'called: lpspec through `build_highs`, linopy through ' + '`to_highspy(set_names=False)`. The simplex is the same work whoever ' + 'filled the model, so timing it would say nothing about the lane that ' + 'filled it.' + ), + 'gurobi': ( + 'Both arms end holding a populated `gurobipy.Model` with `optimize()` ' + 'never called: lpspec through `build_gurobi`, linopy through ' + '`to_gurobipy(set_names=False)`. Opt-in — it needs the `[gurobi]` ' + 'extra — and the same discipline as the `highs` sink.' ), } diff --git a/bench/run.py b/bench/run.py index 39b6b579c..502ca1b15 100644 --- a/bench/run.py +++ b/bench/run.py @@ -277,10 +277,12 @@ def main(argv: list[str] | None = None) -> int: '--sinks', nargs='+', default=['lp', 'highs'], - choices=('lp', 'highs'), - help='where each built model goes. Both by default: the LP file is the ' - 'artifact fewest callers want, and it is not the same comparison — ' - "HiGHS's own model is resident in both arms and narrows the gap.", + choices=('lp', 'highs', 'gurobi'), + help='where each built model goes. `lp` and `highs` by default: the LP ' + 'file is the artifact fewest callers want, and it is not the same ' + "comparison — HiGHS's own model is resident in both arms and narrows " + 'the gap. `gurobi` is opt-in because it needs the [gurobi] extra, and ' + 'it is measured against linopy the same way, through `to_gurobipy()`.', ) ap.add_argument('--timeout', type=float, default=3600.0) ap.add_argument('--skip-gate', action='store_true', help='time without checking the arms agree')