Skip to content

Implement ldiv!() for QRSparse#676

Open
JamesWrigley wants to merge 2 commits intoJuliaSparse:mainfrom
JamesWrigley:spqr-ldiv
Open

Implement ldiv!() for QRSparse#676
JamesWrigley wants to merge 2 commits intoJuliaSparse:mainfrom
JamesWrigley:spqr-ldiv

Conversation

@JamesWrigley
Copy link
Contributor

I originally was going to implement a separate workspace like how FastLapackInterface.jl does it, but seeing as there's already a QRSparse type that kinda functions as a workspace already I decided it made more sense to re-use that. Reusing QRSparse is also consistent with the way that UmfpackLU holds a workspace. Unlike UmfpackLU I did not add a lock because for the example in the workspace reuse test it added a ~15% overhead. Instead the docstring clearly warns that using QRSparse with ldiv!() is not threadsafe and tells users to make an explicit copy.

Preallocating W removed most of the allocations, and taking a view of F.R removed the other big one. Benchmarks:

using SparseArrays, LinearAlgebra

m, n = 100, 10
nn = 100

A = sparse([1:n; rand(1:m, nn - n)], [1:n; rand(1:n, nn - n)], randn(nn), m, n)
F = qr(A)
b = randn(m)
x = zeros(n);

# Before
julia> @benchmark $F \ $b                                                                                                                                                                                                                      
BenchmarkTools.Trial: 10000 samples with 17 evaluations per sample.                                                                                                                                                                            
 Range (min … max):  995.294 ns … 203.601 μs  ┊ GC (min … max): 0.00% … 98.64%                                                                                                                                                                 
 Time  (median):       1.245 μs               ┊ GC (median):    0.00%                                                                                                                                                                          
 Time  (mean ± σ):     1.324 μs ±   3.433 μs  ┊ GC (mean ± σ):  6.15% ±  2.59%                                                                                                                                                                 
                                                                                                                                                                                                                                               
  ▁▅▅▃▁   ▁▂▃▃▂  ▄▄▆█▇▆▅▄▂▁  ▁▁▁▁▂▁                             ▂                                                                                                                                                                              
  ██████▇██████████████████▇███████▇▆▃▅▄▄▄▂▂▃▄▅▂▅▅▇▇▆▇██▇▇███▇▆ █                                                                                                                                                                              
  995 ns        Histogram: log(frequency) by time       1.81 μs <                                                                                                                                                                              

 Memory estimate: 2.14 KiB, allocs estimate: 14.

# After
julia> @benchmark ldiv!($x, $F, $b)
BenchmarkTools.Trial: 10000 samples with 15 evaluations per sample.
 Range (min … max):  957.467 ns …   3.822 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):       1.154 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):     1.148 μs ± 128.500 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

   ▃             █  ▁  ▅                                         
  ▄█▂▂▁▁▁▁▁▁▁▁▁▁▂█▅▃█▃▂█▆▂▆▄▁▂▂▁▁▂▁▁▁▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  957 ns           Histogram: frequency by time         1.61 μs <

 Memory estimate: 96 bytes, allocs estimate: 2.

Fixes #242. Written with help from Claude 🤖

@codecov
Copy link

codecov bot commented Feb 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.23%. Comparing base (4500d86) to head (5a7c482).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #676      +/-   ##
==========================================
+ Coverage   84.19%   84.23%   +0.04%     
==========================================
  Files          12       12              
  Lines        9313     9339      +26     
==========================================
+ Hits         7841     7867      +26     
  Misses       1472     1472              

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

@rayegun
Copy link
Member

rayegun commented Feb 16, 2026

The asymmetry between UMFPACK and SPQR here on thread safety is concerning, but otherwise LGTM.

A 100 x 10 sparse matrix is incredibly small so I'm not sure the 15% hit here is really representative.

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 ldiv! overload on sparse QR

2 participants