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

a Diagonal of a SVector doesn't keep its type #235

Closed
yakir12 opened this issue Jun 22, 2017 · 9 comments
Closed

a Diagonal of a SVector doesn't keep its type #235

yakir12 opened this issue Jun 22, 2017 · 9 comments

Comments

@yakir12
Copy link

yakir12 commented Jun 22, 2017

First seen here:

julia> m = SVector(1,2,3)
3-element SVector{3,Int64}:
 1
 2
 3

julia> x = Diagonal(m)
3×3 Diagonal{Int64}:
 1    
   2  
     3

julia> x.diag
3-element Array{Int64,1}:
 1
 2
 3
@fredrikekre
Copy link
Member

Thats by definition of Diagonal in Base, it is always a Vector.

@yakir12
Copy link
Author

yakir12 commented Jun 22, 2017

I see.

@yakir12 yakir12 closed this as completed Jun 22, 2017
@fredrikekre
Copy link
Member

Could perhaps define an unexported Diagonal in StaticArrays. Something like:

struct Diagonal{T, N}
    diag::SVector{N, T}
end
Base.Diagonal(v::SVector) = Diagonal(v)
julia> m = SVector(1,2,3)
3-element SVector{3,Int64}:
 1
 2
 3

julia> Diagonal(m)
StaticArrays.Diagonal{Int64,3}([1, 2, 3])

Perhaps too many additional methods are needed to make this usable though.

@c42f
Copy link
Member

c42f commented Jun 22, 2017

Oh, thanks for pointing this out. It's another of many places in Base.LinAlg where things don't work quite right if you're using StaticArrays.

Obviously it's best to fix Base for 0.7, though I guess as a rather nasty hack, we could overload Diagonal(::StaticArray) to return our own special StaticArrays.Diagonal. Hmm.

@c42f c42f reopened this Jun 22, 2017
@c42f
Copy link
Member

c42f commented Jun 22, 2017

Or we could just define SDiagonal for now.

@yakir12
Copy link
Author

yakir12 commented Jun 22, 2017

The only objective I'd care about, as a user, is the performance and ease of use. So aesthetics aside, SDiagonal sounds great.

@c42f
Copy link
Member

c42f commented Jun 22, 2017

Yes, SDiagonal is probably the best option for clarity, until Base.Diagonal can support a static vector. Hijacking the Diagonal constructor might still be worth it: it's a bit evil, but probably less of a surprise to users than silently allocating data. If they really want a normal Diagonal, they can get it explicitly with Diagonal(collect(sa)) for some static array sa. It will require reimplementing a bunch of stuff from Base, as usual, but it's probably not that hard if we just go through and port the required pices from base/linalg/diagonal.jl (PRs welcome ;-))

@mschauer
Copy link
Collaborator

@andyferris
Copy link
Member

SDiagonal now exists.

oschulz pushed a commit to oschulz/StaticArrays.jl that referenced this issue Apr 4, 2023
* throw if no fields

* after code review
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

No branches or pull requests

5 participants