faster sparse(::Diagonal)

[fredrikekre]

Missing method sparse(::Diagonal):

Size	master	PR	speedup
100x100	29.076 μs (7 allocations: 4.45 KiB)	210.451 ns (5 allocations: 1.86 KiB)	140
1000x1000	2.600 ms (7 allocations: 39.89 KiB)	1.450 μs (5 allocations: 16.05 KiB)	2k
10000x10000	283.301 ms (12 allocations: 391.22 KiB)	14.472 μs (7 allocations: 156.58 KiB)	20k
100000x100000	27.050 s (12 allocations: 3.82 MiB)	169.198 μs (7 allocations: 1.53 MiB)	160k

[andreasnoack]

Is the code path already exercised by existing tests?

[fredrikekre]

julia/test/sparse/sparse.jl

Lines 1254 to 1255 in b729e58

	D = Diagonal(ones(10,10))
	sm = sparse(D)

and also indirectly on some other places. But might as well add tests to this block

julia/test/sparse/sparse.jl

Lines 1295 to 1308 in b729e58

	@testset "issue #10837, sparse constructors from special matrices" begin
	T = Tridiagonal(randn(4),randn(5),randn(4))
	S = sparse(T)
	@test norm(Array(T) - Array(S)) == 0.0
	T = SymTridiagonal(randn(5),rand(4))
	S = sparse(T)
	@test norm(Array(T) - Array(S)) == 0.0
	B = Bidiagonal(randn(5),randn(4),:U)
	S = sparse(B)
	@test norm(Array(B) - Array(S)) == 0.0
	B = Bidiagonal(randn(5),randn(4),:L)
	S = sparse(B)
	@test norm(Array(B) - Array(S)) == 0.0
	end

[Sacha0]

Perhaps collect(1:(m+1)) in place of [1:m; m+1], and likewise collect(1:m) in place of [1:m;]?

julia> m = 1000
1000

julia> @benchmark [1:$m; $m+1]
BenchmarkTools.Trial:
  memory estimate:  9.23 KiB
  allocs estimate:  46
  --------------
  minimum time:     5.816 μs (0.00% GC)
  median time:      6.441 μs (0.00% GC)
  mean time:        8.004 μs (9.65% GC)
  maximum time:     444.588 μs (93.81% GC)
  --------------
  samples:          10000
  evals/sample:     6

julia> @benchmark collect(1:$m)
BenchmarkTools.Trial:
  memory estimate:  7.97 KiB
  allocs estimate:  2
  --------------
  minimum time:     668.695 ns (0.00% GC)
  median time:      1.164 μs (0.00% GC)
  mean time:        2.173 μs (31.38% GC)
  maximum time:     24.659 μs (72.19% GC)
  --------------
  samples:          10000
  evals/sample:     131

[fredrikekre]

Interesting, will update.

[Sacha0]

Might the convert(Vector{T}, D.diag) produce unexpected aliasing if D.diag isa Vector{T}`?

[fredrikekre]

Yes, but that was intentional from my side 😄

[Sacha0]

Cheers :). Do other sparse methods alias their inputs?

[fredrikekre]

No, because the can't really do that; there are no other arrays that have all their nzvals in one ordered vector like Diagonal does. Perhaps best to make a copy here?

[Sacha0]

Perhaps copying here as well would be for the best then, yes :). Thoughts?

[fredrikekre]

Updated, and updated the benchmarks in top post too.

[Sacha0]

Thanks @fredrikekre! :)