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

Add letters function for PcGroupElem #4202

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 103 additions & 1 deletion src/Groups/pcgroup.jl
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ function _GAP_collector_from_the_left(c::GAP_Collector)
return cGAP::GapObj
end


# Create the collector on the GAP side on demand
function underlying_gap_object(c::GAP_Collector)
if ! isdefined(c, :X)
Expand Down Expand Up @@ -365,3 +364,106 @@ function pc_group(c::GAP_Collector)
end
end

"""
letters(g::Union{PcGroupElem, SubPcGroupElem})

Return the letters of `g` as a list of integers, each entry corresponding to
a group generator.
Copy link
Member

@fingolfin fingolfin Nov 13, 2024

Choose a reason for hiding this comment

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

Note that we can also produce negative numbers: e.g. -3 means "inverse of 3rd generator". This should be explained, and perhaps an example added showing that. E.g. based on this:

julia> x = (gg[1]*gg[2]*gg[3])^-2
g1*g2^-2*g3^3

Perhaps also add something like this (and then mirror it in the other function)

See also [`syllables`](@ref).

Copy link
Author

Choose a reason for hiding this comment

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

I have added a small example with some brief explanation to letters for this. However I am unsure if the example is good as I was not able to get elements with negative exponents and test.

Copy link
Member

Choose a reason for hiding this comment

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

For that you need an infinite group. E.g.

julia> g = dihedral_group(PosInf())
Pc group of infinite order

julia> g[1]^-3 * g[2]^-3
g1*g2^-3

or

julia> g = abelian_group(PcGroup, [5, 0])
Pc group of infinite order

julia> g[1]^-3 * g[2]^-3
g1^2*g2^-3


This method can produce letters represented by negative numbers. A negative number
indicates the inverse of the generator at the corresponding positive index.

For example, as shown below, an output of -1 refers to the "inverse of the first generator".
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For example, as shown below, an output of -1 refers to the "inverse of the first generator".
For example, as shown below, an output of ´-1´ refers to the "inverse of the first generator".


See also [`syllables`](@ref).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
See also [`syllables`](@ref).
See also [`syllables(::Union{PcGroupElem, SubPcGroupElem})`](@ref).

otherwise this may link to any function called syllables, e.g. to syllables(::FPGroupElem) which is not what we want here


```jldoctest
julia> gg = small_group(6, 1)
Pc group of order 6

julia> x = gg([1 => ZZ(-3)])
f1^-3

julia> letters(x)
3-element Vector{Int64}:
-1
-1
-1
```

# Examples
Copy link
Member

Choose a reason for hiding this comment

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

This heading should be above the first code block, not between two code blocks


```jldoctest
julia> gg = small_group(6, 1)
Pc group of order 6

julia> x = gg[1]^5*gg[2]^-4
f1*f2^2

julia> letters(x)
3-element Vector{Int64}:
1
2
2
```
"""
function letters(g::Union{PcGroupElem, SubPcGroupElem})
w = GAPWrap.UnderlyingElement(GapObj(g))
return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
end

"""
syllables(g::Union{PcGroupElem, SubPcGroupElem})

Return the syllables of `g` as a list of pairs of integers, each entry corresponding to
a group generator and its exponent.
fingolfin marked this conversation as resolved.
Show resolved Hide resolved

# Examples
```jldoctest
julia> gg = small_group(6, 1)
Pc group of order 6

julia> x = gg[1]^5*gg[2]^-4
f1*f2^2

julia> s = syllables(x)
2-element Vector{Pair{Int64, ZZRingElem}}:
1 => 1
2 => 2

julia> gg(s)
f1*f2^2

julia> gg(s) == x
true
"""
lgoettgens marked this conversation as resolved.
Show resolved Hide resolved
function syllables(g::Union{PcGroupElem, SubPcGroupElem})
l = GAPWrap.ExtRepOfObj(GapObj(g))
Copy link
Member

Choose a reason for hiding this comment

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

For infinite groups, GAp uses a completely different internal representation. You'll need something like this:

  if GAP.Globals.IsPcpElement(GapObj(g))
     expvec = GAP.Globals.Exponents(GapObj(g))
     ... do something with it....
  else
    ... current code
  end

@assert iseven(length(l))
return Pair{Int, ZZRingElem}[l[i-1] => l[i] for i = 2:2:length(l)]
end

# Convert syllables in canonical form into exponent vector
function _exponent_vector(sylls::Vector{Pair{Int64, ZZRingElem}}, n)
res = zeros(ZZRingElem, n)
for pair in sylls
@assert res[pair.first] == 0 #just to make sure
res[pair.first] = pair.second
end
return res
end

# Convert syllables in canonical form into group element
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}}; check::Bool=true)
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add a similar constructor which takes an exponent vector, i.e., an inverse to letters?

Copy link
Member

Choose a reason for hiding this comment

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

One needs to watch out for the semantic difference to the already existing function for FPGroups in

function (G::FPGroup)(extrep::AbstractVector{T}) where T <: IntegerUnion
, which expects a flattened list of syllable pairs instead.

Copy link
Member

Choose a reason for hiding this comment

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

ugh, OK. perhaps we should kill that (is it documented?) first then. In GAP it made some sense to use such a flat list to avoid memory, as there are no tuples in GAP, only lists. But in Julia there is no real benefit of this over a Vector{Pair{Int64, ZZRingElem}}.

But that is way beyond this PR. So let's leave out the constructor I mentioned.

Copy link
Member

Choose a reason for hiding this comment

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

No, not documented, but used for serialization. I haven't looked into how it is used there, so maybe we can just adapt the deserialization function, in the worst case it needs an upgrade script.

Copy link
Member

Choose a reason for hiding this comment

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

ugh, OK. perhaps we should kill that (is it documented?) first then.

maybe @ThomasBreuer can look into that?

# check if the syllables are in canonical form
if check
indices = map(p -> p.first, sylls)
@req allunique(indices) "given syllables have repeating generators"
@req issorted(indices) "given syllables must be in ascending order"
end

e = _exponent_vector(sylls, ngens(G))
pcgs = Oscar.GAPWrap.FamilyPcgs(GapObj(G))
x = Oscar.GAPWrap.PcElementByExponentsNC(pcgs, GapObj(e, true))
Copy link
Member

Choose a reason for hiding this comment

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

Here you also need to check for if GAPWrap.IsPcGroup(GapObj(G)) and then do something different.

You'll need the function PcpElementByExponents or PcpElementByExponentsNC, see https://gap-packages.github.io/polycyclic/doc/chap4.html#X7882F0F57ABEB680

Also GAP.Globals.Collector(GapObj(G))

return Oscar.group_element(G, x)
end
45 changes: 45 additions & 0 deletions test/Groups/pcgroup.jl
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,48 @@ end
@test GAP.Globals.IsMutable(cgg)
@test cgg !== c.X
end

@testset "generate letters from polycyclic group element" begin

# finite polycyclic groups
c = collector(2, Int);
set_relative_order!(c, 1, 2)
set_relative_order!(c, 2, 3)
set_power!(c, 1, [2 => 1])
gg = pc_group(c)
@test letters(gg[1]^5*gg[2]^-4) == [1, 2]
@test letters(gg[1]^5*gg[2]^4) == [1] # all positive exp
@test letters(gg[1]^-5*gg[2]^-7) == [1, 2, 2] # all negative exp
@test letters(gg[1]^2*gg[2]^3) == [2] # both identity elements

# finite polycyclic subgroup
gg = pc_group(symmetric_group(4))
G = derived_subgroup(gg)[1]
@test letters(G[1]^2) == [2, 2]
@test letters(G[1]^2*G[2]^3*G[3]^3) == [2, 2, 3, 4]
@test letters(G[1]^-2*G[2]^-3*G[3]^-3) == [2, 3, 4]
end

@testset "create polycyclic group element from syllables" begin

# finite polycyclic groups
c = collector(2, Int);
set_relative_order!(c, 1, 2)
set_relative_order!(c, 2, 3)
set_power!(c, 1, [2 => 1])
gg = pc_group(c)

element = gg[1]^5*gg[2]^-4
sylls = syllables(element)
@test sylls == [1 => ZZ(1), 2 => ZZ(1)] # check general usage
@test gg(sylls) == element # this will pass the check

sylls = [1 => ZZ(1), 2 => ZZ(2), 1 => ZZ(3)]
@test_throws ArgumentError gg(sylls) # repeating generators

sylls = [2 => ZZ(1), 1 => ZZ(2)]
@test_throws ArgumentError gg(sylls) # not in ascending order

sylls = [2 => ZZ(1), 1 => ZZ(2), 1 => ZZ(3)] # both conditions
@test_throws ArgumentError gg(sylls)
end