Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added in-place functions 'qraddcol!' and 'qrdelcol!' #10

Merged
merged 6 commits into from
Jun 25, 2024
Merged

Conversation

nabw
Copy link
Contributor

@nabw nabw commented Jun 25, 2024

Thanks for the disposition!

The idea here is that the A and R matrices are allocated from the beginning, so only the temporary vectors in the qraddcol operations are used. I tried to minimize allocation in general to obtain a performant implementation. Because of this, when you add a new column, you must tell the function how full your R matrix is.

So here are the modifications:

  • I added a 'qrdelcol!' function that deletes columns from both A and R. This is mostly untouched from the existing implementation.
  • I added a 'qraddcol!' function that appends a column to A and R. It needs to know how many diagonal elements R currently has.
  • Within the column addition routing, one needs to solve triangular systems. I have also added two auxiliary functions 'solveR!' and 'solveRT! that do forward/backward substitution to solve the associated systems with a non-full R. These are tested as well.

I tried the following code:

using QRupdate
using LinearAlgebra
using TimerOutputs

m, n = 10000, 100
A = randn(m, 0)
R = Array{Float64, 2}(undef, 0, 0)
Rin = zeros(m,m)
Ain = zeros(m, m)
for i in 1:n
    println("======", i)
    a = randn(m)
    @timeit "dynamic" global R = qraddcol(A, R, a)
    @timeit "dynamic" global A = [A a]
    @timeit "in-place" qraddcol!(Ain, Rin, a, i-1)
end
print_timer()

and got the following output:

───────────────────────────
Time Allocations
─────────────────────── ────
Tot / % measured: 8.11s / 60.9% 2.04GiB / 25.3%

Section ncalls time %tot avg alloc %tot avg
────────────────────────────────────
dynamic 200 3.71s 75.1% 18.6ms 512MiB 96.9% 2.56MiB
in-place 100 1.23s 24.9% 12.3ms 16.3MiB 3.1% 167KiB
────────────────────────────────────

So, roughly a 60% time reduction and considerable savings in terms of memory allocations.

@mpf mpf merged commit 66b7df2 into mpf:master Jun 25, 2024
3 of 15 checks passed
@mpf
Copy link
Owner

mpf commented Jun 25, 2024

Thank you @nabw! The improvement is significant! 🎉

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