Fix IsTransitive to require that the group actually acts on the given domain (strictly speaking, the previous behavior was "as documented", but it made no sense and led to bugs elsewhere)

doc/ref/grpoper.xml

@@ -345,6 +345,12 @@ A <E>block system</E> (system of imprimitivity) for the action of a group
 is a partition of <M>\Omega</M> which &ndash;as a partition&ndash;
 remains invariant under the action of <M>G</M>.
 
+For operations concerning block systems, &GAP; assumes that <M>G</M> acts
+transitively on <M>\Omega</M>
+(see <Ref Oper="IsTransitive" Label="for a group, an action domain, etc."/>).
+One may get wrong results or error messages (perhaps at a much later stage)
+if this condition is not satisfied.
+
 <#Include Label="Blocks">
 <#Include Label="MaximalBlocks">
 <#Include Label="RepresentativesMinimalBlocks">

lib/oprt.gd

@@ -897,6 +897,8 @@ end );
 ## permutation equivalence, that is the permutation image of a group element
 ## is given by the positions of points in <A>Omega</A>.)
 ## <P/>
+## The result is undefined if <A>G</A> does not act on <A>Omega</A>.
+## <P/>
 ## By default the homomorphism returned by
 ## <Ref Func="ActionHomomorphism" Label="for a group, an action domain, etc."/>
 ## is not necessarily surjective (its 
@@ -1213,6 +1215,8 @@ DeclareGlobalFunction( "Action" );
 ## described in <Ref Sect="Domains"/> and <Ref Chap="Collections"/>, or (to use
 ## less memory but with a slower performance) an enumerator 
 ## (see <Ref Attr="Enumerator"/> ) of this domain.
+## <P/>
+## The result is undefined if <A>G</A> does not act on <A>Omega</A>.
 ## <Example><![CDATA[
 ## gap> g:=Group((1,2,3),(2,3,4));;
 ## gap> e:=ExternalSet(g,[1..4]);
@@ -1252,15 +1256,17 @@ DeclareOperation("RestrictedExternalSet",[IsExternalSet,IsGroup]);
 
 #############################################################################
 ##
-#O ExternalSubset(<G>,<xset>,<start>,[<gens>,<acts>,]<act>)
+#O ExternalSubset(<G>,<Omega>,<start>,[<gens>,<acts>,]<act>)
 ##
 ## <#GAPDoc Label="ExternalSubset">
 ## <ManSection>
-## <Oper Name="ExternalSubset" Arg='G,xset,start,[gens,acts,]act'/>
+## <Oper Name="ExternalSubset" Arg='G,Omega,start,[gens,acts,]act'/>
 ##
 ## <Description>
-## constructs the external subset of <A>xset</A> on the union of orbits of the
-## points in <A>start</A>.
+## constructs the external subset of <A>Omega</A> on the union of orbits of
+## the points in <A>start</A>.
+## <P/>
+## The result is undefined if <A>G</A> does not act on <A>Omega</A>.
 ## </Description>
 ## </ManSection>
 ## <#/GAPDoc>
@@ -1283,6 +1289,8 @@ OrbitishFO( "ExternalSubset",
 ## <Description>
 ## constructs the external subset on the orbit of <A>pnt</A>. The
 ## <Ref Attr="Representative"/> value of this external set is <A>pnt</A>.
+## <P/>
+## The result is undefined if <A>G</A> does not act on <A>Omega</A>.
 ## <Example><![CDATA[
 ## gap> e:=ExternalOrbit(g,g,(1,2,3));
 ## (1,2,3)^G
@@ -1663,12 +1671,15 @@ OrbitsishOperation( "Transitivity", OrbitsishReq, false, NewAttribute );
 ## Label="for an external set"/>
 ##
 ## <Description>
-## computes a block system for the action.
+## computes a block system for the transitive (see
+## <Ref Oper="IsTransitive" Label="for a group, an action domain, etc."/>)
+## action of <A>G</A> on <A>Omega</A>.
 ## If <A>seed</A> is not given and the action is imprimitive,
 ## a minimal nontrivial block system will be found.
 ## If <A>seed</A> is given, a block system in which <A>seed</A>
 ## is the subset of one block is computed.
-## The action must be transitive.
+## <P/>
+## The result is undefined if the action is not transitive.
 ## <Example><![CDATA[
 ## gap> g:=TransitiveGroup(8,3);
 ## E(8)=2[x]2[x]2
@@ -1708,9 +1719,13 @@ OrbitishFO( "Blocks",
 ##
 ## <Description>
 ## returns a block system that is maximal (i.e., blocks are maximal with
-## respect to inclusion) for the action of <A>G</A> on <A>Omega</A>.
+## respect to inclusion) for the transitive (see
+## <Ref Oper="IsTransitive" Label="for a group, an action domain, etc."/>)
+## action of <A>G</A> on <A>Omega</A>.
 ## If <A>seed</A> is given, a block system is computed in which <A>seed</A>
 ## is a subset of one block.
+## <P/>
+## The result is undefined if the action is not transitive.
 ## <Example><![CDATA[
 ## gap> MaximalBlocks(g,[1..8]);
 ## [ [ 1, 2, 3, 8 ], [ 4 .. 7 ] ]
@@ -1748,7 +1763,11 @@ OrbitishFO( "MaximalBlocks",
 ## <Description>
 ## computes a list of block representatives for all minimal (i.e blocks are
 ## minimal with respect to inclusion) nontrivial block systems for the
-## action. 
+## transitive (see
+## <Ref Oper="IsTransitive" Label="for a group, an action domain, etc."/>)
+## action of <A>G</A> on <A>Omega</A>.
+## <P/>
+## The result is undefined if the action is not transitive.
 ## <Example><![CDATA[
 ## gap> RepresentativesMinimalBlocks(g,[1..8]);
 ## [ [ 1, 2 ], [ 1, 3 ], [ 1, 4 ], [ 1, 5 ], [ 1, 6 ], [ 1, 7 ], 
@@ -1814,7 +1833,8 @@ OrbitsishOperation( "Earns", OrbitsishReq, false, NewAttribute );
 ## <P/>
 ## <Index>transitive</Index>
 ## We say that a group <A>G</A> acts <E>transitively</E> on a domain
-## <M>D</M> if and only if for every pair of points <M>d, e \in D</M>
+## <M>D</M> if and only if <A>G</A> acts on <M>D</M> and for every pair of
+## points <M>d, e \in D</M>
 ## there is an element <M>g</M> in <A>G</A> such that <M>d^g = e</M>.
 ## <P/>
 ## For a permutation group <A>G</A>, one may also invoke this as
@@ -1823,6 +1843,21 @@ OrbitsishOperation( "Earns", OrbitsishReq, false, NewAttribute );
 ## moved by it.
 ## For example the group <M>\langle (2,3,4),(2,3) \rangle</M>
 ## is transitive on the set <M>\{2, 3, 4\}</M>.
+## <Example><![CDATA[
+## gap> G:= Group( (2,3,4), (2,3) );;
+## gap> IsTransitive( G, [ 2 .. 4 ] );
+## true
+## gap> IsTransitive( G, [ 2, 3 ] ); # G does not act on [ 2, 3 ]
+## false
+## gap> IsTransitive( G, [ 1 .. 4 ] ); # G has two orbits on [ 1 .. 4 ]
+## false
+## gap> IsTransitive( G ); # G is transitive on [ 2 .. 4 ]
+## true
+## gap> IsTransitive( SL(2, 3), NormedRowVectors( GF(3)^2 ) );
+## false
+## gap> IsTransitive( SL(2, 3), NormedRowVectors( GF(3)^2 ), OnLines );
+## true
+## ]]></Example>
 ## </Description>
 ## </ManSection>
 ## <#/GAPDoc>
@@ -1851,7 +1886,9 @@ OrbitsishOperation( "IsTransitive", OrbitsishReq, false, NewProperty );
 ## or <K>false</K> otherwise.
 ## <P/>
 ## <Index>primitive</Index>
-## An action is <E>primitive</E> if it is transitive and the action admits
+## An action is <E>primitive</E> if it is transitive (see
+## <Ref Oper="IsTransitive" Label="for a group, an action domain, etc."/>)
+## and the action admits
 ## no nontrivial block systems. See&nbsp;<Ref Sect="Block Systems"/> for
 ## the definition of block systems.
 ## <P/>
@@ -1997,8 +2034,10 @@ OrbitsishOperation( "IsRegular", OrbitsishReq, false, NewProperty );
 ## Label="for an external set"/>
 ##
 ## <Description>
-## returns the rank of a transitive action, i.e. the number of orbits of
-## the point stabilizer.
+## returns the rank of the transitive (see
+## <Ref Oper="IsTransitive" Label="for a group, an action domain, etc."/>)
+## action of <A>G</A> on <A>Omega</A>, i. e., the number of orbits of
+## any point stabilizer.
 ## <Example><![CDATA[
 ## gap> RankAction(g,Combinations([1..4],2),OnSets);
 ## 4

lib/oprt.gi

@@ -2663,12 +2663,16 @@ end);
 ##
 #F IsTransitive( <G>, <D>, <gens>, <acts>, <act> ) . . . . transitivity test
 ##
+## We cannot assume that <G> acts on <D>.
+## Thus it is in general not sufficient to check whether <D> is a subset of
+## the <G>-orbit of a point in <D>, or whether <D> and this orbit have the
+## same size.
+##
 InstallMethod( IsTransitive,
  "compare with orbit of element",
- true,
- OrbitsishReq, 0,
+ OrbitsishReq,
 function( G, D, gens, acts, act )
- return Length(D)=0 or IsSubset( OrbitOp( G, D[ 1 ], gens, acts, act ), D );
+ return Length(D)=0 or IsEqualSet( OrbitOp( G, D[1], gens, acts, act ), D );
 end );
 
 

tst/testinstall/action.tst

@@ -0,0 +1,83 @@
+gap> START_TEST( "action.tst" );
+
+# The following session documents what happens currently
+# if one specifies "group actions" that are in fact not actions.
+# (When some of these tests fail then parts of the documentation
+# may have to be changed.)
+
+# Define an intransitive group.
+gap> G:= Group( (1,2), (3,4,5) );;
+
+#
+gap> RankAction( G ); # error, good
+Error, RankAction: action must be transitive
+gap> RankAction( G, [ 2 .. 5 ] ); # error, good
+Error, RankAction: action must be transitive
+gap> RankAction( G, [ 1 .. 6 ] ); # error, good
+Error, RankAction: action must be transitive
+gap> RankAction( G, [ 1 .. 5 ] ); # error, good
+Error, RankAction: action must be transitive
+gap> RankAction( G, [ 2 .. 6 ] ); # error, good
+Error, RankAction: action must be transitive
+
+#
+gap> Blocks( G, [ 2 .. 5 ] ); # error, good
+Error, <G> must operate transitively on <D>
+gap> Blocks( G, [ 1 .. 6 ] ); # error, good
+Error, <G> must operate transitively on <D>
+gap> Blocks( G, [ 1 .. 5 ] );; # works although not transitive
+gap> bl:= Blocks( G, [ 2 .. 6 ] );; # works although no action
+gap> Action( G, bl, OnSets ); # error, good (but late)
+Error, List Element: <list>[1] must have an assigned value
+
+#
+gap> MaximalBlocks( G, [ 2 .. 5 ] ); # error, good
+Error, <G> must operate transitively on <D>
+gap> MaximalBlocks( G, [ 1 .. 6 ] ); # error, good
+Error, <G> must operate transitively on <D>
+gap> MaximalBlocks( G, [ 1 .. 5 ] );; # works although not transitive
+gap> bl:= MaximalBlocks( G, [ 2 .. 6 ] );; # works although no action
+gap> Action( G, bl, OnSets ); # error, good (but late)
+Error, List Element: <list>[1] must have an assigned value
+
+#
+gap> bl:= RepresentativesMinimalBlocks( G, [ 2 .. 5 ] ); # error, good
+Error, <G> must act transitively on <D>
+gap> bl:= RepresentativesMinimalBlocks( G, [ 1 .. 6 ] ); # error, good
+Error, <G> must act transitively on <D>
+gap> RepresentativesMinimalBlocks( G, [ 1 .. 5 ] );; # works although not transitive
+gap> bl:= RepresentativesMinimalBlocks( G, [ 2 .. 6 ] );; # works although no action
+gap> Action( G, bl, OnSets );
+Error, List Element: <list>[1] must have an assigned value
+
+#
+gap> xset:= ExternalSet( G, [ 2 .. 5 ] );; # works although no action
+gap> Elements( xset );; # works
+gap> Action( xset );; # error, good (but late)
+Error, no method found! For debugging hints type ?Recovery from NoMethodFound
+Error, no 1st choice method found for `GroupByGenerators' on 2 arguments
+
+#
+gap> xset:= ExternalOrbit( G, [ 2 .. 5 ], 2 );; # works although no action
+gap> Elements( xset ); # error, good (but late)
+Error, no method found! For debugging hints type ?Recovery from NoMethodFound
+Error, no 1st choice method found for `[]' on 2 arguments
+The 2nd argument is 'fail' which might point to an earlier problem
+
+
+#
+gap> xset:= ExternalSubset( G, [ 2 .. 5 ], [ 2 ] );; # works (although no action)
+gap> Elements( xset );; # error, good (but late)
+Error, no method found! For debugging hints type ?Recovery from NoMethodFound
+Error, no 1st choice method found for `[]' on 2 arguments
+The 2nd argument is 'fail' which might point to an earlier problem
+
+
+#
+gap> hom:= ActionHomomorphism( G, [ 2 .. 5 ] );; # works (although no action)
+gap> Image( hom ); # error, good (but late)
+Error, no method found! For debugging hints type ?Recovery from NoMethodFound
+Error, no 1st choice method found for `GroupByGenerators' on 2 arguments
+
+#
+gap> STOP_TEST( "action.tst" );

tst/testinstall/oprt.tst

@@ -38,4 +38,9 @@ gap> IsTransitive(eo);
 true
 gap> Blocks(eo);
 [ [ 1, 5, 9 ], [ 2, 6, 10 ], [ 3, 7, 11 ], [ 4, 8, 12 ] ]
-gap> STOP_TEST( "oprt.tst", 1);
+gap> G:= Group( (2,3,4), (2,3) );;
+gap> IsTransitive( G, [ 2, 3 ] );
+false
+gap> Transitivity( G, [ 2, 3 ] );
+0
+gap> STOP_TEST( "oprt.tst" );