Skip to content

Conversation

@Joseph-Edwards
Copy link
Collaborator

@Joseph-Edwards Joseph-Edwards commented Sep 2, 2025

This PR updates the package to use libsemigroups v3.

Presently, this is a draft and there is a lot more work to be done. In a few critical places, I have added errors of the form "<the thing you are trying to do> is not yet implemented". With this, running any of the test files should clearly highlight further changes that need to be made.

Some notable things still to be done:

  • A way to construct Congruence objects from FroidurePin objects.
  • A way to construct a FroidurePin objects from ToddCoxeter objects.
  • An analogue of the old Congruence::ntc function.
  • A way to quotient FroidurePin objects.

@james-d-mitchell
Copy link
Collaborator

Requires libsemigroups/libsemigroups#880 (and possibly a release to be made) to be merged first before this has any chance that the CI will pass.

@Joseph-Edwards
Copy link
Collaborator Author

I've been doing some digging about why the tests on master are failing.

Since the most recent release of GAP, gap-system/gap#6064 has been merged. The change that I think is impacting us is described in that PR:

The only change arising from this should be that the IsomorphismPermGroup method requiring IsGroup and IsFinite and IsHandledByNiceMonomorphism is now ranked below the method requiring IsCyclotomicMatrixGroup, and I think this is intended behaviour.

This is changing the behaviour of the following line that gets called at some point during the call to InjectionPrincipalFactor (the function in the failing test):

map := IsomorphismPermGroup(SchutzenbergerGroup(H));

In the example given in the test file, H has size 8.

Previously, using the same input as given in the test file, this line returned a map whose Range was a group of size 8. Now it returns a map whose Range is a group of size 24261120, but whose Image is a group of size 8. It's not clear to me if this is a bug or not; when an isomorphism is being defined by a map (in the computational sense, rather than a mathematical one) should it be required to be surjective onto its range, or only the image? The way the code is written and tested, it seems like the intention was the latter.

If this isn't a bug, then the following changes in acting.gi seem to resolve the issue:

 InstallMethod(IsomorphismPermGroup, "for H-class of an acting semigroup",
 [IsGreensHClass and IsActingSemigroupGreensClass],
 function(H)
   local map, id, iso, inv;
 
   if not IsGroupHClass(H) then
     ErrorNoReturn("the argument (a Green's H-class) is not a group");
   elif not IsPermGroup(SchutzenbergerGroup(H)) then
     map := IsomorphismPermGroup(SchutzenbergerGroup(H));
   else
     map := IdentityMapping(SchutzenbergerGroup(H));
   fi;
 
   id := ConvertToInternalElement(Parent(H), MultiplicativeNeutralElement(H));
 
   iso := function(x)
     if not x in H then
       ErrorNoReturn("the argument does not belong to the domain of the ",
                     "function");
     fi;
     x := ConvertToInternalElement(Parent(H), x);
     return LambdaPerm(Parent(H))(id, x) ^ map;
   end;
   inv := function(x)
     local S, act;
     if not x in Image(map) then
       ErrorNoReturn("the argument does not belong to the domain of the ",
                     "function");
     fi;
     S := Parent(H);
     act := StabilizerAction(S);
     return ConvertToExternalElement(S,
-                                    act(id, x ^ InverseGeneralMapping(map)));
+                                    act(id, x ^ RestrictedInverseGeneralMapping(map)));
   end;
-  return MappingByFunction(H, Range(map), iso, inv);
+  return MappingByFunction(H, Image(map), iso, inv);
 end);

This makes sure our inverse map satisfies IsTotal by restricting its source to the Image of map.

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.

3 participants