Skip to content

make Diagonal+Symmetric return Symmetric#35333

Merged
andreasnoack merged 4 commits intomasterfrom
dk/symhermdiag
Apr 6, 2020
Merged

make Diagonal+Symmetric return Symmetric#35333
andreasnoack merged 4 commits intomasterfrom
dk/symhermdiag

Conversation

@dkarrasch
Copy link
Copy Markdown
Member

@dkarrasch dkarrasch commented Apr 1, 2020

@dkarrasch dkarrasch added the linear algebra Linear algebra label Apr 1, 2020
andreasnoack
andreasnoack reviewed Apr 1, 2020
View reviewed changes
andreasnoack
andreasnoack reviewed Apr 1, 2020
View reviewed changes
@dkarrasch
Copy link
Copy Markdown
Member Author

dkarrasch commented Apr 2, 2020

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
Copy Markdown
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.

@dkarrasch dkarrasch requested a review from andreasnoack April 3, 2020 19:50
@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)
KristofferC pushed a commit to JuliaLang/LinearAlgebra.jl that referenced this pull request Nov 14, 2024
@jishnub
Restrict binary ops for Diagonal and Symmetric to Number eltypes (#55…
8f8e944 
…251)

The `(::Diagonal) + (::Symmetric)` and analogous methods were
specialized in JuliaLang/julia#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 Nov 22, 2024
@jishnub @KristofferC
Restrict binary ops for Diagonal and Symmetric to Number eltypes (#55251
30573be 
)

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)
KristofferC pushed a commit to JuliaLang/LinearAlgebra.jl that referenced this pull request Dec 2, 2024
@jishnub
Restrict binary ops for Diagonal and Symmetric to Number eltypes (#55…
ed79e3d 
…251)

The `(::Diagonal) + (::Symmetric)` and analogous methods were
specialized in JuliaLang/julia#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 197295c84aab217bffde01a4339f1d0e8e95fb98)
(cherry picked from commit 44fc53552c0a3bf7528af998ebd239f1edc2e78d)
KristofferC pushed a commit that referenced this pull request Jan 13, 2025
@jishnub
Restrict binary ops for Diagonal and Symmetric to Number eltypes (#55251
78de77e 
)

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

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