JuliaLang / LinearAlgebra.jl

Julia Linear Algebra standard library
Other
13 stars 0 forks source link

[LinearAlgebra] `zero(::ArrayWrapper)` should forward to the parent #1050

Open MasonProtter opened 10 months ago

MasonProtter commented 10 months ago

So for the various array wrapper types like Symmetric, Hermitian, and Adjoint, we currently don't have a specialized zero method, but instead fall back to the AbstractArray definition which works in terms of similar. That means that e.g. this gives a MMatrix:

julia> using StaticArrays

julia> zero(Symmetric(SA[1 2; 3 4]))
2×2 Symmetric{Int64, MMatrix{2, 2, Int64, 4}} with indices SOneTo(2)×SOneTo(2):
 0  0
 0  0

I think though for these wrapper types, we should be able to define zero (and maybe some other functions?) like

for func in [:zero, #=Any others?=#]
    for Wrapper in [:Symmetric, :Hermitian, :Adjoint, #=Any others?=#]
       @eval Base.:($func)(M::$Wrapper) = $Wrapper($func(parent(M)))
    end
end

to avoid falling back to similar where possible, and make sure that custom methods like zero(::SMatrix) end up getting called instead.

MasonProtter commented 10 months ago

Should be a good first-issue for someone looking to make a PR (I don't have permission to add labels or I'd label it myself)

jishnub commented 10 months ago

@MasonProtter I think it's the triage permission that lets one label issues. Viral has sent you an invite. Could you please check if you have received the GitHub invite on your email (or a notification)?

MasonProtter commented 10 months ago

Yep, got it, thank you both!

ShrutiRDalvi commented 10 months ago

Hi, is someone working on this already, else I'd love to work on it . Thanks !

MasonProtter commented 10 months ago

I don't think so, please go for it!

ShrutiRDalvi commented 10 months ago

Hi, It's my first time working on open source and I'm still familiarizing myself with Julia. Could you please help me out in proceeding to solve the issue. Thanks a lot

jishnub commented 10 months ago

You would need to clone the Julia repo, and add the suggested code in the top comment (with appropriate modifications) to stdlib/LinearAlgebra/src/special.jl. You would also need to add tests for each added method to stdlib/LinearAlgebra/test/special.jl. Please create a separate @testset mentioning the issue number (you should be able to figure this out from the file). Please ensure that the tests are running. Finally, commit your changes to a new branch (preferably not the master), fork the Julia repo and push the changes to your fork, and open a PR to the main julia repo.

MasonProtter commented 10 months ago

Just a note for anyone else reading, we generally do not 'reserve' issues for people to fix. So if someone says they want to work on something, but doesn't actually start working on it, or if their attempt to fix it stalls out, or if you have a different, better way of solving the issues, it's completely okay to just go and submit your own PR to fix the issue.

This is not to say that people should feel encouraged to try and 'beat' someone else to submitting a PR for a feature, that's also bad behaviour. It's just that you shouldn't feel shy about working on something that someone expressed the intention to work on, but hasn't completed.