Skip to content

Support adding AbstractPDMats backed by static arrays#239

Merged
devmotion merged 2 commits into
masterfrom
dmw/adddiag
Jun 25, 2026
Merged

Support adding AbstractPDMats backed by static arrays#239
devmotion merged 2 commits into
masterfrom
dmw/adddiag

Conversation

@devmotion

Copy link
Copy Markdown
Member

Fixes #167.

+ and pdadd of a PDMat/PDiagMat/ScalMat errored when the underlying storage was a static array, e.g.

PDMat(SMatrix{3,3}(...)) + PDiagMat(SVector{3}(...))
# ERROR: MethodError: no method matching _adddiag(::SMatrix{3,3,Float64,9}, ::SVector{3,Float64})

Root cause

_adddiag/_adddiag! were only defined for Union{Matrix, SparseMatrixCSC} and mutated in place — which is impossible for immutable arrays (and copy(::SMatrix) is still immutable). Subtraction already worked because it routes through Base's broadcasting fallback.

Changes

  • Replace the non-mutating _adddiag wrappers with generic AbstractMatrix methods that build the result by construction (a + Diagonal(v), a + v*I, a .+ c.*Diagonal(v)). Allocation-measured to match the previous copy-and-mutate path for dense matrices (1 allocation), stay sparse for sparse inputs, and preserve the static container otherwise.
  • Add _scaleadddiag for the matrix-scaled pdadd paths (c*a + diagonal), keeping an in-place fast path for mutable storage (avoids a second allocation) and falling back to muladd for other matrix types.
  • Keep the in-place _adddiag! — it is still used by the zero-allocation pdadd! and the dense Matrix(b) fallbacks.
  • Vector-taking variants retain @check_argdims to guard against silent broadcast singleton-expansion.

Tests

Added coverage in test/specialarrays.jl for every +/pdadd combination of PDMat{SMatrix} / PDiagMat{SVector} / ScalMat, asserting both numerical correctness and static-type preservation.

🤖 Generated with Claude Code

devmotion and others added 2 commits June 25, 2026 10:42
`+` and `pdadd` of a `PDMat`/`PDiagMat`/`ScalMat` failed when the
underlying storage was a static array (e.g. `SMatrix`/`SVector`,
issue #167), because `_adddiag`/`_adddiag!` were only defined for
`Union{Matrix, SparseMatrixCSC}` and mutated in place — which is
impossible for immutable arrays.

Replace the non-mutating `_adddiag` wrappers with generic
`AbstractMatrix` methods that construct the result (`a + Diagonal(v)`,
`a + v*I`, `a .+ c.*Diagonal(v)`); these match the previous
single-allocation cost for dense matrices, stay sparse for sparse
inputs, and preserve the static container otherwise. Add
`_scaleadddiag` for the matrix-scaled `pdadd` paths (`c*a + diagonal`),
keeping an in-place fast path for mutable storage and falling back to
`muladd` elsewhere. The in-place `_adddiag!` is retained for the truly
in-place `pdadd!`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.84%. Comparing base (6371e0c) to head (4b9bc14).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #239      +/-   ##
==========================================
+ Coverage   94.78%   94.84%   +0.05%     
==========================================
  Files          11       11              
  Lines         805      814       +9     
==========================================
+ Hits          763      772       +9     
  Misses         42       42              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@devmotion devmotion marked this pull request as ready for review June 25, 2026 09:36
@devmotion devmotion requested a review from andreasnoack June 25, 2026 09:37
@devmotion

Copy link
Copy Markdown
Member Author

This is required for #238

@devmotion devmotion merged commit eab0192 into master Jun 25, 2026
18 checks passed
@devmotion devmotion deleted the dmw/adddiag branch June 25, 2026 11:21
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.

Missing Method for StaticArrays

2 participants