Don't use One on collections that are not domains

gap-system · Oct 21, 2022 · 7a8266a · 7a8266a
1 parent 76a5b6d
commit 7a8266a
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 11 deletions.
diff --git a/lib/monoid.gi b/lib/monoid.gi
@@ -69,21 +69,22 @@ InstallMethod( ViewString,
 InstallOtherMethod(MonoidByGenerators, "for a collection",
 [IsCollection],
 function(gens)
-  local M, pos;
+  local M, pos, one;
 
   M := Objectify(NewType(FamilyObj(gens), IsMonoid
                                           and IsAttributeStoringRep), rec());
   gens := AsList(gens);
 
   if CanEasilyCompareElements(gens) and IsFinite(gens)
       and IsMultiplicativeElementWithOneCollection(gens) then
-    SetOne(M, One(gens));
-    pos := Position(gens, One(gens));
+    one := One(Representative(gens));
+    SetOne(M, one);
+    pos := Position(gens, one);
     if pos <> fail then
       SetGeneratorsOfMagma(M, gens);
-      if Length(gens) = 1 then # Length(gens) <> 0 since One(gens) in gens
+      if Length(gens) = 1 then # Length(gens) <> 0 since one in gens
         SetIsTrivial(M, true);
-      elif not IsPartialPermCollection(gens) or One(gens) =
+      elif not IsPartialPermCollection(gens) or one =
           One(gens{Concatenation([1 .. pos - 1], [pos + 1 .. Length(gens)])}) then
         # if gens = [PartialPerm([1,2]), PartialPerm([1])], then removing the One
         # = gens[1] from this, it is not possible to recreate the semigroup using
@@ -93,7 +94,7 @@ function(gens)
         Remove(gens, pos);
       fi;
     else
-      SetGeneratorsOfMagma(M, Concatenation([One(gens)], gens));
+      SetGeneratorsOfMagma(M, Concatenation([one], gens));
     fi;
   fi;
 

diff --git a/lib/semigrp.gi b/lib/semigrp.gi
@@ -492,7 +492,7 @@ InstallMethod(SemigroupByGenerators,
 "for a collection",
 [IsCollection],
 function(gens)
-  local S, pos;
+  local S, pos, one;
 
   S := Objectify(NewType(FamilyObj(gens), IsSemigroup
                                              and IsAttributeStoringRep), rec());
@@ -502,12 +502,13 @@ function(gens)
   if IsMultiplicativeElementWithOneCollection(gens)
       and CanEasilyCompareElements(gens)
       and IsFinite(gens) then
-    pos := Position(gens, One(gens));
+    one := One(Representative(gens));
+    pos := Position(gens, one);
     if pos <> fail then
       SetFilterObj(S, IsMonoid);
-      if Length(gens) = 1 then # Length(gens) <> 0 since One(gens) in gens
+      if Length(gens) = 1 then # Length(gens) <> 0 since one in gens
         SetIsTrivial(S, true);
-      elif not IsPartialPermCollection(gens) or One(gens) =
+      elif not IsPartialPermCollection(gens) or one =
         One(gens{Concatenation([1 .. pos - 1], [pos + 1 .. Length(gens)])}) then
         # if gens = [PartialPerm([1, 2]), PartialPerm([1])], then removing the One
         # = gens[1] from this, it is not possible to recreate the semigroup using

diff --git a/tst/testinstall/semipperm.tst b/tst/testinstall/semipperm.tst
@@ -289,7 +289,9 @@ true
 # Test IsomorphismPartialPermMonoid for a partial perm semigroup
 gap> S := Semigroup(PartialPerm([2], [2]), PartialPerm([1], [1]));;
 gap> IsomorphismPartialPermMonoid(S);
-Error, the argument must be a semigroup with a multiplicative neutral element
+MappingByFunction( <partial perm monoid of rank 2 with 2 generators>, 
+<partial perm monoid of rank 2 with 2 generators>
+ , function( object ) ... end, function( object ) ... end )
 gap> S := Semigroup(PartialPerm([2, 1]));;
 gap> IsomorphismPartialPermMonoid(S);;
 gap> IsMonoid(Range(last));