test(bench): recover the duckdb arm and the marginal-cost pass - #240
Merged
Conversation
Both were written against #231's branch and kept being committed there after that PR was squash-merged, so none of it reached `polars-engine`: `--duckdb-root`, `--builds`, the `loop` mode, `FOREIGN_ARMS` and the marginal cost table were all absent. This lands them on the current base. **The duckdb arm** runs the engine this branch replaces from its own checkout, its own interpreter and its own `bench/_run_case.py`, interleaved with the other two against one parquet cache, with the parity gate spanning all three. **The loop pass** answers the second question the timings cannot: the harness spawns one process per measurement, so every timing is a *first* build, and each lane does lazy first-call work — ~180 ms eager, ~21 ms duckdb, ~4 ms here. `first` is what a caller pays who builds one model and solves it; `steady` is what a rolling horizon pays for every model after. Both are real, so both are reported. It measures build *and* hand-off, not build alone: linopy defers ~82% of its work to whatever consumes the model, so a build-only loop compares our finished matrix against its placeholder. `bench/README.md` keeps #233's structure — narrow runs go to `/tmp` so they cannot clobber the published file — with the three-engine invocation added to the publishing block, which is what it is. No numbers are published here: the base has moved under them twice (#233, #234) and again with #238, and the linopy pin is now the v1-semantics build rather than 0.9.0. The ladder is re-run separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both were written against #231's branch and kept being committed there after that PR was squash-merged, so none of it reached
polars-engine.--duckdb-root,--builds, theloopmode,FOREIGN_ARMSand the marginal-cost table were all absent. My mistake, and it is why #189's headline numbers cannot be refreshed yet.The duckdb arm runs the engine this branch replaces from its own checkout, its own interpreter and its own
bench/_run_case.py— nothing is reimplemented here — interleaved with the other two against one shared parquet cache, with the parity gate spanning all three.The loop pass answers the question the timings cannot. The harness spawns one process per measurement, so every timing is a first build, and each lane does lazy first-call work: ~180 ms eager, ~21 ms duckdb, ~4 ms here.
firstis what a caller pays who builds one model and solves it;steadyis what a rolling horizon pays for every model after. Both are real.It measures build and hand-off rather than build alone — linopy defers ~82% of its work to whatever consumes the model, so a build-only loop compares our finished matrix against its placeholder. I published a wrong claim from exactly that flaw and it is fixed here.
Smoke-tested on the current base:
bench/README.mdkeeps #233's structure — narrow runs go to/tmpso they cannot clobber the published file — with the three-engine invocation added to the publishing block.No numbers are published here. The base has moved twice under them (#233, #234) and again with #238, and the linopy pin is now the v1-semantics build rather than 0.9.0. The ladder is re-run separately, against that linopy, and
docs/benchmarks.mdand #189 are updated from it.417 passed / 1 xfailed, ruff and pyrefly clean.
🤖 Generated with Claude Code