Skip to content

Commit

Permalink
Fix #6838. Add setindex! for triangular matrix types.
Browse files Browse the repository at this point in the history
  • Loading branch information
@andreasnoack
andreasnoack committed Jan 16, 2015
1 parent 5900fab commit ffc9778
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
5 changes: 5 additions & 0 deletions base/linalg/triangular.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ getindex{T,S}(A::LowerTriangular{T,S}, i::Integer, j::Integer) = i >= j ? A.data
getindex{T,S}(A::UnitUpperTriangular{T,S}, i::Integer, j::Integer) = i == j ? one(T) : (i < j ? A.data[i,j] : zero(A.data[i,j]))
getindex{T,S}(A::UpperTriangular{T,S}, i::Integer, j::Integer) = i <= j ? A.data[i,j] : zero(A.data[i,j])

setindex!(A::UpperTriangular, x, i::Integer, j::Integer) = i <= j ? (A.data[i,j] = x; A) : throw(BoundsError())
setindex!(A::UnitUpperTriangular, x, i::Integer, j::Integer) = i < j ? (A.data[i,j] = x; A) : throw(BoundsError())
setindex!(A::LowerTriangular, x, i::Integer, j::Integer) = i >= j ? (A.data[i,j] = x; A) : throw(BoundsError())
setindex!(A::UnitLowerTriangular, x, i::Integer, j::Integer) = i > j ? (A.data[i,j] = x; A) : throw(BoundsError())

istril{T,S}(A::LowerTriangular{T,S}) = true
istril{T,S}(A::UnitLowerTriangular{T,S}) = true
istril{T,S}(A::UpperTriangular{T,S}) = false
Expand Down
32 changes: 31 additions & 1 deletion test/linalg/triangular.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,41 @@ for elty1 in (Float32, Float64, Complex64, Complex128, BigFloat, Int)
# similar
@test isa(similar(A1), t1)

# Linear indexing
# getindex
## Linear indexing
for i = 1:length(A1)
@test A1[i] == full(A1)[i]
end

## Cartesian indexing
for i = 1:size(A1, 1)
for j = 1:size(A1, 2)
@test A1[i,j] == full(A1)[i,j]
end
end

# setindex! (and copy)
A1c = copy(A1)
for i = 1:size(A1, 1)
for j = 1:size(A1, 2)
if uplo1 == :U
if i > j || (i == j && t1 == UnitUpperTriangular)
@test_throws BoundsError A1c[i,j] = 0
else
A1c[i,j] = 0
@test A1c[i,j] == 0
end
else
if i < j || (i == j && t1 == UnitLowerTriangular)
@test_throws BoundsError A1c[i,j] = 0
else
A1c[i,j] = 0
@test A1c[i,j] == 0
end
end
end
end

# istril/istriu
if uplo1 == :L
@test istril(A1)
Expand Down

4 comments on commit ffc9778

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

This made the tests take about 5-6 minutes longer. Compare https://ci.appveyor.com/project/StefanKarpinski/julia/build/1.0.1734/job/i61faqq5e6q8nt5k to https://ci.appveyor.com/project/StefanKarpinski/julia/build/1.0.1736/job/x4ut90k6v0o3n6u0. Is that expected?

@andreasnoack
Copy link
Member Author

Choose a reason for hiding this comment

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

No. Not at all and I have just confirmed this locally. With latest master I get

julia> @time include("/Users/andreasnoack/julia-dev/test/linalg/triangular.jl")
elapsed time: 126.89969053 seconds (70198032244 bytes allocated, 35.91% gc time)

and just before this commit I get

julia> @time include("/Users/andreasnoack/julia-dev/test/linalg/triangular.jl")
elapsed time: 89.849410512 seconds (9722441316 bytes allocated, 22.65% gc time)

I'll look into it now. I can't really see why testing the triangular matrices would allocate 70 gb.

@andreasnoack
Copy link
Member Author

Choose a reason for hiding this comment

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

@tkelman This one should give a good speedup.

@andreasnoack
Copy link
Member Author

Choose a reason for hiding this comment

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

This one is 1da2b37

Please sign in to comment.