From e9a4ac4d54126d82046aa4baeffe765b01980c11 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Mon, 14 Oct 2024 16:03:06 +0200
Subject: [PATCH 01/19] Add syllable functions to pcgroup

Moved from the mjrodgers-OW_GModules branch.
---
 src/Groups/pcgroup.jl | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index 923170798e44..879e1b6d213d 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -365,3 +365,28 @@ function pc_group(c::GAP_Collector)
   end
 end
 
+function Oscar.syllables(g::Union{PcGroupElem, SubPcGroupElem})
+  l = GAPWrap.ExtRepOfObj(GapObj(g))
+  @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
+#Thomas
+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
+#Thomas
+function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}})
+  e = exponent_vector(sylls, ngens(G))
+  pcgs = Oscar.GAPWrap.FamilyPcgs(GapObj(G))
+  x = Oscar.GAPWrap.PcElementByExponentsNC(pcgs, GapObj(e, true))
+  return Oscar.group_element(G, x)
+end
\ No newline at end of file

From b9109237d9dfeb971059d7dbc6661831e714b2fc Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Thu, 17 Oct 2024 13:35:35 +0200
Subject: [PATCH 02/19] Add letters function for PcGroupElem

---
 src/Groups/pcgroup.jl | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index 879e1b6d213d..056a7bce9ec6 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -365,6 +365,35 @@ function pc_group(c::GAP_Collector)
   end
 end
 
+"""
+    letters(g::PcGroupElem)
+
+Return the letters of `g` as a list of integers, each entry corresponding to
+a group generator.
+
+# Examples
+```jldoctest
+julia> c = collector(2, Int);
+
+julia> Oscar.set_relative_orders!(c, [2, 3])
+
+julia> Oscar.set_conjugate!(c, 2, 1, [2 => 2])
+
+julia> gg = pc_group(c)
+Pc group of order 6
+
+julia> letters(gg[1]^5*gg[2]^-4)
+3-element Vector{Int64}:
+ 1
+ 2
+ 2
+```
+"""
+function letters(g::PcGroupElem)
+  w = GAPWrap.UnderlyingElement(GapObj(g))
+  return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
+end 
+
 function Oscar.syllables(g::Union{PcGroupElem, SubPcGroupElem})
   l = GAPWrap.ExtRepOfObj(GapObj(g))
   @assert iseven(length(l))

From d362f673f21fa4b035f83296d61db183b9277866 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Thu, 17 Oct 2024 13:36:09 +0200
Subject: [PATCH 03/19] Add testset for PcGroup letters function

---
 test/Groups/pcgroup.jl | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index 8ec1f2c3cd3c..dbbab0fa0b10 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -82,3 +82,17 @@ 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, 2]
+  @test letters(gg[1]^5*gg[2]^4) == [1, 2] # 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) == [] # both identity elements
+end
\ No newline at end of file

From 27c24e79d4d8caad27180840407c1ebc664e23da Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Thu, 17 Oct 2024 16:31:05 +0100
Subject: [PATCH 04/19] Add support for SubPcGroupElem in letters

---
 src/Groups/pcgroup.jl  | 4 ++--
 test/Groups/pcgroup.jl | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index 056a7bce9ec6..d6a1853396a9 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -366,7 +366,7 @@ function pc_group(c::GAP_Collector)
 end
 
 """
-    letters(g::PcGroupElem)
+    letters(g::Union{PcGroupElem, SubPcGroupElem})
 
 Return the letters of `g` as a list of integers, each entry corresponding to
 a group generator.
@@ -389,7 +389,7 @@ julia> letters(gg[1]^5*gg[2]^-4)
  2
 ```
 """
-function letters(g::PcGroupElem)
+function letters(g::Union{PcGroupElem, SubPcGroupElem})
   w = GAPWrap.UnderlyingElement(GapObj(g))
   return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
 end 
diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index dbbab0fa0b10..0fcdd0e49427 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -95,4 +95,11 @@ end
   @test letters(gg[1]^5*gg[2]^4) == [1, 2] # 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) == [] # 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
\ No newline at end of file

From 89b8fe61c113ea9d2eeac5561975a78ecead1346 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Thu, 17 Oct 2024 16:37:39 +0100
Subject: [PATCH 05/19] Cleanup code

---
 src/Groups/pcgroup.jl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index d6a1853396a9..a28661978842 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -394,7 +394,7 @@ function letters(g::Union{PcGroupElem, SubPcGroupElem})
   return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
 end 
 
-function Oscar.syllables(g::Union{PcGroupElem, SubPcGroupElem})
+function syllables(g::Union{PcGroupElem, SubPcGroupElem})
   l = GAPWrap.ExtRepOfObj(GapObj(g))
   @assert iseven(length(l))
   return Pair{Int, ZZRingElem}[l[i-1] => l[i] for i = 2:2:length(l)]
@@ -418,4 +418,4 @@ function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}})
   pcgs = Oscar.GAPWrap.FamilyPcgs(GapObj(G))
   x = Oscar.GAPWrap.PcElementByExponentsNC(pcgs, GapObj(e, true))
   return Oscar.group_element(G, x)
-end
\ No newline at end of file
+end

From 94cd03ed8ebbd43eeb96d492a4ed5c8215c259ed Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Fri, 18 Oct 2024 14:33:21 +0100
Subject: [PATCH 06/19] Rename pcgroup exponent_vector function

---
 src/Groups/pcgroup.jl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index a28661978842..fd6ab76bcba0 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -402,7 +402,7 @@ end
 
 # Convert syllables in canonical form into exponent vector
 #Thomas
-function exponent_vector(sylls::Vector{Pair{Int64, ZZRingElem}}, n)
+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 
@@ -414,7 +414,7 @@ end
 # Convert syllables in canonical form into group element
 #Thomas
 function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}})
-  e = exponent_vector(sylls, ngens(G))
+  e = _exponent_vector(sylls, ngens(G))
   pcgs = Oscar.GAPWrap.FamilyPcgs(GapObj(G))
   x = Oscar.GAPWrap.PcElementByExponentsNC(pcgs, GapObj(e, true))
   return Oscar.group_element(G, x)

From 416fd8bbc3f1fa53fd482259263dc17b0948562d Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Wed, 30 Oct 2024 22:30:31 +0100
Subject: [PATCH 07/19] Fix test cases for finite pcgroup

---
 test/Groups/pcgroup.jl | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index 0fcdd0e49427..6b294282d0ba 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -91,10 +91,10 @@ end
   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, 2]
-  @test letters(gg[1]^5*gg[2]^4) == [1, 2] # all positive exp
+  @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) == [] # both identity elements
+  @test letters(gg[1]^2*gg[2]^3) == [2] # both identity elements
 
   # finite polycyclic subgroup
   gg = pc_group(symmetric_group(4))
@@ -102,4 +102,4 @@ end
   @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
\ No newline at end of file
+end

From c350414538f5da744adb16c8df01b62dd9244138 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Thu, 31 Oct 2024 13:03:44 +0100
Subject: [PATCH 08/19] Add check for syllables in canonical form

---
 src/Groups/pcgroup.jl | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index fd6ab76bcba0..dcbca550e5a7 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -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)
@@ -413,7 +412,15 @@ end
 
 # Convert syllables in canonical form into group element
 #Thomas
-function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}})
+function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}}, check::Bool=true)
+  # check if the syllables are in canonical form
+  if check
+    indices = map(p -> p.first, sylls)
+    unq_indices = unique(indices) # maintains order
+    @req length(indices) == length(unq_indices) "given syllables have repeating generators"
+    @req issorted(unq_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))

From b552be689aa02c52217fe20196b67c1b442081b4 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Thu, 31 Oct 2024 13:06:22 +0100
Subject: [PATCH 09/19] Add testset for canonical form check

---
 test/Groups/pcgroup.jl | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index 6b294282d0ba..423b301b0560 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -103,3 +103,27 @@ end
   @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
\ No newline at end of file

From 11bdb258f63060e31cad275e6a4486b003d160e7 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Fri, 1 Nov 2024 13:50:58 +0100
Subject: [PATCH 10/19] Refactor based on PR comments

---
 src/Groups/pcgroup.jl  | 7 +++----
 test/Groups/pcgroup.jl | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index dcbca550e5a7..0271be16ddbc 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -412,13 +412,12 @@ end
 
 # Convert syllables in canonical form into group element
 #Thomas
-function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}}, check::Bool=true)
+function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}}; check::Bool=true)
   # check if the syllables are in canonical form
   if check
     indices = map(p -> p.first, sylls)
-    unq_indices = unique(indices) # maintains order
-    @req length(indices) == length(unq_indices) "given syllables have repeating generators"
-    @req issorted(unq_indices) "given syllables must be in ascending order"
+    @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))
diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index 423b301b0560..413d439ca419 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -126,4 +126,4 @@ end
 
   sylls = [2 => ZZ(1), 1 => ZZ(2), 1 => ZZ(3)] # both conditions
   @test_throws ArgumentError gg(sylls)
-end
\ No newline at end of file
+end

From 1b80584164f4730edd1a8fa6d5d60b0a4cc8cd97 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Fri, 1 Nov 2024 13:52:22 +0100
Subject: [PATCH 11/19] Document pcgroup syllable function

---
 src/Groups/pcgroup.jl | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index 0271be16ddbc..23bc82a36117 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -391,8 +391,31 @@ julia> letters(gg[1]^5*gg[2]^-4)
 function letters(g::Union{PcGroupElem, SubPcGroupElem})
   w = GAPWrap.UnderlyingElement(GapObj(g))
   return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
-end 
+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.
+
+# Examples
+```jldoctest
+julia> c = collector(2, Int);
+
+julia> Oscar.set_relative_orders!(c, [2, 3])
+
+julia> Oscar.set_conjugate!(c, 2, 1, [2 => 2])
+
+julia> gg = pc_group(c)
+Pc group of order 6
+
+julia> syllables(gg[1]^5*gg[2]^-4)
+2-element Vector{Pair{Int64, ZZRingElem}}:
+ 1 => 1
+ 2 => 2
+```
+"""
 function syllables(g::Union{PcGroupElem, SubPcGroupElem})
   l = GAPWrap.ExtRepOfObj(GapObj(g))
   @assert iseven(length(l))

From 2d0bfb858bd4b03102b1499c4081584f42b0ef38 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Fri, 15 Nov 2024 16:07:41 +0100
Subject: [PATCH 12/19] Improve letters and syllables documentation

---
 src/Groups/pcgroup.jl | 53 +++++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index 23bc82a36117..a5621b7cfc60 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -370,18 +370,37 @@ end
 Return the letters of `g` as a list of integers, each entry corresponding to
 a group generator.
 
-# Examples
+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".
+
+See also [`syllables`](@ref).
+
 ```jldoctest
-julia> c = collector(2, Int);
+julia> gg = small_group(6, 1)
+Pc group of order 6
 
-julia> Oscar.set_relative_orders!(c, [2, 3])
+julia> x = gg([1 => ZZ(-3)])
+f1^-3
 
-julia> Oscar.set_conjugate!(c, 2, 1, [2 => 2])
+julia> letters(x)
+3-element Vector{Int64}:
+ -1
+ -1
+ -1
+```
 
-julia> gg = pc_group(c)
+# Examples
+
+```jldoctest
+julia> gg = small_group(6, 1)
 Pc group of order 6
 
-julia> letters(gg[1]^5*gg[2]^-4)
+julia> x = gg[1]^5*gg[2]^-4
+f1*f2^2
+
+julia> letters(x)
 3-element Vector{Int64}:
  1
  2
@@ -401,20 +420,22 @@ a group generator and its exponent.
 
 # Examples
 ```jldoctest
-julia> c = collector(2, Int);
-
-julia> Oscar.set_relative_orders!(c, [2, 3])
-
-julia> Oscar.set_conjugate!(c, 2, 1, [2 => 2])
-
-julia> gg = pc_group(c)
+julia> gg = small_group(6, 1)
 Pc group of order 6
 
-julia> syllables(gg[1]^5*gg[2]^-4)
+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
 """
 function syllables(g::Union{PcGroupElem, SubPcGroupElem})
   l = GAPWrap.ExtRepOfObj(GapObj(g))
@@ -423,7 +444,6 @@ function syllables(g::Union{PcGroupElem, SubPcGroupElem})
 end
 
 # Convert syllables in canonical form into exponent vector
-#Thomas
 function _exponent_vector(sylls::Vector{Pair{Int64, ZZRingElem}}, n)
   res = zeros(ZZRingElem, n)
   for pair in sylls
@@ -434,7 +454,6 @@ function _exponent_vector(sylls::Vector{Pair{Int64, ZZRingElem}}, n)
 end
 
 # Convert syllables in canonical form into group element
-#Thomas
 function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}}; check::Bool=true)
   # check if the syllables are in canonical form
   if check

From e7f4da09f8f0ae871e5fa2b7e59ec68304fd3220 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Fri, 15 Nov 2024 16:12:08 +0100
Subject: [PATCH 13/19] Fix syllables doctest

---
 src/Groups/pcgroup.jl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index a5621b7cfc60..8de1eccf92e7 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -436,6 +436,7 @@ f1*f2^2
 
 julia> gg(s) == x
 true
+```
 """
 function syllables(g::Union{PcGroupElem, SubPcGroupElem})
   l = GAPWrap.ExtRepOfObj(GapObj(g))

From 43532ba0e130dc1cbb5d8d0f7fff4507fcdd5042 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Wed, 20 Nov 2024 22:34:13 +0100
Subject: [PATCH 14/19] Change documentation references

---
 src/Groups/pcgroup.jl | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index ab8a1f203dd2..ee7e9497029c 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -481,9 +481,11 @@ a group generator.
 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".
+For example, as shown below, an output of `-1` refers to the "inverse of the first generator".
 
-See also [`syllables`](@ref).
+See also [`syllables(::Union{PcGroupElem, SubPcGroupElem})`](@ref).
+
+# Examples
 
 ```jldoctest
 julia> gg = small_group(6, 1)
@@ -499,8 +501,6 @@ julia> letters(x)
  -1
 ```
 
-# Examples
-
 ```jldoctest
 julia> gg = small_group(6, 1)
 Pc group of order 6
@@ -527,6 +527,7 @@ Return the syllables of `g` as a list of pairs of integers, each entry correspon
 a group generator and its exponent.
 
 # Examples
+
 ```jldoctest
 julia> gg = small_group(6, 1)
 Pc group of order 6

From a80667bcd610d48251efc1600c6ce3596f6d29c2 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:02:18 +0100
Subject: [PATCH 15/19] Add checks for PcpGroup

---
 src/Groups/pcgroup.jl | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index ee7e9497029c..3808e30eb6a4 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -516,8 +516,20 @@ julia> letters(x)
 ```
 """
 function letters(g::Union{PcGroupElem, SubPcGroupElem})
-  w = GAPWrap.UnderlyingElement(GapObj(g))
-  return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
+  # check if we have a PcpGroup element
+  if GAPWrap.IsPcpElement(GapObj(g))
+    exp = GAPWrap.Exponents(GapObj(g))
+
+    # Should we check if the output is not larger than the
+    # amount of generators? Requires use of `parent`.
+    # @assert length(exp) == length(gens(parent(g)))
+
+    w = [sign(e) * i for (i, e) in enumerate(exp) for _ in 1:abs(e)]
+    return Vector{Int}(w)
+  else # finite PcGroup
+    w = GAPWrap.UnderlyingElement(GapObj(g))
+    return Vector{Int}(GAPWrap.LetterRepAssocWord(w))
+  end
 end
 
 """
@@ -548,7 +560,13 @@ true
 ```
 """
 function syllables(g::Union{PcGroupElem, SubPcGroupElem})
-  l = GAPWrap.ExtRepOfObj(GapObj(g))
+  # check if we have a PcpGroup element
+  if GAPWrap.IsPcpElement(GapObj(g))
+    l = GAPWrap.GenExpList(GapObj(g))
+  else # finite PcGroup
+    l = GAPWrap.ExtRepOfObj(GapObj(g))
+  end
+
   @assert iseven(length(l))
   return Pair{Int, ZZRingElem}[l[i-1] => l[i] for i = 2:2:length(l)]
 end
@@ -573,8 +591,17 @@ function (G::PcGroup)(sylls::Vector{Pair{Int64, ZZRingElem}}; check::Bool=true)
   end
 
   e = _exponent_vector(sylls, ngens(G))
-  pcgs = Oscar.GAPWrap.FamilyPcgs(GapObj(G))
-  x = Oscar.GAPWrap.PcElementByExponentsNC(pcgs, GapObj(e, true))
+
+  # check if G is an underlying PcpGroup
+  GG = GapObj(G)
+  if GAPWrap.IsPcpGroup(GG)
+    coll = GAPWrap.Collector(GG)
+    x = GAPWrap.PcpElementByExponentsNC(coll, GapObj(e, true))
+  else # finite PcGroup
+    pcgs = GAPWrap.FamilyPcgs(GG)
+    x = GAPWrap.PcElementByExponentsNC(pcgs, GapObj(e, true))
+  end
+  
   return Oscar.group_element(G, x)
 end
 

From 30a5a623d3a75b6b4e687b162c7b222abce55282 Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:03:00 +0100
Subject: [PATCH 16/19] Wrap used PcpGroup GAP functions

---
 src/GAP/wrappers.jl | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/GAP/wrappers.jl b/src/GAP/wrappers.jl
index d3013a214bae..015d5901d0dc 100644
--- a/src/GAP/wrappers.jl
+++ b/src/GAP/wrappers.jl
@@ -89,6 +89,7 @@ GAP.@wrap ElementsFamily(x::GapObj)::GapObj
 GAP.@wrap ELMS_LIST(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap Embedding(x::GapObj, y::Int)::GapObj
 GAP.@wrap EpimorphismSchurCover(x::GapObj)::GapObj
+GAP.@wrap Exponents(x::GapObj)::GapObj
 GAP.@wrap ExponentsOfPcElement(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap ExtRepOfObj(x::GapObj)::GapObj
 GAP.@wrap ExtRepPolynomialRatFun(x::GapObj)::GapObj
@@ -104,6 +105,7 @@ GAP.@wrap FusionConjugacyClasses(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap GaloisCyc(x::GAP.Obj, GapInt)::GAP.Obj
 GAP.@wrap GeneratorsOfField(x::GapObj)::GapObj
 GAP.@wrap GeneratorsOfGroup(x::GapObj)::GapObj
+GAP.@wrap GenExpList(x::GapObj)::GapObj
 GAP.@wrap GetFusionMap(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap GF(x::Any)::GapObj
 GAP.@wrap GF(x::Any, y::Any)::GapObj
@@ -221,6 +223,7 @@ GAP.@wrap IsomorphismFpGroupByPcgs(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap IsOne(x::Any)::Bool
 GAP.@wrap IsPcGroup(x::Any)::Bool
 GAP.@wrap IsPcpGroup(x::Any)::Bool
+GAP.@wrap IsPcpElement(x::Any)::Bool
 GAP.@wrap IsPerfectGroup(x::Any)::Bool
 GAP.@wrap IsPermGroup(x::Any)::Bool
 GAP.@wrap IsPGroup(x::Any)::Bool
@@ -304,6 +307,7 @@ GAP.@wrap OnTuples(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap Order(x::Any)::GapInt
 GAP.@wrap OrthogonalComponents(x::GapObj, y::GapObj, z::GapInt)::GapObj
 GAP.@wrap PcElementByExponentsNC(x::GapObj, y::GapObj)::GapObj
+GAP.@wrap PcpElementByExponentsNC(x::GapObj, y::GapObj)::GapObj
 GAP.@wrap Pcgs(x::GapObj)::GapObj
 GAP.@wrap PcpGroupByCollectorNC(x::GapObj)::GapObj
 GAP.@wrap PCore(x::GapObj, y::GapInt)::GapObj

From 2cd5cb952ffb7934f9ef28b4010c641eca71f72f Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:03:35 +0100
Subject: [PATCH 17/19] Clean up docs examples

---
 src/Groups/pcgroup.jl | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/Groups/pcgroup.jl b/src/Groups/pcgroup.jl
index 3808e30eb6a4..e68d5da9c298 100644
--- a/src/Groups/pcgroup.jl
+++ b/src/Groups/pcgroup.jl
@@ -488,17 +488,19 @@ See also [`syllables(::Union{PcGroupElem, SubPcGroupElem})`](@ref).
 # Examples
 
 ```jldoctest
-julia> gg = small_group(6, 1)
-Pc group of order 6
+julia> g = abelian_group(PcGroup, [0, 5])
+Pc group of infinite order
 
-julia> x = gg([1 => ZZ(-3)])
-f1^-3
+julia> x = g[1]^-3 * g[2]^-3
+g1^-3*g2^2
 
 julia> letters(x)
-3-element Vector{Int64}:
+5-element Vector{Int64}:
  -1
  -1
  -1
+  2
+  2
 ```
 
 ```jldoctest
@@ -558,6 +560,25 @@ f1*f2^2
 julia> gg(s) == x
 true
 ```
+
+```jldoctest
+julia> g = abelian_group(PcGroup, [5, 0])
+Pc group of infinite order
+
+julia> x = g[1]^-3 * g[2]^-3
+g1^2*g2^-3
+
+julia> s = syllables(x)
+2-element Vector{Pair{Int64, ZZRingElem}}:
+ 1 => 2
+ 2 => -3
+
+julia> g(s)
+g1^2*g2^-3
+
+julia> g(s) == x
+true
+```
 """
 function syllables(g::Union{PcGroupElem, SubPcGroupElem})
   # check if we have a PcpGroup element

From 5dd04b87785a9d4a8abdf7bd203aa273ded7a08c Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:04:11 +0100
Subject: [PATCH 18/19] Clean and add tests for infinite PcGroups

---
 test/Groups/pcgroup.jl | 67 +++++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 30 deletions(-)

diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index 170ba24ae564..ce6f1f0234b9 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -83,49 +83,56 @@ end
   @test cgg !== c.X
 end
 
-@testset "generate letters from polycyclic group element" begin
+@testset "create letters from polycyclic group elements" 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]
+  G = small_group(6, 1)
+  @test letters(G[1]^5*G[2]^-4) == [1, 2, 2]
+  @test letters(G[1]^5*G[2]^4) == [1, 2] # all positive exp
+  @test letters(G[1]^-5*G[2]^-7) == [1, 2, 2] # all negative exp
+  @test letters(G[1]^2*G[2]^3) == [] # both identity elements
+
+  # finite polycyclic subgroups
+  G = pc_group(symmetric_group(4))
+  H = derived_subgroup(G)[1]
+  @test letters(H[1]^2) == [2, 2]
+  @test letters(H[1]^2*H[2]^3*H[3]^3) == [2, 2, 3, 4] # all positive exp
+  @test letters(H[1]^-2*H[2]^-3*H[3]^-3) == [2, 3, 4] # all negative exp
+  @test letters(H[1]^3*H[2]^4*H[3]^2) == [] # all identity elements
+
+  # infinite polycyclic groups
+  G = abelian_group(PcGroup, [5, 0])
+  @test letters(G[1]^3) == [1, 1, 1]
+  @test letters(G[1]^4*G[2]^3) == [1, 1, 1, 1, 2, 2, 2] # all positive exp
+  @test letters(G[1]^-2*G[2]^-5) == [1, 1, 1, -2, -2, -2, -2, -2] # all negative exp
+  @test letters(G[1]^5*G[2]^-3) == [-2, -2, -2] # one identity element
 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)
+  G = small_group(6, 1)
 
-  element = gg[1]^5*gg[2]^-4
-  sylls = syllables(element)
+  x = G[1]^5*G[2]^-4
+  sylls = syllables(x)
   @test sylls == [1 => ZZ(1), 2 => ZZ(1)] # check general usage
-  @test gg(sylls) == element # this will pass the check
+  @test G(sylls) == x # check if equivalent
 
   sylls = [1 => ZZ(1), 2 => ZZ(2), 1 => ZZ(3)]
-  @test_throws ArgumentError gg(sylls) # repeating generators
+  @test_throws ArgumentError G(sylls) # repeating generators
 
   sylls = [2 => ZZ(1), 1 => ZZ(2)]
-  @test_throws ArgumentError gg(sylls) # not in ascending order
+  @test_throws ArgumentError G(sylls) # not in ascending order
+
+  sylls = [2 => ZZ(1), 1 => ZZ(2), 1 => ZZ(3)]
+  @test_throws ArgumentError G(sylls) # both conditions
+
+  # infinite polycyclic groups
+  G = abelian_group(PcGroup, [5, 0])
 
-  sylls = [2 => ZZ(1), 1 => ZZ(2), 1 => ZZ(3)] # both conditions
-  @test_throws ArgumentError gg(sylls)
+  x = G[1]^3*G[2]^-5
+  sylls = syllables(x)
+  @test sylls == [1 => ZZ(3), 2 => ZZ(-5)] # check general usage
+  @test G(sylls) == x # check if equivalent
 end
   
 @testset "create collectors from polycyclic groups" begin

From c07db3127cabbbcb6bfbf6712a91fb8bdf09c39f Mon Sep 17 00:00:00 2001
From: James Nohilly <184946269+jamesnohilly@users.noreply.github.com>
Date: Mon, 25 Nov 2024 16:04:46 +0100
Subject: [PATCH 19/19] Fix mistake in syllable test

---
 test/Groups/pcgroup.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/Groups/pcgroup.jl b/test/Groups/pcgroup.jl
index ce6f1f0234b9..18978e8ab859 100644
--- a/test/Groups/pcgroup.jl
+++ b/test/Groups/pcgroup.jl
@@ -114,7 +114,7 @@ end
 
   x = G[1]^5*G[2]^-4
   sylls = syllables(x)
-  @test sylls == [1 => ZZ(1), 2 => ZZ(1)] # check general usage
+  @test sylls == [1 => ZZ(1), 2 => ZZ(2)] # check general usage
   @test G(sylls) == x # check if equivalent
 
   sylls = [1 => ZZ(1), 2 => ZZ(2), 1 => ZZ(3)]