Skip to content

Commit

Permalink
Normalize index to CartesianIndex in _modify! (#33187)
Browse files Browse the repository at this point in the history
  • Loading branch information
tkf authored and KristofferC committed Sep 8, 2019
1 parent 3424d2c commit 6a20ad7
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
11 changes: 7 additions & 4 deletions stdlib/LinearAlgebra/src/generic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,15 @@ julia> C
"""
@inline @propagate_inbounds function _modify!(p::MulAddMul{ais1, bis0},
x, C, idx′) where {ais1, bis0}
# Workaround for performance penalty of splatting a number (#29114):
idx = idx′ isa Integer ? (idx′,) : idx′
# `idx′` may be an integer, a tuple of integer, or a `CartesianIndex`.
# Let `CartesianIndex` constructor normalize them so that it can be
# used uniformly. It also acts as a workaround for performance penalty
# of splatting a number (#29114):
idx = CartesianIndex(idx′)
if bis0
C[idx...] = p(x)
C[idx] = p(x)
else
C[idx...] = p(x, C[idx...])
C[idx] = p(x, C[idx])
end
return
end
Expand Down
6 changes: 6 additions & 0 deletions stdlib/LinearAlgebra/test/matmul.jl
Original file line number Diff line number Diff line change
Expand Up @@ -575,4 +575,10 @@ end
end
end

@testset "CartesianIndex handling in _modify!" begin
C = rand(10, 10)
A = rand(10, 10)
@test mul!(view(C, 1:10, 1:10), A, 0.5) == A * 0.5
end

end # module TestMatmul

2 comments on commit 6a20ad7

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.