# Performant conversion of Symmetric(SparseMatrixCSC) to SparseMatrixCSC

**URL:** <https://discourse.julialang.org/t/performant-conversion-of-symmetric-sparsematrixcsc-to-sparsematrixcsc/18865>\
**Category:** Internals & Design\
**Tags:** proposal\
**Created:** [December 20, 2018, 7:23pm UTC](https://discourse.julialang.org/t/performant-conversion-of-symmetric-sparsematrixcsc-to-sparsematrixcsc/18865 "2018-12-20T19:23:54Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![klacru](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/klacru/32/27890_2.png) [@klacru](https://discourse.julialang.org/u/klacru)\
**Post date:** [December 20, 2018, 7:23pm UTC](https://discourse.julialang.org/t/performant-conversion-of-symmetric-sparsematrixcsc-to-sparsematrixcsc/18865/1 "2018-12-20T19:23:55Z")

</div>

I would like to bring the following improvements to `stdlib/SparseArrays`.

```julia
julia> n = 10000
julia> Random.seed!(0); A = sprandn(n, n, 0.01); b = randn(n); nnz(A)
1000598

julia> SA = Symmetric(A);
julia> @btime B = SparseMatrixCSC(SA);
  3.049 s (35 allocations: 40.03 MiB)

julia> using SparseWrappers
julia> @btime B = SparseMatrixCSC(SA);
  10.060 ms (7 allocations: 15.32 MiB)

```

I would probably feel better if somebody welcomed this work. The implementation exists in my package [SparseWrappers](https://github.com/KlausC/SparseWrappers.jl) but needs to be converted into a PR for julia.

---

<div class="post-metadata">

**Author:** ![andreasnoack](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/andreasnoack/32/27_2.png) [@andreasnoack](https://discourse.julialang.org/u/andreasnoack)\
**Post date:** [December 27, 2018, 2:07pm UTC](https://discourse.julialang.org/t/performant-conversion-of-symmetric-sparsematrixcsc-to-sparsematrixcsc/18865/2 "2018-12-27T14:07:40Z")

</div>

`stdlib/SparseArrays` definitely shares the overall goal here. It would be great to improve the interoperability of the special matrix types and I agree that avoid nested special matrix types when possible is most likely part of the solution. We might also need other overall design changes. The current state is the result of a relatively organic process and not a grand design.

However, I disagree in some of the proposals in `SparseWrappers` and some of them go against conclusions in existing Julia issues. E.g we have already discussed your issue 2 and it’s very useful that `Hermitian` works as a Hermitian view that ignores the non-real part of the diagonal. It’s also unclear to me why `Symmetric(Symmetric(A, :U), :L)` would be `Diagonal`. Should it instead have been `UpperTriangular(LowerTriangular(A))`? I agree that the latter should be `Diagonal`.

---

<div class="post-metadata">

**Author:** ![klacru](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/klacru/32/27890_2.png) [@klacru](https://discourse.julialang.org/u/klacru)\
**Post date:** [December 27, 2018, 6:51pm UTC](https://discourse.julialang.org/t/performant-conversion-of-symmetric-sparsematrixcsc-to-sparsematrixcsc/18865/3 "2018-12-27T18:51:00Z")

</div>

Thanks for looking at the SparseWrsappers. Unfortunately the README.md is not maintained at its best.

> [@andreasnoack](#):
>
> E.g we have already discussed your issue 2 and it’s very useful that `Hermitian` works as a Hermitian view that ignores the non-real part of the diagonal.

I see, that this is comfortable sometimes. But I also think, it is not a good idea, because it makes some reasoning about combinations of wrappers unnecessarily complex.  
For example `(Symmetric(Hermitian(A))) == Symmetric(A)` is true if `isreal(diag(A))` but not in general. So with the proposed restriction the simplification is possible, otherwise not.

Other question: does it make sense to have `Symmetric{Complex}` or `Adjoint{Real}`. In all my mathematical experience I always saw either `(Real, Symmetric, Transpose)` or `(Complex,Hermitian,Adjoint)`, but never mixtures like `Transpose(Ajoint(A)) == conj.(A)`…

> `Symmetric(Symmetric(A, :U), :L)` would be `Diagonal`

That is a mistake in README.md only, sorry for that. It should be  
`Symmetric(Symmetric(A, :U), :L) == Symmetric(A, :U)` and not throw an exception `ArgumentError: Cannot construct Symmetric; uplo doesn't match` as it is now.