Skip to content

perf(matrices): keep broadcast constraint fields lazy when assembling matrices - #1117

Open
FabianHofmann wants to merge 4 commits into
masterfrom
perf/matrices-lazy-fields
Open

FabianHofmann wants to merge 4 commits into
masterfrom
perf/matrices-lazy-fields

Conversation

@FabianHofmann

Copy link
Copy Markdown
Collaborator

Changes proposed in this Pull Request

Note

The following content was generated by AI.

Model.matrices used ravel()[active] on each constraint's sign, rhs and scaling in _row_block. When those fields are stored lazily as stride-0 np.broadcast_to views (a scalar sign or scaling), this builds full-size copies. It also copies when every row is active.

Changes in linopy/matrices.py:

  • A broadcast scalar field (all strides zero, size > 0) stays a 0-d array through _RowBlock (sign, rhs, scaling).
  • When all rows of a constraint are active, the field is taken as a raveled view (reshape(-1)) and the boolean-mask copy is skipped.
  • _non_unit handles 0-d scaling. _stack multiplies by a 0-d row scaling directly instead of calling repeat_per_row, and assigns 0-d rhs/sign by broadcasting.
  • The con_scaling concatenation broadcasts 0-d blocks to their row count.

The public Matrices fields and dtypes stay the same (sense is still a U1 array, and b and con_scaling are still full float arrays), so this is not a breaking change.

Tests in test/test_matrices.py:

  • test_row_block_keeps_broadcast_fields_lazy: scalar sign and scaling stay 0-d, with all rows active and with some masked.
  • test_matrices_scalar_and_array_fields (dense and sparse): covers scalar sign/scaling, masked rows, mixed signs and per-cell scaling. It compares A, b, sense, con_scaling and clabels against explicit expected arrays. The same expected arrays also pass against the unmodified master implementation.
Benchmark

SciGrid-DE tiled to 1.79M vars / 4.29M rows / 7.18M nnz, cloud VM, best of 5:

before after
matrix assembly (best) 1.01 s 0.63 s
matrix assembly (median) 1.08 s 0.81 s

In line profiles, con.sign.values.ravel()[active].astype("U1") alone took about 140 ms before this change.

Checklist

  • AI-generated content is marked (see AGENTS.md).
  • Code changes are sufficiently documented; i.e. new functions contain docstrings and further explanations may be given in doc.
  • Unit tests for new features were added (if applicable).
  • A note for the release notes doc/release_notes.rst of the upcoming release is included.
  • I consent to the release of this PR's code under the MIT license.

Generated by Claude Code

… matrices

`_row_block` used `ravel()[active]` on sign, rhs and scaling, which
materialises full-size copies of stride-0 broadcast views and copies even
when every row is active. Broadcast scalar fields now stay 0-d through
`_RowBlock`, all-active fields are taken as raveled views, and `_stack`,
`_non_unit` and the `con_scaling` concatenation handle 0-d blocks. The
public `Matrices` arrays are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F4nx8GJ8mSNvkTagja9dMj
@codspeed

codspeed Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 181 untouched benchmarks
⏩ 181 skipped benchmarks1


Comparing perf/matrices-lazy-fields (b91d191) with master (6c832f2)

Open in CodSpeed

Footnotes

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

Merge _uniform into _active with a row indexer, drop the 0-d special
cases in _non_unit and sign, document 0-d _RowBlock fields, and refocus
the test on fully masked and dimensionless constraints.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F4nx8KhK8QzSzHhpYKvcSx
@FabianHofmann
FabianHofmann marked this pull request as ready for review November 3, 2026 14:38

This branch has not been deployed

No deployments
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