Repository navigation
perf(matrices): trim the Model.matrices build and drop redundant builds - #1012
Merged
Merged
Conversation
Build cost — v1 vs legacyv1 build peak & time relative to legacy, on this commit — not a comparison against master (that is CodSpeed).
Full table (time + peak, mean)📊 Interactive plots + CSV: download the semantics-report-v1-vs-legacy artifact from this run. Report-only · not a gate · refreshed on every push · obsolete once legacy is dropped. |
Merging this PR will not alter performance
Comparing Footnotes
|
5 tasks done
… once per caller Model.matrices returns a fresh accessor again. Dualization binds it once and expression solutions map labels directly, without building the matrices.
4 of 5 tasks
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.
Closes #1008. Part of #972 (phase 2, PR 5).
Note
The following content was generated by AI.
Changes proposed in this Pull Request
Model.matricesstill builds a freshMatrixAccessoron every access. This PR makes the build cheaper and removes callers that built it more than once.CSRConstraint._scalingdirectly).eliminate_zerosnow runs only on blocks of mutable constraints, because freezing already drops explicit zeros. Indicator blocks are always frozen.M.lb/M.ubin place (np.place→np.where).M = m.matricesonce instead of building it four times.expr.solutionmaps variable labels to solution values directly and no longer builds the whole accessor. An expression that references a removed variable still raises aKeyError.benchmark/benchmark_sparse_export.py(sparse,dense,frozenmodes), so the next phase-2 PRs can show before and after numbers. Thematrices,to_highspyandto_file(lp)phases each build the matrices, by design.An earlier version of this PR also cached
Model.matricesfor frozen models. The cache was dropped after @coroa's review.Benchmark (200 buses x 720 snapshots, 2.09M vars, 1.73M cons; best of 3, interleaved runs against master)
matricesto_highspyto_file(lp)Peak traced memory per phase is unchanged. The dense mode and
to_file(lp)differences are within run-to-run noise.Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.