Skip to content

Retry kernel recognition for missing generators #519

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.

Sign up for GitHub

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

Open
fingolfin wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Retry kernel recognition for missing generators #519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 44 additions & 25 deletions gap/base/recognition.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,33 +691,52 @@ InstallGlobalFunction( RecogniseGeneric,
depth,").");
done := ImmediateVerification(ri);
fi;
until done;

# Set nice generators and CalcStdPresentation of ri
SetNiceGens(ri,Concatenation(pregensfac(ri), NiceGens(riker)));
if not HasStdPresentation(ri) and not HasCalcStdPresentation(ri) then
SetCalcStdPresentation(ri,CalcStdPresentationGenericNonLeaf);
fi;
if done then
# Set nice generators and CalcStdPresentation of ri
SetNiceGens(ri,Concatenation(pregensfac(ri), NiceGens(riker)));
if not HasStdPresentation(ri) and not HasCalcStdPresentation(ri) then
SetCalcStdPresentation(ri,CalcStdPresentationGenericNonLeaf);
fi;

# Check that all (input) generators of H lie in the group generated by
# the nice generators.
# One could argue that this step should only be performed if verification is desired,
# but [BHLO15, 5.2 (6)] suggests to always do this.
# In any case, it should be relatively cheap.
# Note that without this step, the success of CalcStdPresentation is not sufficient
# to guarantee verification, even if this is not clear from [BHLO15, 4.3, 4.5, 5.3.1].
for h in GeneratorsOfGroup( H ) do
if SLPforElement(ri, h) = fail then
# NiceGens(ri) is too small, abort
if InfoLevel(InfoRecog) = 1 and depth = 0 then Print("\n"); fi;
Info(
InfoRecog, 1,
"RecogNode <ri> could not be recognised,",
" IsReady(<ri>) is not set, recognition aborts"
);
return ri;
# Check that all (input) generators of H lie in the group generated
# by the nice generators. If a generator reveals an additional
# kernel element, add it and retry kernel recognition.
for i in [1..Length(GeneratorsOfGroup( H ))] do
h := GeneratorsOfGroup( H )[i];
if SLPforElement(ri, h) = fail then
x := ImageElm(Homom(ri), h);

@fingolfin fingolfin Jul 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hrm, I guess if this returns fail at this point, we are in deep troubles, and ought to abort. Actually, if we did everything right before getting here, shouldn't this be effectively impossible? Hrm.

@TWiedemann TWiedemann Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think x could potentially be fail if KernelRecogNode(ImageRecogNode(ri)) is too small. In any case, I agree that we ought to abort in this situation (which is what the code currently does).

@TWiedemann TWiedemann Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Actually, my statement that the code currently aborts in this situation is not entirely correct. I think it does not abort in the following situation:

  • The for loop finds some h with SLPforElement(ri, h) = fail, and x := ImageElm(Homom(ri), h) is not fail for this h, and s := SLPforElement(ImageRecogNode(ri), x) is also not fail. This results in done being set to false.
  • In a later iteration of the for loop, we find some h with SLPforElement(ri, h) = fail, but this time x := ImageElm(Homom(ri), h) is fail. Since done is still false from a previous iteration, the code does not abort.

if x <> fail then
s := SLPforElement(ImageRecogNode(ri), x);
else
s := fail;
fi;
if s <> fail then
y := ResultOfStraightLineProgram(s, ri!.pregensfacwithmem);
z := ri!.gensHmem[i] * y^-1;
if not isone(ri)(z) and
not ForAny(gensN(ri), x -> isequal(ri)(x, z)) then
Add(gensN(ri), z);
Info(InfoRecog, 2,
"Final verification found extra kernel ",
"element (depth=", depth, ").");
done := false;
fi;
fi;
if done then
# NiceGens(ri) is too small, abort
if InfoLevel(InfoRecog) = 1 and depth = 0 then
Print("\n");
fi;
Info(InfoRecog, 1,
"RecogNode <ri> could not be recognised,",
" IsReady(<ri>) is not set, recognition aborts");
return ri;
fi;
Comment on lines +708 to +734

@TWiedemann TWiedemann Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if x <> fail then
s := SLPforElement(ImageRecogNode(ri), x);
else
s := fail;
fi;
if s <> fail then
y := ResultOfStraightLineProgram(s, ri!.pregensfacwithmem);
z := ri!.gensHmem[i] * y^-1;
if not isone(ri)(z) and
not ForAny(gensN(ri), x -> isequal(ri)(x, z)) then
Add(gensN(ri), z);
Info(InfoRecog, 2,
"Final verification found extra kernel ",
"element (depth=", depth, ").");
done := false;
fi;
fi;
if done then
# NiceGens(ri) is too small, abort
if InfoLevel(InfoRecog) = 1 and depth = 0 then
Print("\n");
fi;
Info(InfoRecog, 1,
"RecogNode <ri> could not be recognised,",
" IsReady(<ri>) is not set, recognition aborts");
return ri;
fi;
abort := false;
if x <> fail then
s := SLPforElement(ImageRecogNode(ri), x);
else
s := fail;
abort := true;
fi;
if s <> fail then
y := ResultOfStraightLineProgram(s, ri!.pregensfacwithmem);
z := ri!.gensHmem[i] * y^-1;
if not isone(ri)(z) and
not ForAny(gensN(ri), x -> isequal(ri)(x, z)) then
Add(gensN(ri), z);
Info(InfoRecog, 2,
"Final verification found extra kernel ",
"element (depth=", depth, ").");
done := false;
else
# This should not be possible: If z is one or already in gensN(ri),
# then we should have been able to find an SLP for h in the first place.
abort := true;
fi;
fi;
if abort then
# NiceGens(ri) is too small, abort
if InfoLevel(InfoRecog) = 1 and depth = 0 then
Print("\n");
fi;
Info(InfoRecog, 1,
"RecogNode <ri> could not be recognised,",
" IsReady(<ri>) is not set, recognition aborts");
return ri;
fi;

I think this change makes the code abort as it should.

break;
fi;
od;
fi;
od;
until done;

if InfoLevel(InfoRecog) = 1 and depth = 0 then Print("\n"); fi;
# StopStoringRandEls(ri);
Expand Down
12 changes: 12 additions & 0 deletions tst/working/quick/bugfix.tst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,18 @@ gap> Size(ri);
gap> IsCorrect(ri);
true

# The top-level GoProjective kernel recognition can initially sample only a
# proper subgroup of the scalar kernel. Final generator verification must retry
# the kernel after adding the missing residual kernel element.
gap> seed:=234;; Reset(GlobalMersenneTwister,seed);; Reset(GlobalRandomSource,seed);;
gap> h := GL(19,5);;
gap> g := GroupWithGenerators(ProductReplacer(h)!.team);;
gap> ri := RECOG.TestGroup(g,false,Size(h));;
gap> IsReady(ri);
true
gap> Size(ri) = Size(GL(19,5));
true

#
gap> SetInfoLevel(InfoRecog, oldInfoLevel);
gap> STOP_TEST("bugfix.tst");