Backports release 1.12 #1353
Merged
Backports release 1.12 #1353
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(cherry picked from commit 95d009b)
Since `beta` is not used in this operation, we may set it to a constant to eliminate two of the branches generated by `@stable_muladdmul`. (cherry picked from commit 5122dbf)
(cherry picked from commit e64a3df)
(cherry picked from commit 0c87d0d)
Instead of `@noinline` on the entire size-check function, we now separate the error-throwing part into a separate function and mark it as `@noinline`. This way, the size check may still be evaluated inline, and only the error path will not be inlined. This improves performance for small matmul. ```julia julia> A = [1 2; 3 4]; julia> @Btime $A * $A; 53.361 ns (2 allocations: 112 bytes) # v"1.13.0-DEV.438" 47.504 ns (2 allocations: 112 bytes) # this PR ``` (cherry picked from commit e4e8c19)
Fixes the column ranges, so that these correctly correspond to the ranges over which there are top and bottom rows to consider beyond the bands. The change is to the upper limit of the range. (cherry picked from commit 95703b5)
Fixes
```julia
julia> T = Tridiagonal(1:0, 1:0, 1:0)
0×0 Tridiagonal{Int64, UnitRange{Int64}}
julia> T .+ T
ERROR: ArgumentError: invalid GenericMemory size: the number of elements is either negative or too large for system address width
```
After this,
```julia
julia> T .+ T
0×0 Tridiagonal{Int64, Vector{Int64}}
```
The changes are minor, but the largish diff is because of adding a loop
to the tests, and additional indentation as a consequence.
(cherry picked from commit dccd6f8)
As noted [on discourse](https://discourse.julialang.org/t/add-a-upperhessenberg-row-in-linearalgebra-factorize/128530/4?u=stevengj), we were missing `/` and `\` methods for `UpperHessenberg` matrices, despite the existence of optimized `ldiv!` and `rdiv!` methods, so it was falling back to slow $O(n^3)$ methods. While I was at it, I also added methods for transpose/adjoint Hessenberg matrices. --------- Co-authored-by: Jishnu Bhattacharya <[email protected]> (cherry picked from commit 9d26faf)
In broadcasting over triangular matrices, we loop only over the stored indices. For these indices, indexing into a triangular matrix is equivalent to indexing into its parent. We may therefore replace an `UpperOrLowerTriangular` matrix by its parent, which removes the branch in `getindex`. This improves performance: ```julia julia> L = LowerTriangular(zeros(600,600)); julia> L2 = copy(L); julia> @Btime broadcast!(+, $L2, $L, $L); 161.176 μs (0 allocations: 0 bytes) # master 80.894 μs (0 allocations: 0 bytes) # this PR ``` This replacement is performed recursively on a `Broadcasted` object by looping over its `args`, and non-triangular elements are left untouched. Only `UpperOrLowerTriangular` matrices will be replaced by their `parent`s. (cherry picked from commit 5165fd3)
This is an internal constructor, but many methods involving `Bidiagonal`
matrices currently use the `Char` `uplo` directly to construct a new
`Bidiagonal` matrix. We therefore may require the constructors to accept
a `Char` as well, wherever they accept a `Symbol`. This already works
for some of the `Bidiagonal` constructors, and this PR plugs a missing
case.
Fixes issues like
```julia
julia> B = Bidiagonal(fill(SMatrix{2,3}(1:6), 4), fill(SMatrix{1,3}(1:3), 3), :L)
4×4 Bidiagonal{StaticArraysCore.SArray{S, Int64, 2} where S<:Tuple, Vector{StaticArraysCore.SArray{S, Int64, 2} where S<:Tuple}}:
[1 3 5; 2 4 6] ⋅ ⋅ ⋅
[1 2 3] [1 3 5; 2 4 6] ⋅ ⋅
⋅ [1 2 3] [1 3 5; 2 4 6] ⋅
⋅ ⋅ [1 2 3] [1 3 5; 2 4 6]
julia> 2B
ERROR: MethodError: no method matching Bidiagonal(::Vector{SMatrix{2, 3, Int64, 6}}, ::Vector{SMatrix{1, 3, Int64, 3}}, ::Char)
The type `Bidiagonal` exists, but no method is defined for this combination of argument types when trying to construct it.
Closest candidates are:
Bidiagonal(::Vector{T}, ::Vector{S}, ::Symbol) where {T, S}
@ LinearAlgebra ~/Dropbox/JuliaPackages/LinearAlgebra_2.jl/src/bidiag.jl:77
Bidiagonal(::V, ::V, ::AbstractChar) where {T, V<:AbstractVector{T}}
@ LinearAlgebra ~/Dropbox/JuliaPackages/LinearAlgebra_2.jl/src/bidiag.jl:71
Bidiagonal(::V, ::V, ::Symbol) where {T, V<:AbstractVector{T}}
@ LinearAlgebra ~/Dropbox/JuliaPackages/LinearAlgebra_2.jl/src/bidiag.jl:68
...
```
After this,
```julia
julia> 2B
4×4 Bidiagonal{StaticArraysCore.SArray{S, Int64, 2} where S<:Tuple, Vector{StaticArraysCore.SArray{S, Int64, 2} where S<:Tuple}}:
[2 6 10; 4 8 12] ⋅ ⋅ ⋅
[2 4 6] [2 6 10; 4 8 12] ⋅ ⋅
⋅ [2 4 6] [2 6 10; 4 8 12] ⋅
⋅ ⋅ [2 4 6] [2 6 10; 4 8 12]
```
We may require that each method passes a `Symbol` as `uplo`, but this
would be a much larger change, and the current approach seems to be less
disruptive.
(cherry picked from commit 560276b)
 A comment about the existing docstring is: The lower example reads counter-intuitive, _given_ the experience from reading the upper example. And `X` and `Y` are not like `A` and `B`. Hope the revised version reads clearer and intelligible. (cherry picked from commit 2d35f07)
(cherry picked from commit 2148fa4)
This reverts commit 6d36322.
|
I'm reverting 6d36322 on this branch, as this adds potential method ambiguities. In any case, this is a performance-enhancing PR that should be on master. |
This backports #1312 to the backports-release-1.12 branch. Co-authored-by: Kristoffer Carlsson <[email protected]>
(cherry picked from commit b14390e)
This PR prunes the old `LinearAlgebra` module based on an environment variable that is set in `.ci/run_tests.jl`. Hopefully, this would mean that tests that are being run without this environment variable would run as expected without precompilation issues. The goal of this PR is to ensure that when julia is being built with the master branch of `LinearAlgebra`, the tests would work without the need for pruning (i.e., we want to fix the test failures in JuliaLang/julia#58242). This would also mean that to run the tests locally, one would need to set the `JULIA_PRUNE_OLD_LA` environment variable to `true`. (cherry picked from commit 87d4c8b)
This function didn't exist in Julia 1.11 There is a note in NEWS, here: https://github.com/JuliaLang/julia/blob/master/HISTORY.md#linearalgebra Added in JuliaLang/julia#56175 (cherry picked from commit 2cb1e98)
The ambiguity check was being run in a separate process, but it was not pruning the `LinearAlgebra` module. This was therefore not testing for ambiguities in the latest code, but the one in the sysimg. The PR would be able to replicate the test failure seen in https://buildkite.com/julialang/julia-master/builds/47424#0196c03a-2230-4d64-b404-e04dbfc76c3f (cherry picked from commit 0b46d5c)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-1.12 #1353 +/- ##
================================================
+ Coverage 91.93% 93.56% +1.62%
================================================
Files 34 34
Lines 15381 15443 +62
================================================
+ Hits 14141 14449 +308
+ Misses 1240 994 -246 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
) This fixes a regression in the `triu`/`tril` implementation on v1.12 and nightly. After this, the following works again: ```julia julia> M = fill(ones(2,2), 3, 3) 3×3 Matrix{Matrix{Float64}}: [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] julia> using Test: GenericArray julia> triu(GenericArray(M),1) 3×3 GenericArray{Matrix{Float64}, 2}: [0.0 0.0; 0.0 0.0] [1.0 1.0; 1.0 1.0] [1.0 1.0; 1.0 1.0] [0.0 0.0; 0.0 0.0] [0.0 0.0; 0.0 0.0] [1.0 1.0; 1.0 1.0] [0.0 0.0; 0.0 0.0] [0.0 0.0; 0.0 0.0] [0.0 0.0; 0.0 0.0] ```
`fillstored!` offers a way to fill only the stored indices of a
structured matrix without having to populate the parent. E.g.:
```julia
julia> U = UpperTriangular(zeros(4,4))
4×4 UpperTriangular{Float64, Matrix{Float64}}:
0.0 0.0 0.0 0.0
⋅ 0.0 0.0 0.0
⋅ ⋅ 0.0 0.0
⋅ ⋅ ⋅ 0.0
julia> LinearAlgebra.fillstored!(U, 2)
4×4 UpperTriangular{Float64, Matrix{Float64}}:
2.0 2.0 2.0 2.0
⋅ 2.0 2.0 2.0
⋅ ⋅ 2.0 2.0
⋅ ⋅ ⋅ 2.0
```
This seems like a useful function that should be public. This came up on
discourse:
https://discourse.julialang.org/t/how-to-set-all-elements-in-a-lower-triangular-matrix/128603
jishnub
force-pushed
the
backports-release-1.12
branch
from
5b14dcc to
aefab75
Compare
Currently, we forward copy for unit triangular matrices to the parents.
This also copies the diagonal, which may not be initialized in the
parent. This PR fixes this.
After this, the following works again
```julia
julia> M = Matrix{BigFloat}(undef,2,2);
julia> M[2,1] = 3;
julia> M'
2×2 adjoint(::Matrix{BigFloat}) with eltype BigFloat:
#undef 3.0
#undef #undef
julia> U = UnitUpperTriangular(M')
2×2 UnitUpperTriangular{BigFloat, Adjoint{BigFloat, Matrix{BigFloat}}}:
1.0 3.0
⋅ 1.0
julia> copyto!(similar(M), U)
2×2 Matrix{BigFloat}:
1.0 3.0
0.0 1.0
```
Fixes a regression introduced in v1.12, where we have a special branch for diagonal matrices to improve performance. The issue is that log for `Diagonal` matrices acts elementwise, which, for real matrices, requires the diagonal elements to be greater than zero. This PR adds an extra branch to check this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backported PRs:
LinearAlgebraqualifications incholesky.jl#1209diaginDiagonalkron#1230stable_muladdmulbranches ingeneric matvecmul!#1240peakflopson 32-bit #1255@noinlineerror path inmatmul_size_check#1310_isbanded_impl#1267triu/trilif no zero exists for theeltype#1320Tridiagonalbroadcast #1324iszerocheck in hessenberg setindex #1327HessenbergQ#13261:sizetoaxesin bidiag mul #1337Charuplo inBidiagonalconstructor #1342diagview#1355LinearAlgebramodule in ambiguity test #1349Contains multiple commits, manual intervention needed:
diagmexample #1298fillstored!public #1333Non-merged PRs with backport label: