LieAlgebras: Change normal form for WeylGroupElem; cleanup after #4374

experimental/AlgebraicShifting/src/PartialShiftGraph.jl

@@ -165,10 +165,10 @@ julia> collect(edges(G))
 
 julia> EL[6, 5]
 4-element Vector{WeylGroupElem}:
- s1 * s2
  s2
- s3 * s1 * s2
+ s1 * s2
  s3 * s2
+ s1 * s3 * s2
 
 julia> facets.(VL[[6, 5]])
 2-element Vector{Vector{Set{Int64}}}:

experimental/AlgebraicShifting/test/runtests.jl

@@ -14,6 +14,6 @@
     all_shifts = partial_shift_graph_vertices(QQ, U, W)
     directed_graph, edge_labels = partial_shift_graph(QQ, all_shifts)
     @test collect(edges(directed_graph)) ==  [Edge(t...) for t in [[2, 1], [3, 1], [3, 2]]]
-    @test word.(edge_labels[2, 1]) == word.([s[1], s[1] * s[2], s[1] * s[2] * s[1], s[2] * s[1]])
+    @test issetequal(word.(edge_labels[2, 1]), word.([s[1], s[2] * s[1], s[1] * s[2] * s[1], s[1] * s[2]]))
   end
 end

experimental/LieAlgebras/src/Types.jl

@@ -177,8 +177,8 @@ struct WeylGroupElem <: AbstractAlgebra.GroupElem
 
     @req all(1 <= i <= ngens(W) for i in word) "word contains invalid generators"
     x = new(W, sizehint!(UInt8[], length(word)))
-    for s in Iterators.reverse(word)
-      lmul!(x, s)
+    for s in word
+      rmul!(x, s)
     end
 
     return x

experimental/LieAlgebras/src/WeightLattice.jl

@@ -358,9 +358,9 @@ function conjugate_dominant_weight_with_elem!(w::WeightLatticeElem)
     end
   end
 
-  # reversing word means it is in short revlex normal form
+  # word is already in short lex normal form
   # and it is the element taking original w to new w
-  return w, weyl_group(root_system(w))(word; normalize=true) # TODO: is normalize=true necessary?
+  return w, weyl_group(root_system(w))(word; normalize=false)
 end
 
 function dot(w1::WeightLatticeElem, w2::WeightLatticeElem)

experimental/LieAlgebras/src/WeylGroup.jl

@@ -67,7 +67,7 @@
 
 The word must be a list of integers, where each integer is the index of a simple reflection.
 
-If the word is known to be in reduced form, the normalization can be skipped by setting `normalize=false`.
+If the word is known to be in short lex normal form, the normalization can be skipped by setting `normalize=false`.
 """
 function (W::WeylGroup)(word::Vector{<:Integer}; normalize::Bool=true)
   return WeylGroupElem(W, word; normalize)
@@ -239,11 +239,11 @@
 # Weyl group elements
 
 function Base.:*(x::WeylGroupElem, y::WeylGroupElem)
-  @req x.parent === y.parent "$x, $y must belong to the same Weyl group"
+  @req parent(x) === parent(y) "parent mismatch"
 
-  p = deepcopy(y)
-  for s in Iterators.reverse(word(x))
-    lmul!(p, s)
+  p = deepcopy(x)
+  for s in word(y)
+    rmul!(p, s)
   end
   return p
 end
@@ -281,8 +281,8 @@
 
   px = deepcopy(x)
   for _ in 2:n
-    for s in Iterators.reverse(word(x))
-      lmul!(px, s)
+    for s in word(x)
+      rmul!(px, s)
     end
   end
 
@@ -306,16 +306,16 @@
   end
 
   tx = deepcopy(x)
-  for i in 1:length(y)
-    b, j, _ = explain_lmul(tx, y[i])
+  for i in length(y):-1:1
+    b, j, _ = explain_rmul(tx, y[i])
     if !b
       deleteat!(word(tx), j)
       if isone(tx)
         return true
       end
     end
 
-    if length(tx) > length(y) - i
+    if length(tx) > i - 1
       return false
     end
   end
@@ -356,8 +356,8 @@
 
 function Base.inv(x::WeylGroupElem)
   y = parent(x)(sizehint!(UInt8[], length(x)); normalize=false)
-  for s in word(x)
-    lmul!(y, s)
+  for s in Iterators.reverse(word(x))
+    rmul!(y, s)
   end
   return y
 end
@@ -410,21 +410,21 @@
 end
 
 @doc raw"""
-    lmul(x::WeylGroupElem, i::Integer) -> WeylGroupElem
+    rmul(x::WeylGroupElem, i::Integer) -> WeylGroupElem
 
-Return the result of multiplying `x` from the left by the `i`-th simple reflection.
+Return the result of multiplying `x` from the right by the `i`-th simple reflection.
 """
-function lmul(x::WeylGroupElem, i::Integer)
-  return lmul!(deepcopy(x), i)
+function rmul(x::WeylGroupElem, i::Integer)
+  return rmul!(deepcopy(x), i)
 end
 
 @doc raw"""
-    lmul!(x::WeylGroupElem, i::Integer) -> WeylGroupElem
+    rmul!(x::WeylGroupElem, i::Integer) -> WeylGroupElem
 
-Multiply `x` in-place from the left by the `i`-th simple reflection, and return the result.
+Multiply `x` in-place from the right by the `i`-th simple reflection, and return the result.
 """
-function lmul!(x::WeylGroupElem, i::Integer)
-  b, j, r = explain_lmul(x, i)
+function rmul!(x::WeylGroupElem, i::Integer)
+  b, j, r = explain_rmul(x, i)
   if b
     insert!(word(x), j, r)
   else
@@ -434,17 +434,17 @@
   return x
 end
 
-# explains what multiplication of s_i from the left will do.
+# explains what multiplication of s_i from the right will do.
 # Return a tuple where the first entry is true/false, depending on whether an insertion or deletion will happen,
 # the second entry is the position, and the third is the simple root.
-function explain_lmul(x::WeylGroupElem, i::Integer)
+function explain_rmul(x::WeylGroupElem, i::Integer)
   @req 1 <= i <= rank(root_system(parent(x))) "Invalid generator"
 
-  insert_index = 1
+  insert_index = length(x) + 1
   insert_letter = UInt8(i)
 
   root = insert_letter
-  for s in 1:length(x)
+  for s in length(x):-1:1
     if x[s] == root
       return false, s, x[s]
     end
@@ -458,7 +458,7 @@
     # check if we have a better insertion point now. Since word[i] is a simple
     # root, if root < word[i] it must be simple.
     if root < x[s]
-      insert_index = s + 1
+      insert_index = s
       insert_letter = UInt8(root)
     end
   end
@@ -486,13 +486,13 @@
 
 julia> collect(reduced_expressions(x))
 3-element Vector{Vector{UInt8}}:
- [0x01, 0x02, 0x03, 0x01]
  [0x01, 0x02, 0x01, 0x03]
+ [0x01, 0x02, 0x03, 0x01]
  [0x02, 0x01, 0x02, 0x03]
 
 julia> collect(reduced_expressions(x; up_to_commutation=true))
 2-element Vector{Vector{UInt8}}:
- [0x01, 0x02, 0x03, 0x01]
+ [0x01, 0x02, 0x01, 0x03]
  [0x02, 0x01, 0x02, 0x03]
 ```
 The second expression of the first iterator is not contained in the second iterator
@@ -556,7 +556,7 @@
   end
 
   isoinv = function (g::FPGroupElem)
-    return W(abs.(letters(g)))
+    return W(abs.(letters(g))) # TODO: check if normalize=false can be added here (probably not)
   end
 
   return MapFromFunc(W, G, iso, isoinv)
@@ -602,7 +602,7 @@
           )
         end
       end
-      return W(word)
+      return W(word) # TODO: check if normalize=false can be added here
     end
   else
     error("Not implemented (yet)")
@@ -638,9 +638,9 @@
 
   # we need to copy word; iterate behaves differently when length is (not) known
   next = deepcopy(word)
-  weight = reflect!(weyl_vector(root_system(parent(iter.el))), Int(next[1]))
+  weight = reflect!(weyl_vector(root_system(parent(iter.el))), Int(next[end]))
 
-  i = 1
+  i = length(next)
   s = rk + 1
   while true
     # search for new simple reflection to add to the word
@@ -649,10 +649,10 @@
     end
 
     if s == rk + 1
-      i += 1
-      if i == length(next) + 1
+      i -= 1
+      if i == 0
         return nothing
-      elseif i == 1
+      elseif i == length(next)
         return next, next
       end
 
@@ -662,16 +662,16 @@
       s += 1
     else
       if iter.up_to_commutation &&
-        i < length(word) &&
-        s < next[i + 1] &&
-        is_zero_entry(cartan_matrix(root_system(parent(iter.el))), s, Int(next[i + 1]))
+        i > 1 &&
+        s < next[i - 1] &&
+        is_zero_entry(cartan_matrix(root_system(parent(iter.el))), s, Int(next[i - 1]))
         s += 1
         continue
       end
 
       next[i] = UInt8(s)
       reflect!(weight, s)
-      i -= 1
+      i += 1
       s = 1
     end
   end
@@ -698,22 +698,19 @@
   return state, state
 end
 
-# based on [Ste01], 4.C and 4.D
 function Base.iterate(iter::WeylIteratorNoCopy, state::WeylIteratorNoCopyState)
   state = _iterate_nocopy(state)
   if isnothing(state)
     return nothing
   end
-  # return state, state
-  # TODO: change internals of iterator to return (wt, x) with wt*x == iter.weight instead of the hack below
-  wt, x = state
-  return (wt, inv(x)), state
+  return state, state
 end
 
+# based on [Ste01], 4.C and 4.D
 function _iterate_nocopy(state::WeylIteratorNoCopyState)
   wt, path = state[1], word(state[2])
 
-  ai = isempty(path) ? UInt8(0) : path[end]
+  ai = isempty(path) ? UInt8(0) : first(path)
   # compute next descendant index
   di = UInt8(0)
   while true
@@ -724,12 +721,12 @@
       return nothing
     elseif iszero(di)
       reflect!(wt, Int(ai))
-      di = pop!(path)
-      ai = isempty(path) ? UInt8(0) : path[end]
+      di = popfirst!(path)
+      ai = isempty(path) ? UInt8(0) : first(path)
     end
   end
 
-  push!(path, di)
+  pushfirst!(path, di)
   reflect!(wt, Int(di))
   return state
 end

experimental/LieAlgebras/src/exports.jl

@@ -76,7 +76,6 @@ export is_simple_root
 export is_simple_root_with_index
 export killing_matrix
 export lie_algebra
-export lmul, lmul!
 export longest_element
 export lower_central_series
 export matrix_repr_basis
@@ -93,6 +92,7 @@ export positive_root
 export positive_roots
 export reduced_expressions
 export reflect, reflect!
+export rmul, rmul!
 export root_system, has_root_system
 export root_system_type, has_root_system_type
 export root_system_type_with_ordering

experimental/LieAlgebras/test/WeylGroup-test.jl

@@ -10,10 +10,10 @@ include(
     return word(parent(x)(word(x))) == word(x)
   end
 
-  b3_w0 = UInt8[3, 2, 3, 1, 2, 3, 1, 2, 1]
-  b4_w0 = UInt8[4, 3, 4, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 1, 2, 1]
-  f4_w0 = UInt8[4, 3, 2, 3, 1, 2, 3, 4, 3, 2, 3, 1, 2, 3, 4, 3, 2, 3, 1, 2, 3, 1, 2, 1]
-  g2_w0 = UInt8[2, 1, 2, 1, 2, 1]
+  b3_w0 = UInt8[1, 2, 1, 3, 2, 1, 3, 2, 3]
+  b4_w0 = UInt8[1, 2, 1, 3, 2, 1, 4, 3, 2, 1, 4, 3, 2, 4, 3, 4]
+  f4_w0 = UInt8[1, 2, 1, 3, 2, 1, 3, 2, 3, 4, 3, 2, 1, 3, 2, 3, 4, 3, 2, 1, 3, 2, 3, 4]
+  g2_w0 = UInt8[1, 2, 1, 2, 1, 2]
 
   @testset "weyl_group(::ZZMatrix)" begin
     W = weyl_group(cartan_matrix(:A, 2))
@@ -121,6 +121,7 @@ include(
               @test v * w == inv(iso)(iso(v) * iso(w))
             end
             g = rand_pseudo(G)
+            @test is_in_normal_form(inv(iso)(g))
             @test g == iso(inv(iso)(g))
             h = rand_pseudo(G)
             @test inv(iso)(h * g) == inv(iso)(h) * inv(iso)(g)
@@ -159,6 +160,7 @@ include(
                   @test v * w == inv(iso)(iso(v) * iso(w))
                 end
                 g = rand_pseudo(G)
+                @test is_in_normal_form(inv(iso)(g))
                 @test g == iso(inv(iso)(g))
                 h = rand_pseudo(G)
                 @test inv(iso)(h * g) == inv(iso)(h) * inv(iso)(g)
@@ -276,7 +278,7 @@ include(
     W = weyl_group(:B, 2)
     w0 = longest_element(W)
     @test is_in_normal_form(w0)
-    @test word(w0) == UInt8[2, 1, 2, 1]
+    @test word(w0) == UInt8[1, 2, 1, 2]
 
     # B3
     W = weyl_group(:B, 3)
@@ -327,8 +329,8 @@ include(
     s = gens(W)
     @test parent(s[1] * s[2]) === parent(s[1]) === parent(s[2])
 
-    @test word(s[3] * s[1]) == UInt8[3, 1]
-    @test word(s[1] * s[3]) == UInt8[3, 1]
+    @test word(s[3] * s[1]) == UInt8[1, 3]
+    @test word(s[1] * s[3]) == UInt8[1, 3]
     @test word(s[1] * s[3] * s[1]) == UInt8[3]
     @test word(s[3] * s[1] * s[3]) == UInt8[1]
     @test word(s[1] * s[2] * s[1]) == UInt8[1, 2, 1]
@@ -445,7 +447,7 @@ include(
     re = collect(iter)
     @test length(re) == 16
     @test re[1] == word(w0)
-    @test re[16] == UInt8[3, 2, 1, 3, 2, 3]
+    @test re[16] == UInt8[3, 2, 3, 1, 2, 3]
 
     iter = reduced_expressions(w0; up_to_commutation=true)
     @test iter.el === w0
@@ -454,7 +456,7 @@ include(
     re = collect(iter)
     @test length(re) == 8
     @test re[1] == word(w0)
-    @test re[8] == UInt8[3, 2, 3, 1, 2, 3]
+    @test re[8] == UInt8[3, 2, 1, 3, 2, 3]
   end
 
   @testset "WeylIteratorNoCopy" begin