From bc3931fe8bacea458e0e5a2c041933f6c99a9d7c Mon Sep 17 00:00:00 2001 From: Federico Stra Date: Tue, 6 May 2025 14:43:07 +0200 Subject: [PATCH 1/4] perf: optimize `permutations` by implementing it via `multiset_permutations` As it currently stands, `multiset_permutations` is more efficient than `permutations`; see #151. We can exploit it to optimize `permutations`. --- src/permutations.jl | 54 ++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/src/permutations.jl b/src/permutations.jl index b6ef788..f008849 100644 --- a/src/permutations.jl +++ b/src/permutations.jl @@ -14,42 +14,26 @@ struct Permutations{T} length::Int end -function has_repeats(state::Vector{Int}) - # This can be safely marked inbounds because of the type restriction in the signature. - # If the type restriction is ever loosened, please check safety of the `@inbounds` - @inbounds for outer in eachindex(state) - for inner in (outer+1):lastindex(state) - if state[outer] == state[inner] - return true - end - end - end - return false -end - -function increment!(state::Vector{Int}, min::Int, max::Int) - state[end] += 1 - for i in reverse(eachindex(state))[firstindex(state):end-1] - if state[i] > max - state[i] = min - state[i-1] += 1 - end - end -end - -function next_permutation!(state::Vector{Int}, min::Int, max::Int) - while true - increment!(state, min, max) - has_repeats(state) || break - end -end - -function Base.iterate(p::Permutations, state::Vector{Int}=fill(firstindex(p.data), p.length)) - next_permutation!(state, firstindex(p.data), lastindex(p.data)) - if first(state) > lastindex(p.data) - return nothing +# The following code basically implements `permutations` in terms of `multiset_permutations` as +# +# permutations(a, t::Integer=length(a)) = Iterators.map( +# indices -> [a[i] for i in indices], +# multiset_permutations(eachindex(a), t)) +# +# with the difference that we can also define `eltype(::Permutations)`, which is used in some tests. + +function Base.iterate(p::Permutations, state=nothing) + if isnothing(state) + mp = multiset_permutations(eachindex(p.data), p.length) + it = iterate(mp) + if isnothing(it) return nothing end + else + mp, mp_state = state + it = iterate(mp, mp_state) + if isnothing(it) return nothing end end - [p.data[i] for i in state], state + indices, mp_state = it + return [p.data[i] for i in indices], (; mp, mp_state) end function Base.length(p::Permutations) From 397a684a4bdd93459c1fe179c0fcb12200c6dd36 Mon Sep 17 00:00:00 2001 From: Federico Stra Date: Tue, 6 May 2025 15:06:25 +0200 Subject: [PATCH 2/4] fix: use explicit NamedTuple notation tu support Julia 1.0 --- src/permutations.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/permutations.jl b/src/permutations.jl index f008849..855b30f 100644 --- a/src/permutations.jl +++ b/src/permutations.jl @@ -33,7 +33,7 @@ function Base.iterate(p::Permutations, state=nothing) if isnothing(it) return nothing end end indices, mp_state = it - return [p.data[i] for i in indices], (; mp, mp_state) + return [p.data[i] for i in indices], (mp=mp, mp_state=mp_state) end function Base.length(p::Permutations) From 995a046531e1cea0fd6981810014a349997d595c Mon Sep 17 00:00:00 2001 From: Federico Stra Date: Tue, 6 May 2025 15:10:38 +0200 Subject: [PATCH 3/4] fix: don't use `isnothing` to support Julia 1.0 --- src/permutations.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/permutations.jl b/src/permutations.jl index 855b30f..a9de3d1 100644 --- a/src/permutations.jl +++ b/src/permutations.jl @@ -23,14 +23,14 @@ end # with the difference that we can also define `eltype(::Permutations)`, which is used in some tests. function Base.iterate(p::Permutations, state=nothing) - if isnothing(state) + if state === nothing mp = multiset_permutations(eachindex(p.data), p.length) it = iterate(mp) - if isnothing(it) return nothing end + if it === nothing return nothing end else mp, mp_state = state it = iterate(mp, mp_state) - if isnothing(it) return nothing end + if it === nothing return nothing end end indices, mp_state = it return [p.data[i] for i in indices], (mp=mp, mp_state=mp_state) From 3f6ee6198aff32b47a8572614c03fa0b058ebc11 Mon Sep 17 00:00:00 2001 From: Federico Stra Date: Wed, 28 May 2025 19:32:22 +0200 Subject: [PATCH 4/4] fix: collect the indices into a `Vector` Without this fix there are issues with `OffsetArray`: `eachindex` returns an `OffsetArrays.IdOffsetRange`, which causes troubles when passed to `multiset_permutations` because `f = [sum(c == x for c in a)::Int for x in m]` becomes an `OffsetArray` instead of a Vector`. --- src/permutations.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/permutations.jl b/src/permutations.jl index 093def5..edab08a 100644 --- a/src/permutations.jl +++ b/src/permutations.jl @@ -25,7 +25,7 @@ end function Base.iterate(p::Permutations, state=nothing) if state === nothing - mp = multiset_permutations(eachindex(p.data), p.length) + mp = multiset_permutations(collect(eachindex(p.data)), p.length) it = iterate(mp) if it === nothing return nothing end else