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.

Sign up for GitHub

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

Jump to bottom

make Diagonal+Symmetric return Symmetric #35333

Merged
merged 4 commits into from
Apr 6, 2020
Merged

make Diagonal+Symmetric return Symmetric #35333

merged 4 commits into from
Apr 6, 2020

Conversation

dkarrasch
Copy link
Member

Closes #35307.

@dkarrasch dkarrasch added the domain:linear algebra Linear algebra label Apr 1, 2020
@dkarrasch
Copy link
Member Author

I thought I'll introduce a new methods for of type [+/-](A::AbstractMatrix, D::Diagonal) etc. where I simply copy A, and add/subtract D to/from the diagonal. This speeds up addition and subtraction a little bit. The Symmetric/Hermitian case then simply falls back to that, and wraps the result appropriately.

@dkarrasch
Copy link
Member Author

dkarrasch commented Apr 2, 2020 •
edited
Loading

Ouch, that failed. I'll remove the new generic methods, and leave to the ecosystem handling of +(S.data, D) etc.

@andreasnoack andreasnoack merged commit e25b357 into master Apr 6, 2020
@andreasnoack andreasnoack deleted the dk/symhermdiag branch April 6, 2020 08:24
ravibitsgoa pushed a commit to ravibitsgoa/julia that referenced this pull request Apr 9, 2020
@dkarrasch @ravibitsgoa
make Diagonal+Symmetric return Symmetric (JuliaLang#35333)
816e984 
* make Diagonal+Symmetric return Symmetric

* introduce generic +/- AbstractMatrix with Diagonal

* fix typos in tests

* remove generic addition/subtraction with Diagonals
ztultrebor pushed a commit to ztultrebor/julia that referenced this pull request Apr 17, 2020
@dkarrasch @ztultrebor
make Diagonal+Symmetric return Symmetric (JuliaLang#35333)
bc2de36 
* make Diagonal+Symmetric return Symmetric

* introduce generic +/- AbstractMatrix with Diagonal

* fix typos in tests

* remove generic addition/subtraction with Diagonals
staticfloat pushed a commit that referenced this pull request Apr 21, 2020
@dkarrasch @staticfloat
make Diagonal+Symmetric return Symmetric (#35333)
f99bc6f 
* make Diagonal+Symmetric return Symmetric

* introduce generic +/- AbstractMatrix with Diagonal

* fix typos in tests

* remove generic addition/subtraction with Diagonals
@jishnub jishnub mentioned this pull request Jul 25, 2024
Merged
jishnub added a commit that referenced this pull request Jul 28, 2024
@jishnub
Restrict binary ops for Diagonal and Symmetric to Number eltypes (#55251
197295c 
)

The `(::Diagonal) + (::Symmetric)` and analogous methods were
specialized in #35333 to return a
`Symmetric`, but these only work if the `Diagonal` is also symmetric.
This typically holds for arrays of numbers, but may not hold for
block-diagonal and other types for which symmetry isn't guaranteed. This
PR restricts the methods to arrays of `Number`s.

Fixes, e.g.:
```julia
julia> using StaticArrays, LinearAlgebra

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

julia> S = Symmetric(D)
2×2 Symmetric{AbstractMatrix, Diagonal{SMatrix{2, 2, Int64, 4}, Vector{SMatrix{2, 2, Int64, 4}}}}:
 [1 3; 3 4]      ⋅     
     ⋅       [1 3; 3 4]

julia> S + D
2×2 Symmetric{AbstractMatrix, Diagonal{SMatrix{2, 2, Int64, 4}, Vector{SMatrix{2, 2, Int64, 4}}}}:
 [2 6; 6 8]      ⋅     
     ⋅       [2 6; 6 8]

julia> S[1,1] + D[1,1]
2×2 SMatrix{2, 2, Int64, 4} with indices SOneTo(2)×SOneTo(2):
 2  6
 5  8

julia> (S + D)[1,1] == S[1,1] + D[1,1]
false
```
After this,
```julia
julia> S + D
2×2 Matrix{AbstractMatrix{Int64}}:
 [2 6; 5 8]  [0 0; 0 0]
 [0 0; 0 0]  [2 6; 5 8]
```

Even with `Number`s as elements, there might be an issue with `NaN`s
along the diagonal as `!issymmetric(NaN)`, but that may be a different
PR.
lazarusA pushed a commit to lazarusA/julia that referenced this pull request Aug 17, 2024
@jishnub @lazarusA
Restrict binary ops for Diagonal and Symmetric to Number eltypes (Jul…
696026e 
…iaLang#55251)

The `(::Diagonal) + (::Symmetric)` and analogous methods were
specialized in JuliaLang#35333 to return a
`Symmetric`, but these only work if the `Diagonal` is also symmetric.
This typically holds for arrays of numbers, but may not hold for
block-diagonal and other types for which symmetry isn't guaranteed. This
PR restricts the methods to arrays of `Number`s.

Fixes, e.g.:
```julia
julia> using StaticArrays, LinearAlgebra

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

julia> S = Symmetric(D)
2×2 Symmetric{AbstractMatrix, Diagonal{SMatrix{2, 2, Int64, 4}, Vector{SMatrix{2, 2, Int64, 4}}}}:
 [1 3; 3 4]      ⋅     
     ⋅       [1 3; 3 4]

julia> S + D
2×2 Symmetric{AbstractMatrix, Diagonal{SMatrix{2, 2, Int64, 4}, Vector{SMatrix{2, 2, Int64, 4}}}}:
 [2 6; 6 8]      ⋅     
     ⋅       [2 6; 6 8]

julia> S[1,1] + D[1,1]
2×2 SMatrix{2, 2, Int64, 4} with indices SOneTo(2)×SOneTo(2):
 2  6
 5  8

julia> (S + D)[1,1] == S[1,1] + D[1,1]
false
```
After this,
```julia
julia> S + D
2×2 Matrix{AbstractMatrix{Int64}}:
 [2 6; 5 8]  [0 0; 0 0]
 [0 0; 0 0]  [2 6; 5 8]
```

Even with `Number`s as elements, there might be an issue with `NaN`s
along the diagonal as `!issymmetric(NaN)`, but that may be a different
PR.
KristofferC pushed a commit that referenced this pull request Aug 19, 2024
@jishnub
Restrict binary ops for Diagonal and Symmetric to Number eltypes (#55251
44fc535 
)

The `(::Diagonal) + (::Symmetric)` and analogous methods were
specialized in #35333 to return a
`Symmetric`, but these only work if the `Diagonal` is also symmetric.
This typically holds for arrays of numbers, but may not hold for
block-diagonal and other types for which symmetry isn't guaranteed. This
PR restricts the methods to arrays of `Number`s.

Fixes, e.g.:
```julia
julia> using StaticArrays, LinearAlgebra

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

julia> S = Symmetric(D)
2×2 Symmetric{AbstractMatrix, Diagonal{SMatrix{2, 2, Int64, 4}, Vector{SMatrix{2, 2, Int64, 4}}}}:
 [1 3; 3 4]      ⋅
     ⋅       [1 3; 3 4]

julia> S + D
2×2 Symmetric{AbstractMatrix, Diagonal{SMatrix{2, 2, Int64, 4}, Vector{SMatrix{2, 2, Int64, 4}}}}:
 [2 6; 6 8]      ⋅
     ⋅       [2 6; 6 8]

julia> S[1,1] + D[1,1]
2×2 SMatrix{2, 2, Int64, 4} with indices SOneTo(2)×SOneTo(2):
 2  6
 5  8

julia> (S + D)[1,1] == S[1,1] + D[1,1]
false
```
After this,
```julia
julia> S + D
2×2 Matrix{AbstractMatrix{Int64}}:
 [2 6; 5 8]  [0 0; 0 0]
 [0 0; 0 0]  [2 6; 5 8]
```

Even with `Number`s as elements, there might be an issue with `NaN`s
along the diagonal as `!issymmetric(NaN)`, but that may be a different
PR.

(cherry picked from commit 197295c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreasnoack andreasnoack Awaiting requested review from andreasnoack

Assignees
No one assigned
Labels
domain:linear algebra Linear algebra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Diagonal + Symmetric should be Symmetric not Matrix
2 participants
@dkarrasch @andreasnoack