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

Convert to sparse array by mapping over components #275

Closed
wants to merge 4 commits into from

Conversation

jishnub
Copy link
Member

@jishnub jishnub commented May 24, 2023

Structured matrix types may be converted to sparse matrices efficiently, so it makes sense to map sparse to the components of a StructArray

On master

julia> using LinearAlgebra, SparseArrays, StructArrays, BenchmarkTools

julia> D = Diagonal(Float64[1:4000;]);

julia> S = StructArray{ComplexF64}((D, D));

julia> @btime sparse($S);
  47.009 ms (16 allocations: 192.64 KiB)

This PR

julia> @btime sparse($S);
  74.021 μs (18 allocations: 407.02 KiB)

I have structured this as an extension module to make it easier to move SparseArrays to an extension in the future. I've made this an extension.

@jishnub
Copy link
Member Author

jishnub commented May 24, 2023

Test failures are as seen in #274, and seem to be unrelated to this PR

Edit: tests appear to pass after the latest commit. The failure on nightly is due to changes in how NamedTuples are displayed in the docstrings, and is unrelated to this PR.

@piever
Copy link
Collaborator

piever commented Dec 6, 2023

Thanks for the PR and sorry for the long wait! I've rebase this in #289 (we just a small change to use createinstance to generate the sparse array. If that makes sense to you, I can go ahead and close this one and merge #289.

@jishnub
Copy link
Member Author

jishnub commented Dec 6, 2023

Yes, makes sense, thanks!

@piever
Copy link
Collaborator

piever commented Dec 14, 2023

Perfect, this was merged in #289

@piever piever closed this Dec 14, 2023
@jishnub jishnub deleted the sparse branch December 14, 2023 15:00
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