-
Notifications
You must be signed in to change notification settings - Fork 4
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
Chebyshev rangefinder #4
Chebyshev rangefinder #4
Conversation
src/rangefinder.jl
Outdated
for i in 2:q | ||
Yi = (4 / ν) * A * A' * Yi_1 - 2 * Yi_2 - Yi_2 | ||
Y = hcat(Y, Yi) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if repeated use of hcat
is bad practice in Julia (as opposed to e.g. allocating all the required space in a big matrix right at the start).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where possible, I would allocate at the beginning of the call since Y changes size and then overwrite columns.
Also use non-allocating matrix operations where possible (see https://docs.julialang.org/en/v1/stdlib/LinearAlgebra/#Low-level-matrix-operations).
For example of doing the multiply, see
# Y = A*A'*Y |
src/rangefinder.jl
Outdated
isnothing(Ω) && (Ω = GaussianTestMatrix(m, r)) | ||
|
||
Y0 = qr(Ω.Ω).Q |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this step make the Chebyshev rangefinder incompatible with SSFTTestMatrix
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Matrix(Ω) instead of Ω.Ω
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! See commented notes, but this looks almost done
src/rangefinder.jl
Outdated
isnothing(Ω) && (Ω = GaussianTestMatrix(m, r)) | ||
|
||
Y0 = qr(Ω.Ω).Q |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Matrix(Ω) instead of Ω.Ω
src/rangefinder.jl
Outdated
for i in 2:q | ||
Yi = (4 / ν) * A * A' * Yi_1 - 2 * Yi_2 - Yi_2 | ||
Y = hcat(Y, Yi) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where possible, I would allocate at the beginning of the call since Y changes size and then overwrite columns.
Also use non-allocating matrix operations where possible (see https://docs.julialang.org/en/v1/stdlib/LinearAlgebra/#Low-level-matrix-operations).
For example of doing the multiply, see
# Y = A*A'*Y |
src/rangefinder.jl
Outdated
Yi_1 = Y1 | ||
for i in 2:q | ||
Yi = (4 / ν) * A * A' * Yi_1 - 2 * Yi_2 - Yi_2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be - 2 * Yi_1
…mizedPreconditioners.jl into chebyshev_rangefinder
No description provided.