Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Feb 13, 2025

As noted in #1193, the sqrt(::Digonal{<:Real}) method requires the diagonal element to be positive, so that sqrt is defined for the individual elements. We therefore may restrict the diagonal branch in the dense sqrt method to matrices with positive diag if the eltype is Real.

Fixes #1193

After this,

julia> A = diagm(0 => [1.0, -1.0])
2×2 Matrix{Float64}:
 1.0   0.0
 0.0  -1.0

julia> sqrt(A)
2×2 Matrix{ComplexF64}:
 1.0+0.0im  0.0+0.0im
 0.0+0.0im  0.0+1.0im

@jishnub jishnub added the backport 1.12 Change should be backported to release-1.12 label Feb 14, 2025
@codecov
Copy link

codecov bot commented Feb 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.86%. Comparing base (2a1696a) to head (751b7ea).
Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1203   +/-   ##
=======================================
  Coverage   91.86%   91.86%           
=======================================
  Files          34       34           
  Lines       15365    15365           
=======================================
  Hits        14115    14115           
  Misses       1250     1250           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jishnub jishnub merged commit 16d9d61 into master Feb 17, 2025
4 checks passed
@jishnub jishnub deleted the jishnub/diagsqrt branch February 17, 2025 01:31
KristofferC pushed a commit that referenced this pull request Feb 25, 2025
As noted in #1193,
the `sqrt(::Digonal{<:Real})` method requires the diagonal element to be
positive, so that `sqrt` is defined for the individual elements. We
therefore may restrict the diagonal branch in the dense `sqrt` method to
matrices with positive `diag` if the `eltype` is `Real`.

Fixes #1193

After this,
```julia
julia> A = diagm(0 => [1.0, -1.0])
2×2 Matrix{Float64}:
 1.0   0.0
 0.0  -1.0

julia> sqrt(A)
2×2 Matrix{ComplexF64}:
 1.0+0.0im  0.0+0.0im
 0.0+0.0im  0.0+1.0im
```

(cherry picked from commit 16d9d61)
@KristofferC KristofferC mentioned this pull request Feb 25, 2025
7 tasks
KristofferC added a commit that referenced this pull request Feb 25, 2025
Backported PRs:
- [x] #1194 
- [x] #1207 
- [x] #1196 <!-- Explicitly declare type constructor imports -->
- [x] #1202 <!-- Add fast path in generic matmul -->
- [x] #1203 <!-- Restrict Diagonal sqrt branch to positive diag -->
- [x] #1210 <!-- Indirection in matrix multiplication to avoid
ambiguities -->
@jishnub jishnub mentioned this pull request May 12, 2025
27 tasks
@jishnub jishnub removed the backport 1.12 Change should be backported to release-1.12 label May 13, 2025
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

Successfully merging this pull request may close these issues.

Breaking change in sqrt for Matrix which is secretly Diagonal

2 participants