Problematic definition (property is already set the other way in GAP)

Question

I want to define a probabilistic measure for finite groups in GAP and it seems that I'm doing something wrong but I can't figure out what the mistake is. For example:

prob:=function(g)
local x, y, h, list;
list:=[];
  for x in g do
    for y in g do
    h:=Group(x, y);
      if IsNilpotent(h) then
      Append(list,[h]);
      fi;
    od;
od;
return Size(list)/(Size(g)^2);
end;

The displayed error message is:

Error, Value property is already set the other way in
  SetIsPGroup( G, false 
 ); at /Users/xxx/Downloads/gap-4.9.2/lib/grp.gi:405 called from 
IsNilpotent( h ) at *stdin*:7 called from
<function "prob">( <arguments> )
 called from read-eval loop at *stdin*:15
you can 'return;' to set it anyhow

At first, I thought there might be a problem with how I define h, but I have tried different variations and they all produce the same error message.

Might someone be able to advise?

Answer 1

Short answer:

This was a bug in GAP, which is already fixed in the latest GAP release. Please upgrade your GAP installation.

Long answer:

Your code is syntactically correct, although note that:

• to add one element to a list you can use Add(l,x) instead of Append(l,[x])

• you can use a counter instead of storing the whole list of subgroups, as you are only interested in the number of ordered pairs of elements that generate a nilpotent subgroup.

The actual problem here is more subtle, and it lies outside your code. I will try to outline how one could approach situations like these.

It's useful for a GAP user to understand how to use the break loop to investigate the source of the problem (see Chapters 6 and 7 of the GAP reference manual).

First of all, in this case one could observe from the backtrace (at /Users/xxx/Downloads/gap-4.9.2/lib/grp.gi:405 called from IsNilpotent( h )) that the error occurs not in the body of the prob function, but in IsNilpotent. First questions to think are: Is this function documented? Do you call it with the right kind of arguments? Do you correctly process its output? Since this is indeed the case (although IsNilpotent for a group is not documented, but is defined in lib/overload.g for convenience, and dispatches in this case to IsNilpotentGroup which IS documented), this is likely to be a bug in GAP.

If you think that this is a bug in GAP, the most suitable way to report it is to create an issue in the GAP source code repository on GitHub: https://github.com/gap-system/gap/issues/new. Creating it, please remember to include details necessary to check and reproduce your problem: Which version of GAP is used? Which GAP code to use to construct your example? Which output and/or error message do you observe? etc.

I have tried your code on a few groups in GAP 4.10.2 and can not reproduce this, but from your question I can see that you are using GAP 4.9.2. I was indeed able to reproduced this in GAP 4.9.3:

gap> prob(SmallGroup(60,3));
Error, Value property is already set the other way in
  SetIsPGroup( G, false ); at /Users/xxx/gap-4.9.3/lib/grp.gi:405 called from 
IsNilpotent( h ) at *stdin*:7 called from
<function "prob">( <arguments> )
 called from read-eval loop at *stdin*:17
you can 'return;' to set it anyhow
brk>

So, let's look at the code from gap-4.9.3/lib/grp.gi:405, referenced in the error message:

InstallMethod( IsNilpotentGroup,
    "if group size can be computed and is a prime power",
    [ IsGroup and CanComputeSize ], 25,
    function ( G )
    local s;

    s := Size ( G );
    if IsInt( s ) and IsPrimePowerInt( s ) then
        SetIsPGroup( G, true );
        SetPrimePGroup( G, SmallestRootInt( s ) );
        return true;
    else
        SetIsPGroup( G, false );
    fi;
    TryNextMethod();
    end );

We can inspect the value of G in the scope of this break loop:

brk> G;
<pc group of size 1 with 2 generators>

The value of its property IsPGroup is already known:

brk> HasIsPGroup(G);
true

and we can inspect its value:

brk> IsPGroup(G);
true

(I first called HasIsPGroup to demonstrate that the answer to IsPGroup is already known before, and stored in G, so IsPGroup(G) retrieves the stored answer, instead of determining it again).

Finally, what is the actual group?

brk> Size(G);
1

So, we have a special case of a trivial group, and here is the problem. In GAP 4.9, IsNilpotentGroup calls SetIsPGroup( G, false ), but at that time G has already stored value of the property IsPGroup as true. This is why GAP triggers an error to prevent changing the value of the property to its opposite. I suggest to refer to the Tutorial chapter on Operations and Methods for further details how setting and getting attributes and properties work.

To compare with the code from GAP 4.10.2, which properly handles this special case:

InstallMethod( IsNilpotentGroup,
    "if group size can be computed and is a prime power",
    [ IsGroup and CanComputeSize ], 25,
    function ( G )
    local s;

    s := Size ( G );
    if IsInt( s ) and IsPrimePowerInt( s ) then
        SetIsPGroup( G, true );
        SetPrimePGroup( G, SmallestRootInt( s ) );
        return true;
    elif s = 1 then
        SetIsPGroup( G, true );
        return true;
    elif s <> infinity then
        SetIsPGroup( G, false );
    fi;
    TryNextMethod();
    end );

Thus, both version of IsNilpotentGroup correctly report trivial group is nilpotent, but they also set the value of IsPGroup in the process, and the old version of the code was setting it wrong.

Using revisions history on GitHub, you can find where and why the change have been made in this commit, which belongs to this pull request.

GAP standard test suite also now has a test that check that an error message (which is also now more clear) is produced in the following scenario:

gap> SetIsPGroup(G, false);
gap> SetIsPGroup(G, true);
Error, property is already set the other way

P.S. You can further improve the performance of your code by storing the value of the output of prob(g) as an attribute of a group (see this episode of the GAP Carpentries-style lesson). This will allow to retrieve it from the stored value next time it is used, instead of recomputing it again.