Skip to content

Commit

Permalink
Generalize findn and findnz for non-1 indices
Browse files Browse the repository at this point in the history
(cherry picked from commit 37e9c5c)
ref #17816
  • Loading branch information
timholy authored and tkelman committed Aug 11, 2016
1 parent bee5a34 commit 3d980ca
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
4 changes: 2 additions & 2 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ function findn(A::AbstractMatrix)
I = similar(A, Int, nnzA)
J = similar(A, Int, nnzA)
count = 1
for j=1:size(A,2), i=1:size(A,1)
for j=indices(A,2), i=indices(A,1)
if A[i,j] != 0
I[count] = i
J[count] = j
Expand All @@ -812,7 +812,7 @@ function findnz{T}(A::AbstractMatrix{T})
NZs = Array{T,1}(nnzA)
count = 1
if nnzA > 0
for j=1:size(A,2), i=1:size(A,1)
for j=indices(A,2), i=indices(A,1)
Aij = A[i,j]
if Aij != 0
I[count] = i
Expand Down
8 changes: 8 additions & 0 deletions test/offsetarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,14 @@ pmax, ipmax = findmax(parent(A))
@test amax == pmax
@test A[iamax] == amax
@test amax == parent(A)[ipmax]
z = OffsetArray([0 0; 2 0; 0 0; 0 0], (-3,-1))
I,J = findn(z)
@test I == [-1]
@test J == [0]
I,J,N = findnz(z)
@test I == [-1]
@test J == [0]
@test N == [2]

v = OffsetArray([1,1e100,1,-1e100], (-3,))*1000
v2 = OffsetArray([1,-1e100,1,1e100], (5,))*1000
Expand Down

4 comments on commit 3d980ca

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, vs = "@0030eec2f332f353e6890ca289ac2aca55532dde")

just after the backport of #17816, 3d980ca vs 0.5.0-rc0

I have a theory that the indices generalizations may have caused the slowdown in lucompletepiv.

@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 @jrevels

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

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

@timholy that looks pretty conclusive to me as causing a 2x slowdown in the lucompletepiv benchmarks, compare to https://github.com/JuliaCI/BaseBenchmarkReports/blob/2b5b177a2f45303a24edde8d084d3f4b03134353/aee17b6_vs_0030eec/report.md just a few commits earlier

@timholy
Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll look into it.

Please sign in to comment.