Skip to content

fix(engine): the rows a restriction removed and the rows with no term are reported together - #2029

Merged
FBumann merged 4 commits into
mainfrom
fix/omitted-rows-counted
Oct 13, 2026
Merged

FBumann merged 4 commits into
mainfrom
fix/omitted-rows-counted

Conversation

@FBumann

@FBumann FBumann commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

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.

Cause, test, gates

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.

🤖 Generated with Claude Code

https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG


Generated by Claude Code

…missing term

A strict xfail against the build as it stands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013DSGmeGQfUxE4gyNQy94qG
… are reported together

A constraint that lost rows to a restriction and rows with no term reported
only the second count, because it replaced the first.

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

read-the-docs-community Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 specsolve | 🛠️ Build #35118306 | 📁 Comparing 32ca3d6 against latest (af1dac7)

  🔍 Preview build  

1 file changed
± about/changelog/index.html

@FBumann
FBumann enabled auto-merge (squash) October 13, 2026 13:21
@codspeed

codspeed Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 12.86%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 7 regressed benchmarks
✅ 97 untouched benchmarks
⏩ 307 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_emit[network-s-specsolve-in-memory-lp] 28.2 MB 34.5 MB -18.2%
❌ test_emit[network-s-specsolve-lp] 28.2 MB 34.5 MB -18.2%
❌ test_emit[network-s-specsolve-highs] 19.5 MB 21.8 MB -10.7%
❌ test_emit[network-s-specsolve-in-memory-highs] 19.5 MB 21.8 MB -10.69%
❌ test_window[network-s-specsolve-highs-cold] 19.6 MB 21.9 MB -10.65%
❌ test_window[network-s-specsolve-highs-coefficient] 19.6 MB 21.9 MB -10.54%
❌ test_window[network-s-specsolve-highs-shape] 19.5 MB 21.8 MB -10.54%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing fix/omitted-rows-counted (32ca3d6) with main (b646d61)2

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

  2. No successful run was found on main (454e6de) during the generation of this report, so b646d61 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

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
FBumann disabled auto-merge October 13, 2026 13:36
@FBumann
FBumann merged commit 1bcc271 into main Oct 13, 2026
14 checks passed
@FBumann
FBumann deleted the fix/omitted-rows-counted branch October 13, 2026 13:38
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