Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to MP renamings #31

Merged
merged 23 commits into from
Sep 26, 2023
Merged

Update to MP renamings #31

merged 23 commits into from
Sep 26, 2023

Conversation

blegat
Copy link
Owner

@blegat blegat commented May 9, 2023

No description provided.

@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Attention: 46 lines in your changes are missing coverage. Please review.

Comparison is base (187c7c4) 93.05% compared to head (0227481) 84.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
- Coverage   93.05%   84.31%   -8.74%     
==========================================
  Files           9        8       -1     
  Lines         475      408      -67     
==========================================
- Hits          442      344      -98     
- Misses         33       64      +31     
Files Coverage Δ
src/CondensedMatterSOS.jl 62.50% <ø> (-25.00%) ⬇️
src/action.jl 100.00% <100.00%> (ø)
src/monom-set.jl 100.00% <100.00%> (ø)
src/operators.jl 92.59% <100.00%> (-0.62%) ⬇️
src/sos.jl 100.00% <100.00%> (+5.26%) ⬆️
src/types.jl 91.89% <93.75%> (-2.56%) ⬇️
src/symmetry.jl 68.49% <64.00%> (-17.45%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blegat
Copy link
Owner Author

blegat commented Sep 13, 2023

@kalmarek PermutationGroups.SymmetricGroup is gone, what is the replacement ?

@kalmarek
Copy link
Contributor

this thing came from AA previously;

At the moment probably it's best to define

julia> using PermutationGroups
[ Info: Precompiling PermutationGroups [8bc5a954-2dfc-11e9-10e6-cd969bffa420]

julia> symmetric_group(n) = PermGroup(Perm([2,1]), Perm([2:n;1]))
symmetric_group (generic function with 1 method)

julia> PermutationGroups.order(symmetric_group(4))
24

julia> collect(symmetric_group(3))
6-element Vector{Permutation{Perm{UInt16}, PermGroup{Perm{UInt16}, }}}:
 ()
 (2,3)
 (1,2)
 (1,2,3)
 (1,3,2)
 (1,3)

Do you think PermutationGroups.jl should provide symmetric_group and alternating_group?

@blegat
Copy link
Owner Author

blegat commented Sep 13, 2023

@kalmarek I now get

ERROR: DomainError with 6:
6 is not a square modulo 13
Stacktrace:
  [1] sqrtmod(n::Int64, q::Int64)
    @ SymbolicWedderburn.Characters.FiniteFields ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:109
  [2] sqrt
    @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:103 [inlined]
  [3] normalize!(chtbl::SymbolicWedderburn.Characters.CharacterTable{Lattice1Group, SymbolicWedderburn.Characters.FiniteFields.GF{13}, PermutationGroups.Orbit{CondensedMatterSOS.DirectSum}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:141
  [4] SymbolicWedderburn.Characters.CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{13}}, G::Lattice1Group, cclasses::Vector{PermutationGroups.Orbit{CondensedMatterSOS.DirectSum}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:73
  [5] SymbolicWedderburn.Characters.CharacterTable(R::Type{Rational{Int64}}, G::Lattice1Group, cclasses::Vector{PermutationGroups.Orbit{CondensedMatterSOS.DirectSum}})
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:83
  [6] SymbolicWedderburn.Characters.CharacterTable(R::Type{Rational{Int64}}, G::Lattice1Group)
    @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:82
  [7] symmetry_adapted_basis(T::Type, G::Lattice1Group, action::Action, basis::MonomialBasis{CondensedMatterSOS.SpinMonomial, Vector{CondensedMatterSOS.SpinMonomial}}, S::Type; semisimple::Bool)
    @ SymbolicWedderburn ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/sa_basis.jl:163

Is this a bug in CondensedMatterSOS ? It could be that my group is not satisfying the axioms of a group but we are checking that now

@kalmarek
Copy link
Contributor

@blegat where is this group defined?

@blegat
Copy link
Owner Author

blegat commented Sep 14, 2023

* rewrite the definitions of groups in symmetry.jl

* add Random
@blegat
Copy link
Owner Author

blegat commented Sep 14, 2023

Still the same error it seems.

@kalmarek
Copy link
Contributor

hmm

julia> G = Lattice1Group(2)
Lattice1Group(CondensedMatterSOS.CyclicGroup(2), CondensedMatterSOS.KleinPermGroup())

julia> SymbolicWedderburn.Characters.irreducible_characters(G)
ERROR: DomainError with 6:
6 is not a square modulo 13
Stacktrace:
 [1] sqrtmod(n::Int64, q::Int64)
   @ SymbolicWedderburn.Characters.FiniteFields ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:109
 [2] sqrt
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:103 [inlined]
 [3] normalize!(chtbl::SymbolicWedderburn.Characters.CharacterTable{Lattice1Group, SymbolicWedderburn.Characters.FiniteFields.GF{13}, PermutationGroups.Orbit{CondensedMatterSOS.DirectSumElem{CondensedMatterSOS.CyclicElem, CondensedMatterSOS.KleinPermElement}}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:141
 [4] SymbolicWedderburn.Characters.CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{13}}, G::Lattice1Group, cclasses::Vector{PermutationGroups.Orbit{CondensedMatterSOS.DirectSumElem{CondensedMatterSOS.CyclicElem, CondensedMatterSOS.KleinPermElement}}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:73
 [5] SymbolicWedderburn.Characters.CharacterTable(R::Type{Rational{Int64}}, G::Lattice1Group, cclasses::Vector{PermutationGroups.Orbit{CondensedMatterSOS.DirectSumElem{CondensedMatterSOS.CyclicElem, CondensedMatterSOS.KleinPermElement}}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:83
 [6] irreducible_characters
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:41 [inlined]
 [7] irreducible_characters
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:33 [inlined]
 [8] irreducible_characters(G::Lattice1Group)
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:33
 [9] top-level scope
   @ REPL[21]:1

julia> SymbolicWedderburn.Characters.irreducible_characters(G.K)
ERROR: DomainError with 6:
6 is not a square modulo 13
Stacktrace:
 [1] sqrtmod(n::Int64, q::Int64)
   @ SymbolicWedderburn.Characters.FiniteFields ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:109
 [2] sqrt
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:103 [inlined]
 [3] normalize!(chtbl::SymbolicWedderburn.Characters.CharacterTable{CondensedMatterSOS.KleinPermGroup, SymbolicWedderburn.Characters.FiniteFields.GF{13}, PermutationGroups.Orbit{CondensedMatterSOS.KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:141
 [4] SymbolicWedderburn.Characters.CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{13}}, G::CondensedMatterSOS.KleinPermGroup, cclasses::Vector{PermutationGroups.Orbit{CondensedMatterSOS.KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:73
 [5] SymbolicWedderburn.Characters.CharacterTable(R::Type{Rational{Int64}}, G::CondensedMatterSOS.KleinPermGroup, cclasses::Vector{PermutationGroups.Orbit{CondensedMatterSOS.KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:83
 [6] irreducible_characters
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:41 [inlined]
 [7] irreducible_characters
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:33 [inlined]
 [8] irreducible_characters(G::CondensedMatterSOS.KleinPermGroup)
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:33
 [9] top-level scope
   @ REPL[22]:1

julia> order(G.K)
24

Since all groups of order up to 30 are tested here:
https://github.com/kalmarek/SymbolicWedderburn.jl/blob/master/test/dixon.jl#L229
I'd assume something is wrong with the implementation of KleinPermGroup. I didn't check all the formulas while rewriting.

@kalmarek
Copy link
Contributor

kalmarek commented Sep 15, 2023

TLDR: bug was in gens(::KleinPermGroup), fix as suggested;

There are also two fixes for botched deepcopy_internal I recommend applying.

Longer version if you wish...
julia> K = KleinPermGroup()
KleinPermGroup()

julia> Characters.CharacterTable(Rational{Int}, K)
ERROR: DomainError with 6:
6 is not a square modulo 13
Stacktrace:
 [1] sqrtmod(n::Int64, q::Int64)
   @ SymbolicWedderburn.Characters.FiniteFields ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:109
 [2] sqrt
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:103 [inlined]
 [3] normalize!(chtbl::CharacterTable{KleinPermGroup, SymbolicWedderburn.Characters.FiniteFields.GF{13}, Orbit{KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:141
 [4] CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{13}}, G::KleinPermGroup, cclasses::Vector{Orbit{KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:73
 [5] CharacterTable(R::Type{Rational{Int64}}, G::KleinPermGroup, cclasses::Vector{Orbit{KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:83
 [6] CharacterTable(R::Type{Rational{Int64}}, G::KleinPermGroup)
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:82
 [7] top-level scope
   @ REPL[4]:1

the error that we've seen.

julia> p = Characters.dixon_prime(K)
13

julia> Characters.CharacterTable(Characters.FiniteFields.GF{p}, K)
ERROR: DomainError with 6:
6 is not a square modulo 13
Stacktrace:
 [1] sqrtmod(n::Int64, q::Int64)
   @ SymbolicWedderburn.Characters.FiniteFields ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:109
 [2] sqrt
   @ ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/gf.jl:103 [inlined]
 [3] normalize!(chtbl::CharacterTable{KleinPermGroup, SymbolicWedderburn.Characters.FiniteFields.GF{13}, Orbit{KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:141
 [4] CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{13}}, G::KleinPermGroup, cclasses::Vector{Orbit{KleinPermElement}})
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:73
 [5] CharacterTable(Fp::Type{SymbolicWedderburn.Characters.FiniteFields.GF{13}}, G::KleinPermGroup)
   @ SymbolicWedderburn.Characters ~/.julia/packages/SymbolicWedderburn/hBhTJ/src/Characters/character_tables.jl:61
 [6] top-level scope
   @ REPL[6]:1

actually happens when computing over the finite field. Let's investigate further.
There are 15 groups of order 24:

julia> SWpath = joinpath(dirname(dirname(pathof(SymbolicWedderburn))),)
"/home/kalmar/.julia/packages/SymbolicWedderburn/hBhTJ/test"

julia> include(joinpath(SWpath, "test", "smallgroups.jl"));

julia> length(SmallPermGroups[24])
15

one of them should be isomorphic to our K. Which one? Let's gather some candidates

julia> K_ords = sort!(vec(map(g->order(Int, g), K))))
24-element Vector{Int64}:
 1
 2
 2
 2
 2
 2
 2
 2
 
 3
 4
 4
 4
 4
 4
 4

julia> candidates = [i for (i, G) in pairs(SmallPermGroups[24]) if
           sort!(map(g->order(Int, g), G)) == K_ords]
1-element Vector{Int64}:
 12

julia> K_perm = SmallPermGroups[24][first(candidates)]
Permutation group on 4 generators of order 24 generated by
 (1,2)(3,13)(4,8)(5,7)(6,9)(10,21)(11,20)(12,16)(14,18)(15,17)(19,24)(22,23)
 (1,3,9)(2,6,13)(4,11,23)(5,19,17)(7,15,24)(8,22,20)(10,18,12)(14,21,16)
 (1,4)(2,7)(3,10)(5,12)(6,14)(8,16)(9,17)(11,19)(13,20)(15,22)(18,23)(21,24)
 (1,5)(2,8)(3,11)(4,12)(6,15)(7,16)(9,18)(10,19)(13,21)(14,22)(17,23)(20,24)

Clearly isomorphism should preserve orders, and there's only one group with the same set of orders as K. As expected though we can compute its characters just fine over the same p:

julia> let p = p
           Characters.CharacterTable(Characters.FiniteFields.GF{p}, K_perm)
       end
Character table of PermGroup( (1,2)(3,13)(4,8)(5,7)(6,9)(10,21)(11,20)(12,16)(14,18)(15,17)(19,24)(22,23), (1,3,9)(2,6,13)(4,11,23)(5,19,17)(7,15,24)(8,22,20)(10,18,12)(14,21,16), (1,4)(2,7)(3,10)(5,12)(6,14)(8,16)(9,17)(11,19)(13,20)(15,22)(18,23)(21,24), (1,5)(2,8)(3,11)(4,12)(6,15)(7,16)(9,18)(10,19)(13,21)(14,22)(17,23)(20,24) ) over SymbolicWedderburn.Characters.FiniteFields.GF{13}
       1    2    3    4    5
───┬────────────────────────
χ₁ │ 2₁₃  0₁₃ 12₁₃  2₁₃  0₁₃
χ₂ │ 3₁₃  1₁₃  0₁₃ 12₁₃ 12₁₃
χ₃ │ 1₁₃  1₁₃  1₁₃  1₁₃  1₁₃
χ₄ │ 1₁₃ 12₁₃  1₁₃  1₁₃ 12₁₃
χ₅ │ 3₁₃ 12₁₃  0₁₃ 12₁₃  1₁₃

Ok, since we get a strange error while normalizing "characters" my guess is that something is wrong with conjugacy classes of K. Are the orders equal inside each class?

julia> [order.(Int, c) for c in Characters.conjugacy_classes(K)]
7-element Vector{Vector{Int64}}:
 [1]
 [2, 2, 2]
 [3, 3]
 [2, 2, 2]
 [2, 2, 2]
 [4, 4, 4, 4, 4, 4]
 [3, 3, 3, 3, 3, 3]

julia> [order.(Int, c) for c in Characters.conjugacy_classes(K_perm)]
5-element Vector{Vector{Int64}}:
 [1]
 [2, 2, 2, 2, 2, 2]
 [3, 3, 3, 3, 3, 3, 3, 3]
 [2, 2, 2]
 [4, 4, 4, 4, 4, 4]

yes they are, but it seems that some elements which should be conjugated are not?

Lets take a permutation representation of K acting on itself via right multiplication:

julia> S = let elts = collect(K)
           idict = Dict(e=>i for (i,e) in pairs(vec(elts)))

           map(gens(K)) do g
               Perm([idict[h*g] for h in vec(elts)])
           end
       end;

julia> PermGroup(S)
Permutation group on 2 generators generated by
 (1,3)(2,4)(5,6)(7,15)(8,16)(9,13)(10,14)(11,18)(12,17)(19,21)(20,22)(23,24)
 (1,4,5)(2,3,6)(7,16,23)(8,15,24)(9,18,20)(10,17,19)(11,13,22)(12,14,21)

julia> order(Int, ans)
6

only two generators and order 6! we have a solution:

julia> gens(K)
2-element Vector{KleinPermElement}:
 KleinPermElement((1,2), KleinElement(0))
 KleinPermElement((1,2,3), KleinElement(0))

these surely don't generate K :D


With

function GroupsCore.gens(::KleinPermGroup)
    Kid = one(KleinGroup())
    elts = [KleinPermElement(g, Kid) for g in __symmetric_grp_gens(3)]
    Pid = one(first(__symmetric_grp_gens(3)))
    append!(elts, KleinPermElement(Pid, k) for k in gens(KleinGroup()))
    return elts
end

I get

julia> Characters.CharacterTable(Rational{Int}, K)
Character table of KleinPermGroup() over Cyclotomics.Cyclotomic{Rational{Int64}, SparseArrays.SparseVector{Rational{Int64}, Int64}}
        1     2     3     4     5
───┬─────────────────────────────
χ₁ │ 2//1  0//1 -1//1  2//1  0//1
χ₂ │ 3//1  1//1  0//1 -1//1 -1//1
χ₃ │ 1//1  1//1  1//1  1//1  1//1
χ₄ │ 1//1 -1//1  1//1  1//1 -1//1
χ₅ │ 3//1 -1//1  0//1 -1//1  1//1

julia> Characters.CharacterTable(Rational{Int}, K_perm)
Character table of PermGroup( (1,2)(3,13)(4,8)(5,7)(6,9)(10,21)(11,20)(12,16)(14,18)(15,17)(19,24)(22,23), (1,3,9)(2,6,13)(4,11,23)(5,19,17)(7,15,24)(8,22,20)(10,18,12)(14,21,16), (1,4)(2,7)(3,10)(5,12)(6,14)(8,16)(9,17)(11,19)(13,20)(15,22)(18,23)(21,24), (1,5)(2,8)(3,11)(4,12)(6,15)(7,16)(9,18)(10,19)(13,21)(14,22)(17,23)(20,24) ) over Cyclotomics.Cyclotomic{Rational{Int64}, SparseArrays.SparseVector{Rational{Int64}, Int64}}
        1     2     3     4     5
───┬─────────────────────────────
χ₁ │ 2//1  0//1 -1//1  2//1  0//1
χ₂ │ 3//1  1//1  0//1 -1//1 -1//1
χ₃ │ 1//1  1//1  1//1  1//1  1//1
χ₄ │ 1//1 -1//1  1//1  1//1 -1//1
χ₅ │ 3//1 -1//1  0//1 -1//1  1//1

src/symmetry.jl Outdated
Comment on lines 98 to 115
symmetric_group(n) = PermutationGroups.PermGroup(PermutationGroups.Perm([2,1]), PermutationGroups.Perm([2:n;1]))
function PermutationGroups.gens(::KleinPermGroup)
els = [KleinPermElement(Perm(3), KleinElement(1))]
for g in gens(symmetric_group(3))
function PermutationGroups.gens(g::KleinPermGroup)
els = [one(g)]
for g in gens(G3)
Copy link
Contributor

Choose a reason for hiding this comment

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

@blegat this is where the bug was introduced;
The old version generated KleinPermGroup, since Sym(3) acts transively on non-trivial KleinElements, the new one doesn't since KleinElement(0) is fixed under Sym(3).

I just updated the function without thinking ;)

src/symmetry.jl Outdated Show resolved Hide resolved
src/symmetry.jl Outdated
Comment on lines 128 to 135
function Base.deepcopy_internal(g::KleinPermElement, dict::IdDict)
p = if haskey(dict, objectid(g.p))
dict[objectid(g.p)]
else
el, st = el_st
return KleinPermElement(P3[el[1]], KleinElement(el[2])), st
Base.deepcopy_internal(g.p, dict)
end
return KleinPermElement(p, g.k)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I've botched both deepcopy_internals. Here's the corrected version:

function Base.deepcopy_internal(g::KleinPermElement, dict::IdDict)
    haskey(dict, g) && return dict[g]

    h = KleinPermElement(Base.deepcopy_internal(g.p, dict), g.k)
    dict[g] = h
    return h
end

src/symmetry.jl Outdated
Comment on lines 231 to 238
function Base.deepcopy_internal(g::DirectSumElem, dict::IdDict)
h = if isbits(g.h)
g.h
elseif haskey(dict, objectid(g.h))
dict[objectid(g.h)]
else
Base.deepcopy_internal(g.h, dict)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

and again:

function Base.deepcopy_internal(g::DirectSumElem, dict::IdDict)
    haskey(dict, g) && return g
    h = if isbits(g.h)
        g.h
    elseif haskey(dict, g.h)
        dict[g.h]
    else
        Base.deepcopy_internal(g.h, dict)
    end
    k = if isbits(g.k)
        g.k
    elseif haskey(dict, g.k)
        dict[g.k]
    else
        Base.deepcopy_internal(g.k, dict)
    end
    new_g = DirectSumElem(h, k)
    dict[g] = new_g
    return new_g
end

blegat and others added 6 commits September 15, 2023 21:48
Co-authored-by: Marek Kaluba <marek.kaluba@kit.edu>
* sort out deepcopy_internal stuff

* move imports/exports away from symmetry.jl

* update action.jl to the rewrite of symmetry.jl
@blegat blegat merged commit 19bf2eb into master Sep 26, 2023
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants