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

Create a copy while evaluating eigvals(::Diagonal) #45048

Merged
merged 2 commits into from
May 2, 2022

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Apr 21, 2022

Currently, eigvals(D::Diagonal{<:Number}) returns D.diag, which means that subsequent changes to D will change previously computed eigenvalues.

julia> D = Diagonal([1,2,3])
3×3 Diagonal{Int64, Vector{Int64}}:
 1    
   2  
     3

julia> λ = eigvals(D)
3-element Vector{Int64}:
 1
 2
 3

julia> D[3,3] = 4
4

julia> λ
3-element Vector{Int64}:
 1
 2
 4

Creating a copy should fix this.

jishnub added 2 commits April 21, 2022 13:51
Currently, `eigvals(D::Diagonal{<:Number})` returns `D.diag`, which means that subsequent changes to `D` will change previously computed eigenvalues.

```
julia> D = Diagonal([1,2,3])
3×3 Diagonal{Int64, Vector{Int64}}:
 1  ⋅  ⋅
 ⋅  2  ⋅
 ⋅  ⋅  3

julia> λ = eigvals(D)
3-element Vector{Int64}:
 1
 2
 3

julia> D[3,3] = 4
4

julia> λ
3-element Vector{Int64}:
 1
 2
 4
```

Creating a copy should fix this
@jishnub
Copy link
Contributor Author

jishnub commented Apr 23, 2022

@dkarrasch would like your view on this

@dkarrasch
Copy link
Member

dkarrasch commented Apr 26, 2022

Yes, this is the right way to do it. This should be backported to all versions >= v1.6. This aliasing issue was also found in #43856, but the "fix" (in the context of eigen) there included tests that weren't compatible with v1.6.

@dkarrasch dkarrasch added backport 1.6 Change should be backported to release-1.6 backport 1.7 backport 1.8 Change should be backported to release-1.8 linear algebra Linear algebra labels Apr 26, 2022
@dkarrasch dkarrasch merged commit b9d8280 into JuliaLang:master May 2, 2022
@jishnub jishnub deleted the patch-2 branch May 2, 2022 10:02
KristofferC pushed a commit that referenced this pull request May 16, 2022
@KristofferC KristofferC mentioned this pull request May 16, 2022
45 tasks
KristofferC pushed a commit that referenced this pull request May 16, 2022
@KristofferC KristofferC mentioned this pull request May 16, 2022
67 tasks
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label May 26, 2022
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Jul 6, 2022
KristofferC pushed a commit that referenced this pull request Dec 21, 2022
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants