gap-packages / polycyclic

Computation with polycyclic groups
https://gap-packages.github.io/polycyclic/
Other
4 stars 9 forks source link

PreImagesSet does not work if subgroup is not a subset of image #47

Open stertooy opened 4 years ago

stertooy commented 4 years ago

The following error occurs when using PreImage on a subgroup that is not an element of the image:

gap> G := AbelianPcpGroup([0]);
Pcp-group with orders [ 0 ]
gap> phi := GroupHomomorphismByImages(G,G,[G.1],[One(G)]);
[ g1 ] -> [ id ]
gap> PreImage(phi,G);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 4th choice method found for `PreImagesSet' on 2 arguments at /usr/lib/gap/lib/methsel2.g:249 called from
PreImagesSet( map, img ) at /usr/lib/gap/lib/mapping.gi:320 called from
<function "PreImage">( <arguments> )

The problem seems to be that the implementations of PreImagesSet cannot deal with PreImagesRepresentative returning fail.

prei := List( Igs( U ), function ( x )
        return PreImagesRepresentative( hom, x );
    end );
if fail in prei then
    TryNextMethod();
fi;
genpreimages := GeneratorsOfMagmaWithInverses( elms );
if Length( genpreimages ) > 0 and CanEasilyCompareElements( genpreimages[1] ) then
    genpreimages := Filtered( genpreimages, function ( i )
            return i <> One( i );
        end );
fi;
genpreimages := List( genpreimages, function ( gen )
        return PreImagesRepresentative( map, gen );
    end );
if fail in genpreimages then
    TryNextMethod();
fi;

I believe that before #41, PreImagesSet relied on the (incorrect) behaviour of PreImagesRepresentative, namely that it would return id instead of fail. Perhaps replacing the fail's in the implementations of PreImagesSet by the identity or throwing them away alltogether would work?

For now, a quick workaround is to use PreImage( hom, Intersection( Image( hom ), U ) ) rather than PreImage( hom, U ), but I suspect this is quite inefficient.

stertooy commented 4 years ago

Of interest: in this issue, it is mentioned that perhaps #39 was not an issue after all and PreImagesRepresentative is, in fact, not meant to return fail if the argument does not lie in the image of the homomorphism. In that case, perhaps #41 could just be reverted, which should(?) fix this issue.