Merge pull request #35 from kalmarek/mk/fixes_for_Wedderburn

fixes for wedderburn

Project.toml

@@ -1,7 +1,7 @@
 name = "PermutationGroups"
 uuid = "8bc5a954-2dfc-11e9-10e6-cd969bffa420"
 authors = ["Marek Kaluba <kalmar@amu.edu.pl>", "tweisser <tillmann.weisser@web.de>"]
-version = "0.4.1"
+version = "0.4.2"
 
 [deps]
 GroupsCore = "d5909c97-4eac-4ecc-a3dc-fdd0858a4120"

src/Perms/arithmetic.jl

@@ -31,6 +31,7 @@ function Base.:(*)(
 end
 
 function Base.:(*)(σ::AbstractPermutation, τs::AbstractPermutation...)
+    isempty(τs) && return σ
     deg = max(degree(σ), maximum(degree, τs))
     img = Vector{inttype(σ)}(undef, deg)
     for i in Base.OneTo(deg)

src/Perms/perm_images.jl

@@ -45,34 +45,6 @@ else
     end
 end
 
-# performance ?
-@static if VERSION < v"1.7"
-    function Base.copy(p::Perm)
-        imgs = copy(p.images)
-        q = typeof(p)(imgs, false)
-        if isdefined(p, :inv)
-            inv_imgs = copy(p.inv.images)
-            q⁻¹ = typeof(p)(inv_imgs, false)
-            q.inv = q⁻¹
-            q⁻¹.inv = q
-        end
-        return q
-    end
-
-else
-    function Base.copy(p::Perm)
-        imgs = copy(p.images)
-        q = typeof(p)(imgs, false)
-        if isdefined(p, :inv, :sequentially_consistent)
-            inv_imgs = copy(@atomic(p.inv).images)
-            q⁻¹ = typeof(p)(inv_imgs, false)
-            @atomic q.inv = q⁻¹
-            @atomic q⁻¹.inv = q
-        end
-        return q
-    end
-end
-
 # convienience constructor: inttype(::Type{<:AbstractPermutation}) defaults to UInt32
 function Perm(images::AbstractVector{<:Integer}, check = true)
     return Perm{inttype(Perm)}(images, check)
@@ -86,6 +58,7 @@ end
 
 # inttype must be T (UInt16 by default) since we store it in e.g. cycles
 inttype(::Type{Perm{T}}) where {T} = T
+inttype(::Type{Perm}) = UInt16
 
 # ## Interface of AbstractPermutation
 degree(σ::Perm) = length(σ.images)
@@ -94,6 +67,18 @@ function Base.:^(n::Integer, σ::Perm)
     return 1 ≤ n ≤ degree(σ) ? oftype(n, @inbounds σ.images[n]) : n
 end
 @static if VERSION < v"1.7"
+    function Base.copy(p::Perm)
+        imgs = copy(p.images)
+        q = typeof(p)(imgs, false)
+        if isdefined(p, :inv)
+            inv_imgs = copy(p.inv.images)
+            q⁻¹ = typeof(p)(inv_imgs, false)
+            q.inv = q⁻¹
+            q⁻¹.inv = q
+        end
+        return q
+    end
+
     function Base.inv(σ::Perm)
         if !isdefined(σ, :inv)
             σ⁻¹ = typeof(σ)(invperm(σ.images), false)
@@ -102,22 +87,7 @@ end
         end
         return σ.inv
     end
-else
-    function Base.inv(σ::Perm)
-        if !isdefined(σ, :inv, :sequentially_consistent)
-            σ⁻¹ = typeof(σ)(invperm(σ.images), false)
-            # we don't want to end up with two copies of inverse σ floating around
-            if !isdefined(σ, :inv, :sequentially_consistent)
-                @atomic σ.inv = σ⁻¹
-                @atomic σ⁻¹.inv = σ
-            end
-        end
-        return σ.inv
-    end
-end
 
-# optional
-@static if VERSION < v"1.7"
     function cycles(σ::Perm)
         if !isdefined(σ, :cycles)
             cdec = CycleDecomposition(σ)
@@ -126,6 +96,34 @@ end
         return σ.cycles
     end
 else
+    function Base.copy(p::Perm)
+        imgs = copy(p.images)
+        q = typeof(p)(imgs, false)
+        if isdefined(p, :inv, :sequentially_consistent)
+            inv_imgs = copy(@atomic(p.inv).images)
+            q⁻¹ = typeof(p)(inv_imgs, false)
+            @atomic q.inv = q⁻¹
+            @atomic q⁻¹.inv = q
+        end
+        return q
+    end
+
+    function Base.inv(σ::Perm)
+        if !isdefined(σ, :inv, :sequentially_consistent)
+            if isone(σ)
+                @atomic σ.inv = σ
+            else
+                σ⁻¹ = typeof(σ)(invperm(σ.images), false)
+                # we don't want to end up with two copies of inverse σ floating around
+                if !isdefined(σ, :inv, :sequentially_consistent)
+                    @atomic σ.inv = σ⁻¹
+                    @atomic σ⁻¹.inv = σ
+                end
+            end
+        end
+        return σ.inv
+    end
+
     function cycles(σ::Perm)
         if !isdefined(σ, :cycles, :sequentially_consistent)
             cdec = CycleDecomposition(σ)

src/PermutationGroups.jl

@@ -4,13 +4,9 @@ using GroupsCore
 import GroupsCore: istrivial
 using Random
 
-abstract type AbstractPermutationGroup <: Group end
-Base.IteratorSize(::Type{<:AbstractPermutationGroup}) = Base.HasLength()
-
 include("Perms/Perms.jl")
 import .Perms:
     Perm, AbstractPermutation, cycles, degree, firstmoved, permtype, @perm_str
-
 export @perm_str, Perm, permtype
 
 export AbstractOrbit,
@@ -22,13 +18,18 @@ export AbstractOrbit,
     Permutation,
     StabilizerChain
 export firstmoved, fixes, fixedpoints, lastmoved, nfixedpoints
-export base, degree, representative, schreier_sims, sgs, sift
+export base, degree, perm, representative, schreier_sims, sgs, sift
+
+abstract type AbstractPermutationGroup <: Group end
+Base.IteratorSize(::Type{<:AbstractPermutationGroup}) = Base.HasLength()
+Perms.degree(G::AbstractPermutationGroup) = maximum(degree, gens(G))
 
 include("orbit.jl")
 include("stabchain.jl")
 include("schreier_sims.jl")
 include("perm_group.jl")
 include("group_interface.jl")
+include("io.jl")
 
 include("precompile.jl")
 

src/io.jl

@@ -1,27 +1,29 @@
-Base.show(io::IO, ::Type{<:PermGroup{I}}) where I = print(io, PermGroup, "{$I, …}")
-Base.show(io::IO, ::Type{<:Permutation{I}}) where I = print(io, Permutation, "{$I, …}")
-
+function Base.show(io::IO, ::Type{<:PermGroup{I}}) where {I}
+    return print(io, PermGroup, "{$I, …}")
+end
+# function Base.show(io::IO, ::Type{<:Permutation{I, G}}) where {I, G}
+#     return print(io, Permutation, "{$I, …}")
+# end
 function Base.show(io::IO, G::PermGroup)
-    init = isdefined(G, :stabchain) ? " of order $(order(StabilizerChain(G)))" : ""
-    ngen = length(gens(G))
-
-    print(io, "Permutation group on ", ngen, " generator", ngen > 1 ? "s" : "", init)
+    print(io, "PermGroup( ")
+    join(io, gens(G), ", ")
+    return print(io, " )")
 end
 
 function Base.show(io::IO, ::MIME"text/plain", G::PermGroup)
-    init = isdefined(G, :stabchain) ? " of order $(order(StabilizerChain(G)))" : ""
-    ngen = length(gens(G))
+    o = isdefined(G, :stabchain) ? " of order $(order(StabilizerChain(G)))" : ""
+    ngen = ngens(G)
 
     println(
         io,
         "Permutation group on ",
         ngen,
         " generator",
         ngen > 1 ? "s" : "",
-        init,
+        o,
         " generated by",
     )
-    Base.print_array(io, gens(G))
+    return Base.print_array(io, gens(G))
 end
 
-Base.show(io::IO, g::Permutation) = show(io, perm(g))
+# Base.show(io::IO, g::Permutation) = show(io, Perm.perm(g))

src/perm_group.jl

@@ -16,7 +16,8 @@ and stabilizer chain are computed (and cached) _when needed_.
             T::Type{<:AbstractTransversal},
             gens::AbstractVector{<:AbstractPermutation},
         )
-            return new{eltype(gens),T}([Perms.perm(s) for s in gens])
+            gens_raw = [Perms.perm(s) for s in gens]
+            return new{eltype(gens_raw),T(eltype(gens_raw))}(gens_raw)
         end
     end
 else
@@ -30,34 +31,27 @@ else
             T::Type{<:AbstractTransversal},
             gens::AbstractVector{<:AbstractPermutation},
         )
-            return new{eltype(gens),T}([Perms.perm(s) for s in gens])
+            gens_raw = [Perms.perm(s) for s in gens]
+            return new{eltype(gens_raw),T(eltype(gens_raw))}(gens_raw)
         end
     end
 end
 
-for T in (:Transversal, :SchreierTransversal)
-    @eval function PermGroup(
-        ::Type{$T},
-        gens::AbstractVector{<:AbstractPermutation},
-    )
-        return PermGroup($T(eltype(gens)), gens)
-    end
-end
-
 function PermGroup(gens::AbstractVector{<:AbstractPermutation})
     return PermGroup(Transversal, gens)
 end
 
 PermGroup(gens::Vararg{P,N}) where {P,N} = PermGroup(collect(gens))
 
+__gens_raw(G::PermGroup) = G.__gens_raw
+
 struct Permutation{P,G<:PermGroup} <: AbstractPermutation
     perm::P
     parent::G
 end
 
-__gens_raw(G::PermGroup) = G.__gens_raw
-
 Perms.perm(p::Permutation) = Perms.perm(p.perm)
+Base.copy(σ::Permutation) = Permutation(σ.perm, parent(σ))
 
 # Perms.Perm interface
 Perms.inttype(::Type{<:Permutation{P}}) where {P} = Perms.inttype(P)
@@ -82,7 +76,7 @@ end
 
 function Base.conj(σ::Permutation{P}, τ::AbstractPermutation) where {P}
     deg = max(degree(σ), degree(τ))
-    img = Vector{Perms.inttype(Perms.perm(σ))}(undef, deg)
+    img = Vector{Perms.inttype(σ)}(undef, deg)
     for i in Base.OneTo(deg)
         img[i^τ] = (i^σ)^τ
     end

src/precompile.jl

@@ -10,4 +10,3 @@ PrecompileTools.@setup_workload begin
         @assert sum(ff, G) == 108
     end
 end
-

src/schreier_sims.jl

@@ -65,6 +65,7 @@ function extend_chain!(
     stabch::StabilizerChain{P,T},
     g::AbstractPermutation,
 ) where {P,T}
+    @assert istrivial(stabch)
     @assert !isone(g)
 
     # we want to modify stabch in-place, so we access the fields directly
@@ -76,7 +77,7 @@ function extend_chain!(
     k = length(transversal(stabch))
     if k < order(g)
         # gᵏ stabilizes point(stabch) so is a generator for stabilizer(pts)
-        extend_chain!(stabch, g^k)
+        extend_chain!(stabch.stabilizer, g^k)
     end
     return stabch
 end

test/AbstractPerm_interface.jl

@@ -48,6 +48,11 @@ function abstract_perm_interface_test(P::Type{<:PG.AbstractPermutation})
 
         @test Perm(r) * p isa Perm
 
+        @test *(p) == p
+        @test p * p * p * p == p^4
+
+        @test length(unique(p^i for i in 1:5)) == 3
+
         @test (1:5) .^ p == [3, 1, 2, 4, 5]
         @test sprint(show, p) == "(1,3,2)"
         @test sprint(show, PG.cycles(p)) == "Cycle Decomposition: (1,3,2)"

test/benchmark.jl

@@ -74,8 +74,8 @@ end
         @test order(G) == 384
         if BENCHMARK_TIME
             @info "Rubik cube 2×2×2 group:"
-            @btime order(Int, G) setup =
-                (G = PermGroup(Transversal, $cube222)) evals = 1
+            @btime order(Int, G) setup = (G = PermGroup(Transversal, $cube222)) evals =
+                1
             @btime order(Int, G) setup =
                 (G = PermGroup(SchreierTransversal, $cube222)) evals = 1
             # 24.767 μs (533 allocations: 44.37 KiB)
@@ -147,7 +147,7 @@ end
             @btime order(Int64, G) setup =
                 (G = PermGroup(SchreierTransversal, $([a, b]))) evals = 1
             # gap> G := Group([a,b]);; StabChain(G);; time;
-            # ~15ms
+            # ~35ms
             # 10.258 ms (30072 allocations: 11.49 MiB)
             # 98.011 ms (240549 allocations: 102.46 MiB)
         end
@@ -219,7 +219,7 @@ end
             @btime order(Int, G) setup =
                 (G = PermGroup(SchreierTransversal, $([a, b, c, d]))) evals = 1
             # gap> G := Group([a,b,c,d]);; StabChain(G);; time;
-            # ~15ms
+            # ~35ms
             # 2.759 ms (9133 allocations: 3.18 MiB)
             # 83.738 ms (221318 allocations: 77.73 MiB)
             G = PermGroup(Transversal, [a, b, c, d])

test/perm_groups.jl

@@ -8,7 +8,7 @@
     @test order(Int, G) isa Int
     @test isdefined(G, :stabchain) == true
 
-    G = PermGroup(perm"(1,2,3,4)", perm"(1,2)(4)")
+    G = PermGroup(perm"(1,2)", perm"(1,2,3,4)")
     @test StabilizerChain(G) isa StabilizerChain
     sc = StabilizerChain(G)
     @test length(sc) == 3
@@ -18,6 +18,11 @@
 
     @test length(PG.basis(G)) == 3
     @test order(G) == factorial(4)
+    @test degree(G) == 4
+
+    H = PermGroup(Permutation(perm"(1,2,3)", G))
+    @test order(Int, H) == 3
+    @test degree(H) == 3
 
     SN(n) = [Perm(circshift(collect(1:n), -1)), Perm([[2, 1]; 3:n])]
 
@@ -31,20 +36,27 @@
         @test order(G) == factorial(N)
     end
 
-    G = PermGroup(perm"(1,2,3,4)", perm"(1,2)")
+    G = PermGroup(perm"(1,2)", perm"(1,2,3,4)")
+    @test sprint(show, G) == "PermGroup( (1,2), (1,2,3,4) )"
+    @test sprint(show, MIME"text/plain"(), G) ==
+          "Permutation group on 2 generators generated by\n (1,2)\n (1,2,3,4)"
+    @test sprint(show, [G]) ==
+          "PermGroup{Perm{UInt16}, …}[PermGroup( (1,2), (1,2,3,4) )]"
 
-    m = match(r"order (\d+)", string(G))
+    m = match(r"order (\d+)", sprint(show, MIME"text/plain"(), G))
     @test m === nothing
 
     @test perm"(1,3)" in G
     @test perm"(1,5)" ∉ G
 
-    m = match(r"order (\d+)", string(G))
+    order(Int, G)
+
+    m = match(r"order (\d+)", sprint(show, MIME"text/plain"(), G))
     @test parse(Int, m.captures[1]) == 24
 
     A = PermGroup(perm"(1,2,3)", perm"(2,3,4)")
     @test order(A) == 12
-    m = match(r"order (\d+)", string(A))
+    m = match(r"order (\d+)", sprint(show, MIME"text/plain"(), A))
     @test parse(Int, m.captures[1]) == 12
 
     @test perm"(1,2)" ∉ A
@@ -65,4 +77,7 @@
         @test order(K2) == length(uniq_elements) == 60
         @test uniq_elements == elements
     end
+
+    G = PermGroup(perm"(1,4,6)(3,5)", perm"(1,5,4,3)")
+    @test order(Int, G) == 120
 end