Skip to content

fix(engine): a quadratic row keeps its own pair when a row before it has no term - #2027

Merged
FBumann merged 6 commits into
claude/batched-buildfrom
claude/gallant-wright-89731k
Oct 13, 2026
Merged

FBumann merged 6 commits into
claude/batched-buildfrom
claude/gallant-wright-89731k

Conversation

@FBumann

@FBumann FBumann commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Prompt: APply the changes, fixes and refactorings you find reasonable as a stacked PR as you finish the review

Note

The following content was generated by AI.

Stacked on #2022, from a review of it. A block that drops a row with no term now renumbers its quadratic share too, and the omitted-row count adds up. Both bugs are on main as well.

What changed, what was verified, what was not done

Commits

  1. test — two strict xfails that fail on the perf(engine): a model with many variables and constraints builds about 15% faster #2022 head (611d936):
    • test_a_quadratic_row_keeps_its_own_pair_when_a_row_before_it_is_not_built: k * p * p <= 10 over g=[a,b,c] with k only at b, c. Row a has no term and is dropped. The quadratic share kept rows 1, 2 and was then filtered to the renumbered rows 0, 1, so qmatrix was [(1, 1, 1, 1.0)]: row c held b's pair and b held none.
    • test_rows_a_restriction_removed_and_rows_with_no_terms_are_counted_together: one row removed by a restriction and one with no term reported rows_not_built = 1, not 2. _drop_termless_rows used = where _labelled uses .get(name, 0) +.
  2. fix — _drop_termless_rows renumbers the quadratic share with the same map, and adds to the omitted count. The filter in _block goes, because every row the quadratic share holds is in kept. The markers come off, and what was wrong moves into the docstrings.
  3. chore(bench) — bench/profile_build.py also wraps pl.collect_all, and patches labels.frames rather than labels.frame, which the build no longer calls. Before this, the batched passes of perf(engine): a model with many variables and constraints builds about 15% faster #2022 were not timed at all. On network/xs they now show as collect_all of N: … entries under _build_constraints, _build_variables and frames.
  4. chore(engine) — a constraint's labelled block stays an in-memory frame through the build and moves onto the model's rows with _moved, as a variable's does. Before, it was a lazy plan (filter, replace_strict, + offset) that ran again on every dual or activity read.
  5. The changelog line, and a merge of perf(engine): a model with many variables and constraints builds about 15% faster #2022's new head 44423fb, which merged main and reworded perf(engine): a model with many variables and constraints builds about 15% faster #2022's changelog line. The merge touches no file under src/ or tests/. After it, the touched test modules were re-run.

Mutation table, by hand (the tree had to be committed first; restored with git checkout --, __pycache__ dropped before and after):

mutation result
constraint offset _moved(frame, 'row', offset).lazy() → frame.lazy() caught, 5 failed beyond the 2 xpress failures
quadratic share not renumbered caught by the new quadratic test (it was the xfail)
+= back to = on the omitted count caught by the new diagnostics test (it was the xfail)

Gates, in a uv venv on polars 2.0.0 (no pixi in this container): ruff check . and ruff format --check . at 0.16.9, pyrefly check on src/ at 1.3.1 (0 errors), pytest -n auto tests: 5446 passed, 657 skipped, 4 xfailed, and 2 failed. The 2 failures are the [xpress] cases, which fail the same way on 611d936 because the xpress package is not installed here. bench/test_harness.py: 396 passed. 1 failed (test_an_ab_refuses_to_measure_a_checkout_it_did_not_import) because specsolve is on PYTHONPATH here and not installed. docs-test was not run.

Not done, from the same review:

  • Peak memory: _build_constraints holds every constraint's raw term table at once, and run now stacks them all before one collapse. Not measured here; it needs a memory ladder at network/l before a change.
  • The refuse_uncovered guard still has no probe. The door refuses a null parameter value, and a missing coordinate is caught first by refuse_short_constants, so I found no input that reaches it. It may be dead code, which is a decision for the author.
  • The first error now follows pass order, not file order. No change.
  • declared_height, refuse_null_constants and refuse_short_constants still run once per constraint. Batching them needs its own measurement.
  • profile_build attributes attach to setup: the module attribute it patches is not the one the caller imported. This is also the case on main.

Departures from Part 2: none.

🤖 Generated with Claude Code

https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG

…ows two causes removed

Both are strict xfails against the build as it stands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG
…has no term

A block that dropped a row with no term renumbered its rows and its matrix
share but not its quadratic share, so a later row took an earlier row's
pair and lost its own. The omitted count of that block now adds to the rows
a restriction removed rather than replacing them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG
…mory, as a variable's do

The block stays an in-memory frame through the build and moves once with
_moved, so a read-back no longer re-runs the filter, the renumbering and the
offset on every read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG
@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 specsolve | 🛠️ Build #35116420 | 📁 Comparing 54f4059 against latest (af1dac7)

  🔍 Preview build  

2 files changed
± about/benchmarks/index.html
± about/changelog/index.html

@codspeed

codspeed Bot commented Oct 13, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 16.23%

⚡ 9 improved benchmarks
✅ 95 untouched benchmarks
⏩ 307 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ test_emit[network-s-specsolve-lp] 34.5 MB 28.1 MB +22.59%
⚡ test_emit[network-s-specsolve-in-memory-lp] 34.5 MB 28.1 MB +22.58%
⚡ test_window[network-s-specsolve-highs-one] 21.9 MB 18.6 MB +18.23%
⚡ test_window[network-s-specsolve-highs-values] 21.9 MB 18.6 MB +18.23%
⚡ test_emit[network-s-specsolve-highs] 21.9 MB 19.4 MB +13.34%
⚡ test_emit[network-s-specsolve-in-memory-highs] 21.9 MB 19.4 MB +13.34%
⚡ test_window[network-s-specsolve-highs-coefficient] 21.9 MB 19.5 MB +12.78%
⚡ test_window[network-s-specsolve-highs-cold] 21.9 MB 19.5 MB +12.78%
⚡ test_window[network-s-specsolve-highs-shape] 21.9 MB 19.4 MB +12.78%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/gallant-wright-89731k (54f4059) with claude/batched-build (44423fb)

Open in CodSpeed

Footnotes

  1. 307 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@FBumann
FBumann merged commit 54f4059 into claude/batched-build Oct 13, 2026
16 checks passed
@FBumann
FBumann deleted the claude/gallant-wright-89731k branch October 13, 2026 13:08

FBumann commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Note

The following content was generated by AI.

Folded into #2022 as a fast-forward of claude/batched-build onto this branch's head 54f4059, to keep the change in one PR. The fix's changelog line now links #2022. On #2022's branch with pixi, xpress and Gurobi installed, pixi run check passes (5599 passed, the two [xpress] cases included), and so does pixi run test-bench (411 passed).


Generated by Claude Code

FBumann added a commit that referenced this pull request Oct 13, 2026
…has no term (#2028)

> **Prompt:** And what of this is a bug on main that should land there
independently?

> [!NOTE]
> The following content was generated by AI.

A quadratic constraint whose block drops a row with no term now keeps
each quadratic pair on its own row. Before, a later row took an earlier
row's pair and lost its own, and the wrong model solved silently.

<details><summary>Cause, test, gates, what was not done</summary>

**Cause.** `_drop_termless_rows` renumbered `rows` and the matrix share,
but not the quadratic share. `_block` then filtered the quadratic share
against the renumbered rows. So an entry kept its old row number and
landed on the wrong row, or was dropped.

**Test**, written first as a strict xfail and watched to fail on
`af1dac7`:
`test_a_quadratic_row_keeps_its_own_pair_when_a_row_before_it_is_not_built`.
`k * p * p <= 10` over `g=[a,b,c]` with `k` only at `b, c`. On `main`,
`qmatrix` is `[(1, 1, 1, 1.0)]` where `[(0, 1, 1, 1.0), (1, 2, 2, 4.0)]`
is correct. The test also solves with Gurobi against the optimum worked
out by hand.

**Fix.** `_drop_termless_rows` takes the quadratic share and renumbers
it with the same map. The filter in `_block` goes, because every row the
quadratic share holds is in `kept`. `replace_strict` raises if that ever
stops being true.

**Gates**, in a uv venv on polars 2.0.0 (no pixi in this container):
`ruff check .` and `ruff format --check .` at 0.16.9, `pyrefly check` on
`src/` at 1.3.1 (0 errors), `pytest -n auto tests`: 5449 passed, 657
skipped, 4 xfailed, and 2 failed. The 2 failures are the `[xpress]`
cases, which fail on `main` too because the xpress package is not
installed here. `docs-test` was not run.

**Not done.**
- `_drop_termless_rows` has a second caller in `_build_constraint`, for
a constraint with no linear and no quadratic term. That caller now
passes `None` for the quadratic share. No test reaches that branch: the
suite stays green with it replaced by a `raise`. The loader refuses a
constraint with no variable, so the branch looks unreachable. #2022
removes it.
- The omitted-row count of the same function is a separate bug, fixed in
#2029.
- #2022 rewrites this function. When it merges `main`, the resolution
keeps its version and adds this renumbering; #2027 already has it there.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG

---------

Co-authored-by: Claude <noreply@anthropic.com>
FBumann added a commit that referenced this pull request Oct 13, 2026
… are reported together (#2029)

> **Prompt:** And what of this is a bug on main that should land there
independently?

> [!NOTE]
> The following content was generated by AI.

`diagnostics().omissions` now counts every row a constraint did not
build. Before, a constraint that lost rows to a restriction and also
rows with no term reported only the second count.

<details><summary>Cause, test, gates</summary>

**Cause.** `_build_constraint` records the rows a restriction removed
with `.get(name, 0) +`. `_drop_termless_rows` then wrote its own count
with `=`, which replaced the first one.

**Test**, written first as a strict xfail and watched to fail on
`af1dac7`:
`test_rows_a_restriction_removed_and_rows_with_no_terms_are_counted_together`.
`p` is absent at `a`, so the restriction removes that row. `k` has no
value at `b`, so that row has no term. On `main`, `rows_not_built` is 1,
where 2 is correct.

**Fix.** `+=` in `_drop_termless_rows`. The built model does not change;
only the report does.

**Gates**, in a uv venv on polars 2.0.0 (no pixi in this container):
`ruff check .` and `ruff format --check .` at 0.16.9, `pyrefly check` on
`src/` at 1.3.1 (0 errors), `pytest -n auto tests`: 5449 passed, 657
skipped, 4 xfailed, and 2 failed. The 2 failures are the `[xpress]`
cases, which fail on `main` too because the xpress package is not
installed here. `docs-test` was not run.

**Not done.** #2022 rewrites this function. When it merges `main`, the
resolution keeps its version with this `+=`; #2027 already has it there.

</details>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG

---
_Generated by [Claude
Code](https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants