OnTuples([-1], (1,2)) returns [-1]

[james-d-mitchell]

I get:

 gap> OnTuples([-1], (1,2));
 [ -1 ]

which contradicts the manual's assertion that:

Let  `tup`  be  a  list. `OnTuples` returns the list formed by the images of all
points  `x` of `tup` via the action function `OnPoints` (41.2-1), applied to `x` and
`g`.

Since:

gap> OnPoints(-1, (1,2));
Error, Perm. Operations: <point> must be a positive integer (not -1)
not in any function at line 5 of *stdin*
you can replace <point> via 'return <point>;'
brk>

This is also not consistent with the behaviour of OnSets:

gap> OnSets([-2, -1], (1,2));
Error, OnSets: <set> must be a set (not a list (plain,cyc,nsort))
not in any function at line 7 of *stdin*
you can replace <set> via 'return <set>;'
brk>

This arises from the following lines in the function OnTuplesPerm in permutat.c:

if ( k <= 0 || k > lmp )
    tmp = *ptTup;

[markuspf]

On Fri, Apr 08, 2016 at 07:42:33AM -0700, james-d-mitchell wrote:

I get:

gap> OnTuples([-1], (1,2));
[ -1 ]

which contradicts the manual's assertion that:

Let tup be a list. OnTuples returns the list formed by the images of all
points x of tup via the action function OnPoints (41.2-1), applied to x and
g.

This seems to indeed be a bug indeed caused by

This arises from the following lines in the function OnTuplesPerm in
permutat.c:

if ( k <= 0 || k > lmp )
tmp = *ptTup;

(this should be an error)

Since:

gap> OnPoints([-1], (1,2));
Error, Perm. Operations: must be a positive integer (not -1)
not in any function at line 5 of stdin
you can replace via 'return ;'
brk>

You'd want OnPoints(-1, (1,2,)) here though, wouldn't you? And this
errors out (consistently with the manual) since you're trying to act
on somtehing that is not a positive integer.

This is also not consistent with the behaviour of OnSets:

gap> OnSets([-1, -2], (1,2));
Error, OnSets: must be a set (not a list (plain,cyc,nsort))
not in any function at line 7 of stdin
you can replace via 'return ;'
brk>

[-1,-2] is not a set. If you use OnSets(AsSet([-1,-2]), (1,2)) you also get
behaviour consistent with the manual.

[markuspf]

Added bonus in permutat.c OnTuplesPerm is of course that k is an UInt. Testing an UInt for being less than zero is a bit pointless.

[james-d-mitchell]

Oops, I've just corrected the types you pointed, out, it should have been (and now is)

OnPoints(-1, (1,2));

and

OnSets([-2, -1], (1,2));

Both of these give an error, which is what I meant.

[markuspf]

Entering a break loop when trying to act on negative points seems mildly hilarious too, after all the code might have done all kinds of assumptions or computations based on the fact that there was a negative entry there...