Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Aug 27, 2024

This replaces AbstractTriangular matrices by their parents in certain broadcast operations, where we only loop over the filled half. We also replace Diagonal/Bidiagonal/Tridiaognal/SymTridiagonal matrices by the corresponding zeros away from the bands. As a consequence, this removes branching in the indexing operations in the broadcast, which improves performance considerably.

julia> using LinearAlgebra

julia> U = UpperTriangular(rand(3000,3000)); D = Diagonal(rand(size(U,1)));

julia> @btime $U .+ $D;
  23.295 ms (3 allocations: 68.66 MiB) # nightly
  15.305 ms (3 allocations: 68.66 MiB) # This PR

julia> @btime parent($U) .+ $D; # dense matrix broadcast for reference
  26.637 ms (3 allocations: 68.66 MiB)

Also

julia> @btime UpperTriangular($D) .+ UpperTriangular($D);
  30.613 ms (3 allocations: 68.66 MiB) # nightly
  14.669 ms (3 allocations: 68.66 MiB) # This PR

Most of the performance gain comes from replacing the Diagonal by zero(eltype(D)) in the broadcasting. It's a bit surprising that branch-prediction doesn't do this already.

@jishnub jishnub added linear algebra Linear algebra arrays [a, r, r, a, y, s] labels Aug 27, 2024
@jishnub jishnub requested a review from dkarrasch August 29, 2024 10:52
@jishnub jishnub marked this pull request as draft August 29, 2024 11:55
@jishnub jishnub marked this pull request as ready for review August 29, 2024 12:00
@jishnub jishnub closed this Aug 29, 2024
@jishnub jishnub deleted the jishnub/triangular_broadcast branch August 29, 2024 12:07
@jishnub jishnub restored the jishnub/triangular_broadcast branch August 29, 2024 12:32
@jishnub jishnub reopened this Aug 29, 2024
@jishnub jishnub marked this pull request as draft August 29, 2024 12:32
@jishnub
Copy link
Member Author

jishnub commented Aug 29, 2024

This PR may be simplified quite a bit after #55626, as processing the structured matrices may not be necessary.

jishnub added a commit that referenced this pull request Aug 30, 2024
This provides most of the benefits seen in
#55604. The simpler
implementation appears to help with branch-prediction in inferring the
zero elements of the structured matrices.
The improved performance as a consequence:
```julia
julia> using LinearAlgebra

julia> U = UpperTriangular(rand(3000,3000)); D = Diagonal(rand(size(U,1)));

julia> @Btime $U .+ $D;
  23.405 ms (3 allocations: 68.66 MiB) # nightly
  15.266 ms (3 allocations: 68.66 MiB) # This PR
```

---------

Co-authored-by: Matt Bauman <[email protected]>
@jishnub jishnub force-pushed the jishnub/triangular_broadcast branch from 4e563f4 to a8439c1 Compare August 30, 2024 05:26
KristofferC pushed a commit that referenced this pull request Sep 12, 2024
This provides most of the benefits seen in
#55604. The simpler
implementation appears to help with branch-prediction in inferring the
zero elements of the structured matrices.
The improved performance as a consequence:
```julia
julia> using LinearAlgebra

julia> U = UpperTriangular(rand(3000,3000)); D = Diagonal(rand(size(U,1)));

julia> @Btime $U .+ $D;
  23.405 ms (3 allocations: 68.66 MiB) # nightly
  15.266 ms (3 allocations: 68.66 MiB) # This PR
```

---------

Co-authored-by: Matt Bauman <[email protected]>
@jishnub jishnub force-pushed the jishnub/triangular_broadcast branch from a8439c1 to bf12d91 Compare September 24, 2024 07:00
@jishnub jishnub removed the request for review from dkarrasch September 24, 2024 07:00
KristofferC pushed a commit to JuliaLang/LinearAlgebra.jl that referenced this pull request Nov 14, 2024
This provides most of the benefits seen in
JuliaLang/julia#55604. The simpler
implementation appears to help with branch-prediction in inferring the
zero elements of the structured matrices.
The improved performance as a consequence:
```julia
julia> using LinearAlgebra

julia> U = UpperTriangular(rand(3000,3000)); D = Diagonal(rand(size(U,1)));

julia> @Btime $U .+ $D;
  23.405 ms (3 allocations: 68.66 MiB) # nightly
  15.266 ms (3 allocations: 68.66 MiB) # This PR
```

---------

Co-authored-by: Matt Bauman <[email protected]>
@DilumAluthge
Copy link
Member

We have moved the LinearAlgebra stdlib to an external repo: https://github.com/JuliaLang/LinearAlgebra.jl

@jishnub If you think that this PR is still relevant, please open a new PR on the LinearAlgebra.jl repo.

@DilumAluthge DilumAluthge deleted the jishnub/triangular_broadcast branch January 12, 2025 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays [a, r, r, a, y, s] linear algebra Linear algebra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants