Backports release 1.12 #1217
Backports release 1.12 #1217
Conversation
This reinstates slightly altered versions of the methods that were removed in JuliaLang/julia#52389. Sort of fixes #1205, although this doesn't recover the full performance. However, this version is more general, and works with the example presented in JuliaLang/julia#52389. There's still a performance regression, but the full performance may only be obtained for mutable matrices, and we may not assume mutability in general. Performance: v1.10: ```julia julia> n = 100 100 julia> A = adjoint(sparse(Float64, I, n, n)); julia> B = Diagonal(ones(n)); julia> @Btime $A * $B; 837.119 ns (5 allocations: 2.59 KiB) ``` This PR ```julia julia> @Btime $A * $B; 1.106 μs (15 allocations: 5.56 KiB) ``` We need double the allocations here compared to earlier, as we firstly materialize `D' * A'`, and then we again copy the adjoint of this result. I wonder if this may be reduced. --------- Co-authored-by: Daniel Karrasch <[email protected]>
…#1194) Co-authored-by: Alexis Montoison <[email protected]>
|
Shouldn't we pull in all the other PRs that were merged since branching? #1186 is the only one I am not sure of. |
|
I am thinking we pull in everything until we get to RCs (except for bug changes). Does it make sense to stay on Linalg master and branch much later? That would mean holding off PRs like #1186 from merging when we are in the feature freeze (all the way to RCs), and that can often be many months. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-1.12 #1217 +/- ##
================================================
+ Coverage 91.89% 91.90% +0.01%
================================================
Files 34 34
Lines 15360 15385 +25
================================================
+ Hits 14115 14140 +25
Misses 1245 1245 ☔ View full report in Codecov by Sentry. |
|
Won't that make it much harder to deal with regressions on the backport branches? Before the repo was split, only bugfixes used to be backported. It's easier if regressions stay on the master branch, where firstly it's possible to address them faster, and secondly they don't delay releases. |
These would otherwise give a warning after JuliaLang/julia#57311, but are a good change even independently. --------- Co-authored-by: Daniel Karrasch <[email protected]> (cherry picked from commit 2a1696a)
This manually adds the critical optimisation investigated in JuliaLang/julia#56954. While we could rely on LLVM to continue doing this optimisation, it's more robust to add it manually. (cherry picked from commit ed35a37)
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)
(cherry picked from commit 5cf41c4)
|
Yes agree. Let's just focus on getting bugfixes. |
|
If #1214 doesn't get backported, it's a breaking change. I might be the only one who cares, but I'd like to know if that's actually a decision or a can kicked down the road. |
|
Yes we should backport that PR (it seems to be happening in #1237) |
7 participants
Backported PRs:
BLAS.trsm!instead ofLAPACK.trtrs!in left-triangular solves #1194Non-merged PRs with backport label:
algto a keyword argument in symmetric eigen #1214