Skip to content

Conversation

@DanielVandH
Copy link
Contributor

@DanielVandH DanielVandH commented Nov 19, 2025

This switches from a loop approach over each axis to an approach that instead uses copyto! over each of the diagonal components. Moreover, I switch to PermutedDimsArray instead of permutedims in the bandedrowsdata implementation since permutedims allocates an entire new matrix while PermutedDimsArray is lazy (see, e.g.

julia> A = view(rand(5, 5), 1:2, 1:3)
2×3 view(::Matrix{Float64}, 1:2, 1:3) with eltype Float64:
 0.346535  0.240277  0.96813
 0.888985  0.105294  0.77876

julia> permutedims(A)
3×2 Matrix{Float64}:
 0.346535  0.888985
 0.240277  0.105294
 0.96813   0.77876

julia> PermutedDimsArray(A, (2, 1))
3×2 PermutedDimsArray(view(::Matrix{Float64}, 1:2, 1:3), (2, 1)) with eltype Float64:
 0.346535  0.888985
 0.240277  0.105294
 0.96813   0.77876

)

(I see there are some overloads in src/banded that try and make permutedims lazy for certain types, but for the matrices I was dealing with these overloads were not reaching it. This seemed like a simpler fix for the time being, since I think an alternative requires piracy for my matrices.)

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.99%. Comparing base (62e8561) to head (57d63c9).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #488      +/-   ##
==========================================
+ Coverage   89.80%   89.99%   +0.18%     
==========================================
  Files          27       27              
  Lines        3719     3739      +20     
==========================================
+ Hits         3340     3365      +25     
+ Misses        379      374       -5     

☔ View full report in Codecov by Sentry.
📢 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.

u, ℓ = bandwidths(A)
m, n = size(A)
data_new = similar(bdata, eltype(bdata), size(bdata, 2), n)
for i in axes(bdata, 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be copyto!(data_new, A) and then overload _copyto!(::BandedColumns, ::BandedRows, dest, A) with this definition?

@dlfivefifty dlfivefifty merged commit e1cb2bc into JuliaLinearAlgebra:master Nov 20, 2025
17 checks passed
@DanielVandH DanielVandH deleted the djv/improvedbandedrows branch November 20, 2025 16:20
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