perf: drop explicit zeros from the constraint matrix (#814)#815
Draft
FBumann wants to merge 1 commit into
Draft
Conversation
Expressions that broadcast against a dense coordinate store one coefficient per pair, most of them structurally zero. Those explicit zeros were carried all the way into `matrices.A` and thus into every solver handoff. `highspy.Highs.addRows` (and the other direct backends' matrix loaders) scale with *stored* nnz, so the handoff spent most of its work describing zeros — e.g. sparse_network(250) stored 1.5M entries for 18k structural nonzeros (98.8% zeros). Prune them once, centrally, in `_stack` via `eliminate_zeros()`, so `A` and `indicator_A` — and hence HiGHS, gurobi, xpress, copt, mosek and the LP/MPS writers — all hand the solver only structural nonzeros. A zero coefficient never changes a constraint, so this is mathematically identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merging this PR will improve performance by 29.51%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
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.
This opportunity was discovered while exploring the regression introduced by highspy==1.15
Depeneding on the model, this is quite a impressive improvement. But id only merge it after someone tested it with real world models and maybe also hardened tests with other solvers.
Closes #814.
Note
The following was generated by AI.
What & why
Expressions that broadcast against a dense coordinate (e.g. a
bus × lineincidence block) store one coefficient per coordinate pair, most of them
structurally zero. Those explicit zeros were carried into
model.matrices.Aand handed to every solver.
highspy.Highs.addRows— and the other directbackends' matrix loaders — scale with stored nnz, so the handoff spent most
of its work describing zeros (
sparse_network(250): 1.5M stored entries for18k structural nonzeros, 98.8% zeros).
Change
Prune explicit zeros once, centrally, in
matrices._stackviaeliminate_zeros(). This covers bothAandindicator_A, so allconsumers benefit — HiGHS, gurobi, xpress, copt, mosek and the LP/MPS
writers — not just HiGHS. A zero coefficient never changes a constraint, so
the result is mathematically identical; row count (and thus
clabelsalignment) is preserved, and the
dual/solpaths recompute independently ofA.Chose the central
matriceslayer over the per-backend_build_solver_modelpatch from the issue precisely so every backend and writer shares the win.
Tests
test_matrices_drops_explicit_zerosasserts a dense-broadcast constraintyields only structural nonzeros (stored-vs-structural nnz regression).
test_matrices.pyandtest_solvers.pypass unchanged.Expected addRows speedup (from #813 standalone repro)
Side effect: makes linopy insensitive to the #813 highspy
addRowsregression.🤖 Generated with Claude Code