Skip to content

Support FillArrays.OffDiagonal and remove sublayout piracy for DiagonalLayout (require ArrayLayouts v1.15) - #511

Merged
dlfivefifty merged 7 commits into
masterfrom
dl/offdiagonal
Oct 15, 2026
Merged

dlfivefifty merged 7 commits into
masterfrom
dl/offdiagonal

Conversation

@dlfivefifty

Copy link
Copy Markdown
Member

Depends on JuliaLinearAlgebra/ArrayLayouts.jl#285, which adds OffDiagonalLayout and will be released as ArrayLayouts v1.15. CI will fail until that version is registered.

Changes

  • Require ArrayLayouts v1.15 and FillArrays v1.18 (which adds FillArrays.OffDiagonal). Bump to v1.14.0.
  • Remove the pirated sublayout(::DiagonalLayout, ...) methods. ArrayLayouts v1.15 defines sublayout(::DiagonalLayout, ::Type{<:Tuple{AbstractUnitRange{<:Integer},AbstractUnitRange{<:Integer}}}) returning OffDiagonalLayout. That method has to be broader than the pirated one here, which takes AbstractUnitRange{Int}, so that loading BandedMatrices v1.13 with ArrayLayouts v1.15 does not trigger method overwriting.
  • Add banded support for OffDiagonalLayout, which covers OffDiagonal, its transpose and adjoint, and views of Diagonal/OffDiagonal by unit ranges:
    • bandwidths(::OffDiagonalLayout, A) = (-k, k)
    • inbands_getindex and inbands_setindex! for OffDiagonal
    • BandedMatrix(A) copies the band directly via offdiagonaldata.

Behaviour changes

  • view(D::Diagonal, kr, jr) with unit ranges now has OffDiagonalLayout instead of BandedColumns, and layout_getindex(D, kr, jr) returns a FillArrays.OffDiagonal instead of a BandedMatrix. The same applies to Eye. The tests are updated to match.
  • Views of a Diagonal by non-unit-range indices (e.g. view(D, :, 1) or view(D, [1,2], 1:2)) now get ArrayLayouts' default sublayout rather than a BandedColumns-derived one.

🤖 Generated with Claude Code

…alLayout

Require ArrayLayouts v1.15, which adds OffDiagonalLayout and returns it for
views of a DiagonalLayout matrix by unit ranges. Remove the pirated
sublayout(::DiagonalLayout, ...) methods, which would otherwise overwrite
ArrayLayouts' method, and add bandwidths, inbands_getindex/inbands_setindex!
and BandedMatrix conversion for OffDiagonalLayout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.74%. Comparing base (70c629c) to head (39b9968).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/interfaceimpl.jl 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #511      +/-   ##
==========================================
+ Coverage   90.69%   90.74%   +0.05%     
==========================================
  Files          27       27              
  Lines        3784     3805      +21     
==========================================
+ Hits         3432     3453      +21     
  Misses        352      352              

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

Keeps BandedMatrix .+ OffDiagonal banded once OffDiagonal has its own
broadcast style (JuliaArrays/FillArrays.jl#458).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dlfivefifty

Copy link
Copy Markdown
Member Author

Added :OffDiagonal to the StructuredMatrixStyle/BandedStyle rules so that BandedMatrix .+ OffDiagonal stays a BandedMatrix once JuliaArrays/FillArrays.jl#458 gives OffDiagonal the style StructuredMatrixStyle{OffDiagonal}. Tests pass with both FillArrays v1.18.0 and #458.

dlfivefifty added a commit to JuliaArrays/FillArrays.jl that referenced this pull request Oct 13, 2026
`OffDiagonal` now has `BroadcastStyle`
`LinearAlgebra.StructuredMatrixStyle{OffDiagonal}`, so zero-preserving
broadcasts keep the single-band structure:

```julia
julia> sin.(Eye(5)[2:end,:])
4×5 FillArrays.OffDiagonal{Float64, Fill{Float64, 1, Tuple{Base.OneTo{Int64}}}, …}
```

- Zero preservation is detected with LinearAlgebra's `fzeropreserving`,
as for `Diagonal` etc. (`fzero(::OffDiagonal)` is defined like for
`RectDiagonal`).
- The result is an `OffDiagonal` only if every `OffDiagonal` argument
lies on the same band and has the same axes as the result. Otherwise
(different bands, a 1×1 `OffDiagonal` broadcast out, non-scalar
non-`OffDiagonal` arguments, or non-zero-preserving functions such as
`cos` or `.+ 1`) it falls back to a dense `Matrix`.
- `copy` broadcasts over the bands themselves, so lazy bands stay lazy:
`sin.(Eye(5)[2:end,:]) ≡ OffDiagonal(Fill(sin(1.0), 4), 1, 4, 5)`.
- In-place `dest .= f.(A)` with `dest::OffDiagonal` on the same band
only writes the band. Other cases fall back to the generic `copyto!`,
which throws when writing a non-zero entry off the band.
- `structured_broadcast_alloc` is defined so that `similar(bc, T)` also
gives an `OffDiagonal`.

Tests cover zero-preserving functions (`sin`, `abs`, scaling, `.^2`, `A
.+ A`, `A .* A`, `round.(Int, A)`, nested broadcasts), dense fallbacks,
mismatched bands, in-place broadcasting and lazy bands. They pass on
Julia 1.10 and 1.13. On 1.11 the only failure is Aqua's `ambiguities`
check being an unexpected pass, which also happens on `master`.

**Downstream:** with this change `BandedMatrix .+ OffDiagonal` would
become a dense `Matrix` instead of a `BandedMatrix`, as BandedMatrices
only defines `BroadcastStyle` rules between `BandedStyle` and the
`StructuredMatrixStyle`s of specific types. Adding `:OffDiagonal` to
that list in `src/interfaceimpl.jl` (e.g. in
JuliaLinearAlgebra/BandedMatrices.jl#511) restores a `BandedMatrix`
result. ArrayLayouts, LazyArrays and BandedMatrices (#511 plus that
one-line change) test suites pass with this PR.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@dlfivefifty
dlfivefifty merged commit 07eeae5 into master Oct 15, 2026
31 of 37 checks passed
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.

1 participant