[go: nahoru, domu]

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

UniformScaling: copyto! banded matrices #48445

Merged
merged 3 commits into from
Feb 11, 2023
Merged

Conversation

jishnub
Copy link
Contributor
@jishnub jishnub commented Jan 29, 2023

The fallback method fill!s the array with zeros before copying to the diagonal. This seems redundant for Diagonal and almost-diagonal banded matrices, as this involves two passes over the diagonal. This also resolves issues like JuliaArrays/FillArrays.jl#206, where the diagonal must necessarily be non-zero, and an attempt to set it to zero will lead to an error.

Removing the redundancy cuts runtime by half, as expected:
On nightly v"1.10.0-DEV.450"

julia> using LinearAlgebra,BenchmarkTools

julia> D = Diagonal(rand(10000));

julia> @btime copyto!($D, I);
  3.329 μs (0 allocations: 0 bytes)

This PR

julia> @btime copyto!($D, I);
  1.659 μs (0 allocations: 0 bytes)

I've added a simplified FillArray test-helper module, as I imagine this might be useful elsewhere as well.

Something similar might also be necessary for UnitLowerTriangular and UnitUpperTriangular, but that can be a separate PR

@jishnub jishnub requested a review from N5N3 January 29, 2023 10:40
stdlib/LinearAlgebra/src/uniformscaling.jl Outdated Show resolved Hide resolved
@jishnub jishnub added the domain:linear algebra Linear algebra label Feb 1, 2023
@dkarrasch
Copy link
Member

Why not have separate copyto! methods with each copyto_diagonal! spelled out?

@jishnub
Copy link
Contributor Author
jishnub commented Feb 1, 2023

Can have that as well. Do you foresee any issues with ambiguities?

@dkarrasch
Copy link
Member

Do you foresee any issues with ambiguities?

I don't expect any. The signatures are so special, almost concrete.

@jishnub
Copy link
Contributor Author
jishnub commented Feb 10, 2023

Updated now

@N5N3 N5N3 merged commit 6cbcee9 into JuliaLang:master Feb 11, 2023
@jishnub jishnub deleted the copytobandedI branch February 11, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants