gap-system / gap Goto Github PK

This is not related to the today's merge from the master branch - I've started to observe this last week. I am just creating this issue as a reminder for myself to look.

The Windows version of HPC-GAP is compiled under Cygwin. We were able to build it in the past, but then it became unstable in July 2014 - segfault happened straight after start. Now, since the morning March 19th the test fails with another symptoms:

+ bin/gap.sh -r -A -S tst/testinstall.g
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/cmdledit.g line 297
 READLINEINITLINE(ShallowCopy(str));
                 ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/cmdledit.g line 301
 BINDKEYSTOGAPHANDLER(ShallowCopy(str));
                     ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/cmdledit.g line 311
   BindKeysToGAPHandler(a);
                       ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/cmdledit.g line 316
   ReadlineInitLine(a);
                   ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/cmdledit.g line 322
 ReadlineInitLine(Concatenation("\"", seq, "\": \"", subs, "\""));
                 ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/actor.g line 72
 RunAsyncTask(ACTOR_LOOP, actor_state);
             ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/error.g line 192
   if QUITTING or not BreakOnError then
                                      ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/session.g line 62
   BreakOnError := false;
                 ^
Syntax error: warning: unbound global variable in /mnt/disk2/opt/hudson-\
slave/workspace/GAP-HPC-compilers/GAPABI/32/GAPGC/boehm/GAPGMP/gmp/label\
/32bit/GAP-HPC-snapshot/lib/session.g line 93
   BreakOnError := false;
                 ^
/tmp/hudson14346.sh: line 37:  8647 Segmentation fault      bin/gap.sh -r -A -S tst/testinstall.g > testinstalllog.txt
Build step 'Execute shell' marked build as failure

MakeImmutable breaks the backwards compatibility of HPCGAP with GAP4:

gap> LoadPackage( "MatricesForHomalg" );
true
gap> ZZ := HomalgRingOfIntegers( );
Z
gap> m := HomalgInitialMatrix( 2, 2, ZZ );
<An initial 2 x 2 matrix over an internal ring>
gap> InfoOfObject( m );
rec( attributes := rec( NrColumns := 2, NrRows := 2 ), components := rec( ring := Z ),
  object := <An initial 2 x 2 matrix over an internal ring>, properties := rec( IsEmptyMatrix := false, IsInitialMatrix := true ) )
gap> MakeImmutable( m );
Error, MakeImmutable: Argument has inaccessible subobjects
not in any function at line 5 of stream

I want to shorten filenames from the GAP standard library in backtraces.

The development version of GAP now prints out filename and lines in backtraces. For example, given a file t.g in the current directory containing f := function() Stabilizer(AlternatingGroup(5), [1.0], OnSets); end;, then running f() produces the following output (on my machine):

gap> f();
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
Error, no 1st choice method found for `^' on 2 arguments called from
act( pnt, id ) on line 439 of file /Users/caj/work/source/gap/gap/lib/oprt.gd called from
TestIdentityAction( acts, pnt, act ) on line 844 of file /Users/caj/work/source/gap/gap/lib/oprt.gd called from
CallFuncList( StabilizerFunc, arg ) on line 2841 of file /Users/caj/work/source/gap/gap/lib/oprt.gi called from
Stabilizer( AlternatingGroup( 5 ), [ 1.0 ], OnSets ); on line 2 of file t.g called from
<function "f">( <arguments> )
 called from read-eval loop at line 5 of *stdin*
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue

Notice that t.g is given a short name, but /Users/caj/work/source/gap/gap/lib/oprt.gi is given the full, long file-system name.

I would like to change that second one to something shorter, but I'm not sure what the best option is. Perhaps {GAP_ROOT}/lib/oprt.gi? That does leave the problem that GAP can have multiple roots of course, but I feel something shorter would be better for most users.

After doing some work on cleaning up the library around ConvertToVectorRep and CopyToVectorRep,
I am experimenting with having the kernel check for, and ban, in-place conversion of lists and matrices in the global/atomic region (region zero). This is basically immutable lists and matrices.

This is easy to do, but currently breaks quite a few tests. Mostly these are easy to fix and I'm working through them. If I can make this work it might be worth doing the same for string conversion, blist conversion, etc.

The nice thing is it then legal to make a new mutable (and so thread-local) vector, convert it and then optionally make it immutable, which can never cause a race condition, but illegal to convert an object that could in principle be accessible by another thread.

Running the grpperm.tst in HPC-GAP on my machine does not only take an exorbitant amount of time (4 minutes in GAP, more than 10 in HPC-GAP) it also Diffs:

########> Diff in tst/grpperm.tst, line 76:
# Input is:
NrMovedPoints(g/h);
# Expected output:
168
# But found:
112
########

I can reproduce this with the current hpcgap-default branch on my laptop and lovelace, one of our Linux test machines. Hence, it is not just a fluke caused by my obscure operating system (DragonFly 4.1-DEVELOPMENT).

Disclaimer: I like GAPDoc a lot, and this is not about getting rid of it, or not using it, or not installing it, or anything like that -- just making it easier to bootstrap a fresh GAP instance from source (see issue #5).

I think GAP should be a stand-alone project in the sense that it should be possible to download its source code, compile and then run it, without further extras. (I do not consider GMP and readline "extras" in this sense, as they are easily installable via package managers on most / all Unix systems, including most Linux distros and Mac OS X; and in any case, GAP works without readline, and by default builds its own GMP).

This is currently not possible, because GAP refuses to start if it cannot find GAPDoc. I see two ways around that:

1. Ship GAPDoc with GAP.
2. Allow starting GAP without GAPDoc.

Regarding the first option: We could include a copy of it in the repository. Or if Frank made a public git repository for GAPDoc somewhere, we could include it as a git submodule into the repository. I'd be happy with this/

As to the second option: Of course we should then print a big warning, as the documentation may not work. (Thinking about that, we probably should also print such a warning if GAPDoc is present, but the user has not compiled the manual...).

I just tested, and this minimal diff actually allowed me to start GAP without GAPDoc:

diff --git a/lib/package.gi b/lib/package.gi
index 375ebb2..4411a36 100644
--- a/lib/package.gi
+++ b/lib/package.gi
@@ -1832,6 +1832,10 @@ BindGlobal( "BANNER", false );
       fi;
     fi;

+    if not IsBound( GAPInfo.LoadPackageLevel ) then
+      GAPInfo.LoadPackageLevel:= 0;
+    fi;
+
     # If necessary then load the implementation part of the GAP library,
     # and the implementation parts of the packages loaded up to now.
     if not IsBound( GAPInfo.LibraryLoaded ) then
diff --git a/lib/system.g b/lib/system.g
index e4f5570..b11a880 100644
--- a/lib/system.g
+++ b/lib/system.g
@@ -28,7 +28,7 @@ BIND_GLOBAL( "GAPInfo", rec(

 # Without the needed packages, GAP does not start.
     Dependencies := rec(
-      NeededOtherPackages := [
+      SuggestedOtherPackages := [
         [ "gapdoc", ">= 1.2" ],
       ],
     ),

Some caveats apply:

1. There is an error in transatl.g, because it uses SubstitutionSublist which is supplied by GAPDoc. Solution: Add a copy of SubstitutionSublist (under a different name) to transatl.g
2. A big warning should be printed if GAPDoc is not present.
3. Tests should be run to see if other parts of GAP (or possibly packages) indirectly depend on GAPDoc functions. For code in GAP itself, I think we should then try to cut the dependency; for packages that use GAPDoc functions, I think those are fine, we just should suggest to their authors that they add GAPDoc to the NeededOtherPackages list.

Overall, this doesn't sound too difficult to me.

Thoughts?

An attempt to view GAPInfo or GAPInfo.TestData now fails with:

gap> GAPInfo.TestData;
Error, RecNames: <rec> must be a record (not a thread-local record) in
  rec( 
  names := List( RecNames( record ) )
     ; on line 296 of file /Users/alexk/GITREPS/hpcgap/lib/record.g

Indeed, GAPInfo is an atomic record and TestData is a thread local record. Perhaps TestData should not be kept in GAPInfo any more.

When HPC-GAP is loaded with the "color prompt" turned on - that is, with a line like

SetUserPreference( "UseColorPrompt", true );

in gap.ini - an error is encountered every time a command is executed:

gap> 1+1;<3> Attempt to read object 89230224 of type object (positional) without having read access in
<3>   Error, return WRITE_STRING_FILE_NC( stream![1], str ); on line 1560 of file /home/mtorpey/hpcgap/gap/lib/streams.gi called from 
<3> WriteAll( STDOut, "\033[0m" ); on line 185 of file /home/username/hpcgap/gap/lib/colorprompt.g called from
<3> <function "EndLineHook">( <arguments> )
<3>  called from read-eval loop at line 0 of *defin*
2
gap> 

and the prompt is not shown in colour.

The following example is the first where grpmat.tst trips over:

1. In the main thread, call

gap> g:=Group( ( 9,11,10), ( 2, 3, 4),  (14,17,15), (13,16)(15,17), 
   >        ( 8,12)(10,11), ( 5, 7)(10,11), (15,16,17), (10,11,12) );;
Sum(ConjugacyClasses(g),Size)=Size(g);
gap> true

1. Switch to another thread and call the same:

[5] gap> g:=Group( ( 9,11,10), ( 2, 3, 4),  (14,17,15), (13,16)(15,17), 
          ( 8,12)(1[5] 0> ,11), ( 5, 7)(10,11), (15,16,17), (10,11,12) );;
Sum(ConjugacyClasses(g),Size)=Size(g);
[5] gap> [5] Error, Attempt to read object 4342401168 of type object (data) without having read access in
[5]   iso := GroupHomomorphismByImagesNC( G, A, pcgs, GeneratorsOfGroup( A ) )
[5]  ; on line 30 of file /Users/alexk/GITREPS/hpcgap-fork/lib/permdeco.gi called from 
[5] FittingFreeLiftSetup( G 
[5]  ) on line 2227 of file /Users/alexk/GITREPS/hpcgap-fork/lib/clashom.gi called from
[5] ConjugacyClassesViaRadical( G 
[5]  ) on line 2940 of file /Users/alexk/GITREPS/hpcgap-fork/lib/clashom.gi called from
[5] <function "unknown">( <arguments> )
[5]  called from read-eval loop at line 3 of stream
[5] brk> !0 

1. Check inaccessible object in the main thread:

--- Switching to thread 0
gap> OBJ_HANDLE(4342401168);
f1

Seems related to the discussion in #74 on associative words in syllable format...

For a long time, GAP allowed creating inline functions with a single parameter, e.g.
f := x -> x + 1;
Recently, as a feature backport from HPC-GAP, a similar feature for parameterless inline functions was added:
g := -> 42;
It woulde be nice if there was a similar syntax for functions with more than one parameter, e.g.
h := (x,y) -> x+y;
This can be used in many places, e.g. when sorting a list by a custom function:
Sort(list, (x,y) -> 2*x < y);
This examples also shows why I did not suggest this simpler syntax (as it woudl lead to ambiguities):
h := x,y -> x+y;
For consistency, I'd then also allow adding parentheses for the 1 and 0 parameter versions, i.e.
f := (x) -> x + 1;
and
g := () -> 42;

Creating a permutation group object and performing any operation on it creates StabChainMutable, a mutable stabiliser chain for it, which is a mutable attribute, and is placed in the creating thread's thread local region.

Trying to perfom any other operation on that same group in a different thread leads to access errors.

We have agreed to get rid of mutable attributes like StabChainMutable.

I think we agreed to use an ImmutableStabChain, which can be copied and changed, or just computing an entirely new stabiliser chain thread locally.

This will require some sifting through the library, but it is really necessary because running the following code in HPC-GAP leads to an access problem:

gap> g:=WreathProduct(MathieuGroup(11),Group((1,2)));     
<permutation group of size 125452800 with 5 generators>
gap> Length(ConjugacyClassesSubgroups(g));
!sh
--- Switching to thread 5
[5] gap> h:=WreathProduct(MathieuGroup(11),Group((1,2)));
[5] <permutation group of size 125452800 with 5 generators>
[5] gap> Length(ConjugacyClassesSubgroups(h));
[5] Error, Object not in a readable region in
[5]   chain := StructuralCopy( StabChainMutable( G ) ); on line 705 of file /home/makx/ac/hpcgap/lib/grpperm.gi called from 
[5] ClosureGroup( C, list, rec(
[5]     random := 0,
[5]     temp := true ) ) on line 985 of file /home/makx/ac/hpcgap/lib/grpperm.gi called from
[5] CommutatorSubgroup( m[i], m[j] ) on line 625 of file /home/makx/ac/hpcgap/lib/morpheus.gi called from
[5] SomeVerbalSubgroups( G, H ) on line 1241 of file /home/makx/ac/hpcgap/lib/morpheus.gi called from
[5] Morphium( G, H, false ) on line 1980 of file /home/makx/ac/hpcgap/lib/morpheus.gi called from
[5] IsomorphismGroups( G, UnderlyingGroup( t ) ) on line 2660 of file /home/makx/ac/hpcgap/lib/grplatt.gi called from
[5] ...  at line 2 of stream
[5] gap>

In regular GAP, I can run individual GAP tests in isolation just fine; in particular, the Test function takes care of setting the "virtual" screen width for output of the tests to 80, so that there are no differences caused by my actual window size. For example, I can do this:

gap> Test("tst/grppc.tst");
grppc.tst
GAP4stones: 155136
true
gap> 

In contrast, this mechanism is broken in HPC-GAP. Thus, I get this:

gap> Test("tst/grppc.tst");
########> Diff in tst/grppc.tst, line 65:
# Input is:
c:=11173863408701668767758108041977982393767296210528199906493002694797783834150538286350266065216327192789065778954586535410\
5698690880491419382732505129685548945886493976382779091529311779061982182942409366242406420035526825355893426176;
# Expected output:
111738634087016687677581080419779823937672962105281999064930026947977838341505\
382863502660652163271927890657789545865354105698690880491419382732505129685548\
945886493976382779091529311779061982182942409366242406420035526825355893426176
# But found:
111738634087016687677581080419779823937672962105281999064930026947977838\
341505382863502660652163271927890657789545865354105698690880491419382732\
505129685548945886493976382779091529311779061982182942409366242406420035\
526825355893426176
########
grppc.tst
GAP4stones: 95368
false
gap>

This is quite inconvenient, e.g. when testing regressions that involve manually (re)running tests over and over. I imagine this might be caused by SizeScreen not working as expected in HPC-GAP...

Just to inform that make testmanuals now shows segmentation faults in many chapters. I've narrowed this down to Jenkins build at 6pm on March 19 th running fine and another Jenkins build at 9.36am on March 20th showing the problem.

This is another example which is hard to reproduce. If you run make testmanuals, the log in dev/log directory shows reproducible segfaults at same chapters each time. However, trying to extract examples to run them using Test, even with assertion level 2, does not trigger the bug at all.

Finally, I've found a way to reproduce it. Workspace is involved. Create a test file bug.tst with the following lines:

gap> s8 := Group( (1,2), (1,2,3,4,5,6,7,8) );
Group([ (1,2), (1,2,3,4,5,6,7,8) ])
gap> a8 := DerivedSubgroup( s8 );
Group([ (1,2,3), (2,3,4), (2,4)(3,5), (2,6,4), (2,4)(5,7), 
  (2,8,6,4)(3,5) ])
gap> IsNaturalAlternatingGroup(a8);
true
gap> a8;
Alt( [ 1 .. 8 ] )
gap> syl2 := SylowSubgroup( a8, 2 );; Size( syl2 );
64
gap> Normalizer( a8, syl2 ) = syl2;
true

Then load GAP with -r -A and save workspace as test.wsp. After that load GAP from that workspace with -r -A -L test.wsp and try to perform the test:

gap> Test("~/Desktop/bug.tst");
Segmentation fault: 11

Most current tests we invoke with Test read and write to global variables. This means that we can't run the tests in parallel, as they scribble over each other's variables.

The best fix I think I can think of for this is to move away from tst files to, wherever possible, running unit tests which are small GAP functions. We will want to keep some tst to check input/output I imagine.

I am mainly creating this issue in case anyone has a great idea for saving Test.

The following currently happens in HPC-GAP:

gap> m:=[[1,2],[3,4]];
[ [ 1, 2 ], [ 3, 4 ] ]
gap>  IsMatrix(m);
true
gap>  MakeReadOnly(m);
[ [ 1, 2 ], [ 3, 4 ] ]
gap>  IsMatrix(m);
false

InitCopyGVar is used to link C and GAP level variables. I think this doesn't understand thread-local variables, which upsets compiling. I'm not sure how to fix this -- it might be fundamentally incompatable with the way we currently do compiling (we would have to make the thread-local GAP variables into thread-local C variables, for a start). In that case, we might need some hack to get around this problem.

So we we would for example add --help as alias for -h, --root-paths for -l, and so on.

I think those are easier to remember, and in particular, easier to understand when reading a script invoking GAP that one wrote a long time ago. Of course the existing short names should stay for now to ensure compatibility. Though I think we might want to remove some of them eventually, so that we can repurpose them for more useful things in the distant future...

My students are having quite some trouble finding the Random( from, to ) variant of Random they need for some course problems. This excerpt of the ref manual shows why:

Random, for a list 14.7-2
Random, for a list or collection 30.7-1
Random, for a range of integers 14.7-2
Random, for integers 14.2-12
Random, for lower and upper bound 30.7-1
Random, for rationals 17.2-7
random element, of a list or collection 30.7-1

I.e. they need to look at 14.2, 14.7, 17.2 and 30.7, and then also understand each.

I propose we unify these entries in a single entry, unless somebody has a suggestion as to why it would be useful to keep those separate entries (which also spam the built-in GAP help function with distracting data).

The pkg directory is not under version control in the master branch. However, it is under version control in the hpc-gap default branch because it contains an older version of GAPDoc package by @frankluebeck with slight modification to work under HPC-GAP.

Since in my clone of the GAP repository I have a symlink pkg pointing to pkg in my GAP installation, I've just created another clone to work with hpcgap-default branch there.

I think it would cause less confusion if we would remove pkg subdirectory from the hpcgap-default branch too. I will try to summarise below the changes that I have made to GAPDoc - hopefully they may be re-applied to make a new version of GAPDoc working with both GAP and HPC-GAP.

It seems there are some fundamental difference in the way HPC-GAP handles component objects vs. positional objects. The former seem to be automatically in the public region:

gap> fam := NewFamily("foo");;
gap> comType := NewType( fam, IsComponentObjectRep );;
gap> comObj := Objectify( comType, rec(a:=1, b:=2, c:=3) );;
gap> RegionOf(comObj);
<region: public region>

Not so for positional objects, they are put into a thread local region:

gap> fam := NewFamily("foo");;
gap> posType := NewType( fam, IsPositionalObjectRep );;
gap> posObj := Objectify( posType, [1,2,3] );;
gap> RegionOf(posObj);
<region: thread region #0>

Unfortunately, not even taking an immutable copy helps (perhaps not surprising, since IS_MUTABLE_OBJ and IsMutable return false for posObj anyway):

gap> RegionOf(`posObj);
<region: thread region #0>

The only way to get a positional object in the public region that I found is to pass an immutable list to Objectify:

gap> posObj := Objectify( posType, `[1,2,3] );;
gap> RegionOf(posObj);
<region: public region>

But in actual code, this is not always an option. E.g. when dealing with collectors and rws groups: The collectors are positional objects. Initially, they are created mutable. Later, one creates an immutable copy. But due to the above behavior, that copy is in a thread local region, which is annoying.

I tried to use MakeReadOnly on those objects as a workaround, but that doesn't really help, as it doesn't seem to recurse to the subobjects.

I've been looking at why g:=WreathProduct(MathieuGroup(9),Group((1,2)));;ConjugacyClassesSubgroups(g);; is still slower in HPC-GAP. I think I have an idea based on line-by-line profiling (now functional in HPC-GAP). The big remaining issue is that NewType keeps not using the cache, which in turn means method selection is much more expensive.

To give an idea, in g:=WreathProduct(MathieuGroup(9),Group((1,2)));;ConjugacyClassesSubgroups(g);;

On the first call in a GAP session, NewType is called 220,122 times and there are 3,784 cache misses.
On a second call of the same code, NewType is called 218,010 times and there are 1,517 cache misses.

In HPCGAP the times called are very similar, but on 166,014 calls we purposefully avoid the cache, leading to 166,294 cache misses. SImilarly, on the second call we again purposefully avoid the cache and get 164,018 cache misses.

This currently happens in hpcgap-default branch:

gap> TransitiveIdentification(TransitiveGroup(30,4064)^(1,4,5,2)
> (6,20,15,21,7,16,12,24)(8,18,14,22,10,17,13,23,9,19,11,25)(26,30,29,28));
Error, Cannot lock required regions in
  Panic: tried to print an expression of unknown type '64'
Segmentation fault: 11

(I suspect that this happened after some recent merge yesterday - we may have to retrace this via Travis logs).

In the most recent development version of non-HPC GAP (commit 51025ca) using Orb 4.7.3 (whether or not it is compiled) [https://github.com/gap-system/orb/] testing of the Semigroups package has revealed a segfault:

If load GAP with:
bin/gap.sh -r -A
And then call:

gap> LoadPackage("orb", false);
gap> comp := BlistList([ 1 .. 3 ], [  ]);;
gap> opts := 
> rec(lookingfor := function(o, x)
>   if not IsEmpty(x) then
>     return IsPosInt(comp[ x[ 1 ] ]);
>   fi;
>   return false;
> end);;
gap> gens := [ PartialPerm( [ 1, 3 ], [ 3, 2 ] ) ];;
gap> o := Orb(gens, [ 1 ], OnSets, opts);;
gap> Enumerate(o);;
gap> Elements(o);
[ [  ], [ 1 ], [ 2 ], [ 3 ] ]
gap> Elements(o);;
Segmentation fault: 11

Calling Elements(o) the second time triggers a segfault.

LLDB says the segfault seems to happen in the GAP library in src/plist.c on line 706:

/* This had better return the cyclotomics family, could be speeded up */
family = FAMILY_TYPE( TYPE_OBJ( ELM_PLIST( list, 1 ) ) );

The list list is empty but this is not expected. Markus thinks this might be a problem with partial perms in GAP.

I originally found by using the most recent HPC-GAP. Using HPC-GAP, the segfault happens in the Enumerate(o) function. Perhaps these are different bugs, but it seems reasonable to think it is the same problem.

Support for atomic functions is not yet fully backported to the master branch. It works in assignments:

gap>  f := atomic function(x) return x; end;
function( x ) ... end

but it doesn't work in statements:

gap>  atomic function(x) return x; end;
Syntax error: do or comma expected
 atomic function(x) return x; end;
                                 ^

We have to find the fix for this in HPC-GAP and properly backport it to the master branch.

This function is like Error, except users cannot return and continue the function. I'm putting this here for discussion / testing, before I make a proper pull request

ErrorMayQuit := function ( arg )
    ErrorInner( rec(
         context := ParentLVars( GetCurrentLVars(  ) ),
         mayReturnVoid := false, mayReturnObj := false,
         lateMessage := "type 'quit;' to quit to outer loop",
         printThisStatement := false), arg);
 return;
 end;

the master branch already supports atomic as a keyword, and also AtomicList, AtomicRecord and AtomicIncorporateObj. To facilitate using same code for GAP 4 and HPC-GAP, I suggest to add also replacement for the following constructs:

• IsAtomicPositionalObjectRep
• IsNonAtomicComponentObjectRep
• MakeWriteOnceAtomic
• FixedAtomicList

As discussed on issue #5, it is useful to have a global directory where one places packages, so that one can point multiple GAP instances to it, and have them share it.

Unfortunately, the only way I am aware of doing this right now is via the -l option, by adding another root directory to GAP.

This means I cannot just create a directory my-gap-packages somewhere and point GAP to it. Instead, I need to create a subdirectory pkg of that, and places the packages in there, and point GAP to the parent directory.

So, I'd like to see a new option that allows the user to skip that bit, so that one can directly point at the my-gap-packages -- i.e. add that directory to a list of "package search directories" or "package roots", without touching the list of GAP roots.

In fact, it would be nice if one could also directly point GAP at a package directory (so at my-gap-packages/my-new-package) -- this would be particularly useful while developing a package, if one has multiple versions around; when I want to test a change in a specific clone of a package, I could then just start GAP in a way that tells it to search in the current directory for a package.

This brings me to the question how the new option should look. The simplest way might be to add something that is very similar to -l, i.e. including the way it treats semicolons in the string to decide whether to replace / append to / prepend to the list of paths. Then I might invoke GAP like this:

gap --package-roots=".;"

This means: prepend the current directory to the existing list of package roots (derived from the GAP roots). Advantage: this is consistent with existing behavior of -l. Disadvantage: This existing behavior is not exactly self-explanatory... but that is perhaps a separate problem ;-), if it is a problem at all (I am fine with the way it is, but I am not sure how people new to GAP feel about it).

BTW, in the issue title I mention "config options": I'd love if I could add additional package paths to my gap.ini file -- right now, in order to get GAP to load my external package trees (yes, plural), I write a shell script to invoke GAP with the right options. This way, the packages would be picked up by any GAP binaries that I have around, not just the "master" one.

Oh, and double bonus points if I were able to add additional package search paths during runtime... ;-).

After recent merges from master into hpcgap-default branch, the following two new diffs appeared in bugfix.tst when the test is run in the main execution thread:

# Input is:
ProbabilityShapes(x^5+5*x^2+3);
# Expected output:
[ 2 ]
# But found:
Error, Attempt to read object 548962944 of type list (plain,ndense) with\
\
out having read access

and

# Input is:
GaloisType(x^12+63*x-450); # this was causing an infinite loop
# Expected output:
301
# But found:
Error, Attempt to read object 548962944 of type list (plain,ndense) with\
\
out having read access

Jenkins tests of HPC-GAP are running out of time on all Jenkins slaves since Friday morning after the following three commits by @ChrisJefferson :

• Clean up GAP signalling
• Add atomic handling to compiler, and alter thread local variable in type1.g
• Use VAL_GVAR/ASS_GVAR in random.g, so we can compile it correctly

I can reproduce this on my machine, but @markuspf can not. Could anyone else reproduce this?

gap> Test("tst/access.tst");
Error, Attempt to read object 4324496608 of type list (string) without having r\
ead access in
  <compiled or corrupted statement>  called from 
<compiled or corrupted statement>  called from
<compiled or corrupted statement>  called from
NewType( Fam, IsZmodpZObjSmall and IsModulusRep 
 ) on line 84 of file /Users/alexk/GITREPS/hpcgap/lib/zmodnz.gi called f\
rom
ZmodnZObj( F, 1 
 ) on line 1002 of file /Users/alexk/GITREPS/hpcgap/lib/zmodnz.gi called\
 from
ZmodpZNC( n 
 ) on line 1038 of file /Users/alexk/GITREPS/hpcgap/lib/zmodnz.gi called\
 from
...  at line 0 of *defin*
brk> 

These two files are to be sharpened and updated to our needs, preferably really soon.

I'm looking for examples of where GAP incorrectly reports where errors occur, or prints <compiled or corrupted statement>, when the code is purely in GAP. For now I'll keep them all in one bug report.

• Matrix deref, for example:
  function() local S; S := []; return S[0]; end;
• atomic access (in HPC-GAP)
  function() atomic R do od; end;

Any other occurrences welcome (no promises about fixing them, but let's at least find them).

@hulpke and I have been discussing this on email and he suggested I post it here.

The intention of the string and print functions is described in the manual, “ViewString returns a string which would be displayed by ViewObj (6.3-5) for an object”.

This is not true for double cosets as of GAP 4.7.5 of 24-May-2014:

gap> dc := DoubleCoset(Group( [ (1,2) ] ),(2,4),Group( [ (3,4), (1,2), (1,3)(2,4) ] ));;
gap> ViewObj(dc);
DoubleCoset(Group( [ (1,2) ] ),(2,4),Group( [ (3,4), (1,2), (1,3)(2,4) ] ))
gap> ViewString(dc);
"<object>"
gap>

I worked around this by writing to a string stream:

MyPrintToString := function(obj)
    local str, out;
    str := "";;
    out := OutputTextString( str, true );;
    PrintTo(out, obj);
    CloseStream(out);
    return str;
end;;

The following short test breaks hpcgap in a number of different ways.

check := function(g)
    local  x;
    for x in g do
        if Order(x) > 1 and Order(x) mod 2 = 1 then
            return x;
        fi;
        return fail;
    od;
end;

checks := function(maker, stride, start, limit)
    local  i, x;
    for i in [start, start + stride .. limit -1 - ((limit-1) mod stride) + start] do
        x := check(maker(i));
        if x <> fail then
            return x;
        fi;
    od;
    return fail;

end;

tasks := function(m,n) 
    return List([1..m], i->RunTask(checks, j->SmallGroup(n,j), m, i, NrSmallGroups(n)));
end;

For various values of m and n I get errors reading files, creating types and from the collectors. If we could make this work I think we would have made a lot of progress.

This is a very old idea, but I realised this evening it may be a bit easier than I had previously assumed.

The idea is to allow function expressions

function(a,b,c,arg).....end;

This would require at least three arguments and assign the first three to a b and c and then a list containing any remaining ones to arg.

This requires extending the parser to recognise this case, which is easy and signal the numbert of arguments as -1- (this is consistent with the current setup where arg is signaled as -1).

Then when we code such a thing we just have to install the right handlers -- DoFail0Args..DoFail2Args (in this case) and then DoWrap3Args...DoWrap6Args. So far these all exist.

Then we would need a new handler DoPartiallyUnwrapAndExecFunc (or some nicer name) which would be installed as the XArgs handler and would know to expect a list, look at the aritty of the function to find that it had to copy the first three elements into the first three lvars and shift the remaining ones left to the start of the list (adjusting the length accordingly) and then assign the list to the fourth lvar.

There would need to be some printing and other stuff, but it's a lot less work than I thought.

I have taken all the hpcgap branches (other than hpcgap-default) and turned them into tags, as they are not expected to continued in future.

The branches stable-4.1 to stable-4.6 and stable-4.beta were all closed in mercurial some time ago. I intend to turn them into tags as well, called (for want of better names) 'closedbranch/stable-4.1', etc.

This will leave:

hpcgap-default
master
stable-4.7

as the only branches.

The documentation says that it

loads the GAP package with name `name`. 

I think it should be formulated as

loads the GAP package with name given by the string `name`".

(just today I referred to the manual a novice user who was trying to load package with LoadPackage(sglppow) without quotes around the package name, and then we figured out that this may not be so obvious...)

This is an easy task for anyone who'd like to try to practice to work with git, pull requests, GAP documentation etc., so I am marking it as "help wanted" to advertise as such.

If you try to load a package in a thread other than thread 0, it raises an error.

We just debugged an instance of C code trying to run ElmPRec on an atomic record, which went horribly wrong.

Ideally we add assertions of types in all accessor functions to make (HPC-)GAP crash rather than trying to read or write bogus data.

This actually came up in a discussion on the io package. @fingolfin suggested that we add functionality for packages to register TNUMs. I will paste the jist of the discussion here for reference. I'd implement and pull-request this, if @fingolfin doesn't already have code for this.

This will enable a clean solution to a global variable problem in io (and probably elsewhere).

This is from io issue 4

We do this already in the packages SingularInterface (formerly "libsing") and NormalizInterface. The way this is done right now is the following: For each package that needs it, a TNUM is reserved (SingularInterface uses T_SINGULAR; NormalizInterface currently uses T_SPARE1; and there is also T_POLYMAKE, but I think it is not in use right now).

GAP itself never creates objects (or rather, "bags") with these TNUMs, but external packages do this. The cool thing about this is that it allows those external packages to do proper memory management, as the GAP garbage collector GASMAN provides hooks for this, set via InitFreeFuncBag and InitMarkFuncBags. With this, wrapping e.g. C++ objects becomes relatively easy; the code for NormalizInterface is rather short, see normaliz.cc.

This system is not perfect, but works reasonably well. The only problem is that it currently requires a modification to the GAP kernel for each package that wants to do this. This is clearly a problematic model that does not scale well. And I really hope it will have to scale, as I'd like to see wrappers for e.g. libcurl (full-blown HTTP client support), libz/libzlib/libarchive (various libraries for dealing with compressed files or even archives), pcre (or any other regular expression library), GMP (exposing the high-level GMP functions, which we currently can't use in GAP -- this should be rather easy to achieve, and I'd like to do it in a way that integrates well with SingularInterface and NormalizInterface).

The simplest ad-hoc solution for this is to reserve a bunch of TNUMs for packages and let packages register a TNUM. I have been using this internally for quite some time now, and it boils down to a rather short code change, essentially this (plus some header modifications):

static Int lastFreePackageTNUM = FIRST_PACKAGE_TNUM;

/****************************************************************************
**
*F  RegisterPackageTNUM( <name>, <typeObjFunc> )
**
**  Allocates a TNUM for use by a package. The parameters <name> and
**  <typeObjFunc> are used to initialize the relevant entries in the
**  InfoBags and TypeObjFuncs arrays.
**
**  If allocation fails (e.g. because no more TNUMs are available),
**  a negative value is returned.
*/
Int RegisterPackageTNUM( const char *name, Obj (*typeObjFunc)(Obj obj) )
{
   if (lastFreePackageTNUM > LAST_PACKAGE_TNUM)
      return -1;

   Int tnum = lastFreePackageTNUM++;

   InfoBags[tnum].name = name;
   TypeObjFuncs[tnum] = typeObjFunc;

   return tnum;
}

Packages then would call RegisterPackageTNUM from their postRestore function, like this:

extern Obj T_SINGULAR = 0;

static Int PostRestore(StructInitInfo* module)
{
...
   T_SINGULAR = RegisterPackageTNUM("singular wrapper object", _SI_TypeObj);
   if (T_SINGULAR == -1)
       return 1; /* signal an error */
...

so that even workspaces should work, provided that packages are loaded in the exact same order as before. (To fix the latter constraint, one could store which TNUM was assigned to which package; alas, for now I deemed it not worth the effort, as other things are likely to break when the load order changes, such as types get assigned which flag values etc. -- but it certainly would be possible to improve this).

Note that I deliberately did not include a mechanism for querying TNUMs. This keeps the code very simple and makes it easy to keep it bug free, while not preventing packages from exchanging TNUM. For example, I imagine we'll have a GMPInterface package at some point, and both SingularInterface and NormalizInterface will want to use its "external GMP integers". Well, no problem, it can simply assign the TNUM integer value to a global gap variable, say GMPINTERFACE_TNUM, and then SingularInterface and NormalizInterface can use that

Document how to make a checked out copy of the GAP repository even usable. It requires downloading of a selection of packaes, at the very least GAPDoc.

Using a symlink to a directory outside of the GAP tree is probably more suited for GAP development.

Python (and various other languages) has a nice feature for unpacking a list into multple variables:
a,b,c := [1,2,3];
assigns the values 1,2,3 to the variables a,b,c. This is quite useful for functions returning multiple values as tuples. Say you have a function which returns two values in a list. Then often, you might write code like this:

tmp := QuotientRemainder(17,4);
q := tmp[1];
r := tmp[2];

which you can turn into this with list unpacking:

q,r := QuotientRemainder(17,4);

There are various pitfalls with that, though, once you go beyond plain identifiers on the left side. But I think python already thought about most (?) of those. Here are some example of a Python session (note that Python indexes starting from 0):

>>> x,x = [1,2]; x
2
>>> x,x[1] = [[1,2],42]; x
[1, 42]
>>> y = [17,23]
>>> x,y[x] = [1,2]; y
[17, 2]

But I am not sure whether we can "easily" support that in our parser...?

Merging various bugs into one report. If you try to name members with long names (longer than about 1024 characters) into records, all kinds of things break, including:

• Enter break loop when you try to print them out
• Strings get truncated in RecNames
• If you put enough values in, older values will get over-written

To see this, type:

gap> r := rec();;
gap> r.(List([1..2000], x -> 'a')) := 1;;
gap> Length(RecNames(r)[1]); # should be 2000
1023
gap> r;

... hits an error because r doesn't have a member with name RecNames(r)[1]. I hit this because I was using records like dictionaries, as dictionaries were behaving badly.

I can see two routes of attack:

1. Enforce a limit on name of a record (probably 1023, same as identifiers), and error when anyone tries to insert such a longer name into a record.
2. Remove the hard-wired limitations on lengths of record members.

(1) is obviously easier, and I assume no-one else has ever hit this issue before. Happy to look at 2, obviously there is the risk of a slight efficiency loss

This is a note of a discussion about how to produce a usable UI for HPCGAP Beta.

1. GNU readline is not supported for now. It may be possible, but will be a lot of work and is not urgent.
2. The existing hooks used by the command-line editor which call bits of GAP "out of band" should be removed (or reduced to noops) and replaced by much simpler calls "in-band" from the input thread.
3. The input and output threads should provide some simple customisation mechanism for colouring etc.
4. Existing colourisation mechanisms should be mapped to this or cleanly ignored.
5. The input and output threads should cooperate to maintain a "normal" division of the terminal between one line for input, one for status and the rest for output. Input is echoed in the input area as you type and copied into the output stream when the foreground thread reads it.
   • It would be nice for the status line to include a report of which, if any, threads have pending output.
6. If input lines are pending (so they have been typed, the user has hit return and they have not yet been delivered to the foreground thread, and the next line is an ! command that changes foreground thread, then the lines before that are delivered to the OLD foreground thread, while the ones after it are delivered to the NEW foreground thread.
7. A Thread which needs complete control of the terminal (eg to run an external process) can have it. While it has it, all other output is buffered, any thread that wants input is blocked, as is any other thread trying to establish this mode, and no input is delivered to the input thread.
8. We should add !h[elp] and document the mechanism for installing new ! functions

To get and set entries in matrices, we currently have to write

x := a[row][col];
a[row][col] := y;

to access an entry. This internall has to frist fetch a row object, then an entry in that. This is problematic for e.g. packed matrices, where there are no real "row" objects; in that case, a temporary object representing the row needs to be represnted. This is a big problem for alternative matrix representations, e.g. sparse representations.

This is why for the new MatrixObj interfaces for matrices-as-objects (as opposed to matrices-as-lists-of-lists), new operationsSetMatElm(a,row,col,val) and MatElm(a,row,col) were defined. Example:

x := MatElm(a, row, col);
SetMatElm(a, row, col, y);

This works, but of course is syntactically not so nice. Hence, I propose we add new syntactic sugar allowing this syntax (similar syntax exists in various other languages):

x := a[row,col];
a[row,col] := y;

Note that the code in PR #28 almost allows for this; namely, that codes allows passing arbitrary objects as list indices, and thus allows to use lists as indices... so with that patch, one could install methods for \[\] and \[\]\:\= which then allow writing code like this:

x := a[[row,col]];
a[[row,col]] := y;

However, this syntax is slightly less nice and less "standard". It also involves creating termporary lists [row,col], so it has a little extra overhead -- if one does many matrix operations, then one ends up creating tons of small termporary objects, which tends to hurt garbage collectors (but perhaps it is not so much of an issue for the generational collector in GAP?).

Title says it all.

At the moment ReadGapRoot looks for compiled versions of GAP Library files as statically linked modules (which we use a little) and as dynamically loadable modules in directories under compiled/. If found they are loaded instead of the GAP file (provided various checksums match).

I don't think anyone uses the dynamic option and, in the interest of slightly cleaning up a very complicated piece of code, I'd quite like to withdraw it.

It's not urgent. It isn't causing a current bug, but ReadGapRoot is horrible and making it less horrible would be nice.

Testing the Semigroups package has revealed a segfault caused by IndexPeriod.

In usual GAP, this is the expected behaviour:

gap> a := Transformation( [ 4, 2, 3, 3, 4 ] );
Transformation( [ 4, 2, 3, 3, 4 ] )
gap> IndexPeriodOfTransformation(a);
[ 2, 1 ]

However in HPC-GAP I get:

gap> a := Transformation( [ 4, 2, 3, 3, 4 ] );
Transformation( [ 4, 2, 3, 3, 4 ] )
gap> IndexPeriodOfTransformation(a);
Segmentation fault: 11

The segfault happens in line 52 of trans.c:

static inline void ResizeTmpTrans( UInt len ){
  if(SIZE_OBJ(TmpTrans)<len*sizeof(UInt4)){  THIS LINE
    ResizeBag(TmpTrans,len*sizeof(UInt4));
  }
}

which must eventually get called from INDEX_PERIOD_TRANS from the same file. I'm not sure how pursue this problem beyond here.

It would be useful to get Info messages displayed on the screen even if they are produced by a background thread.

• I downloaded io-hpc-4.2.tar.gz from http://hpc.gap-system.org/hpcgap-alpha/
• ./configure worked
• make failed:

$ make
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I./src  -I/Users/mo/software/gap/hpcgap/gap -I/Users/mo/software/gap/hpcgap/gap/bin/x86_64-apple-darwin14.1.0-gcc-hpc -DCONFIG_H -isystem /sw/include  -g -O2 -MT io_la-io.lo -MD -MP -MF .deps/io_la-io.Tpo -c -o io_la-io.lo `test -f 'src/io.c' || echo './'`src/io.c
libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I./src -I/Users/mo/software/gap/hpcgap/gap -I/Users/mo/software/gap/hpcgap/gap/bin/x86_64-apple-darwin14.1.0-gcc-hpc -DCONFIG_H -isystem /sw/include -g -O2 -MT io_la-io.lo -MD -MP -MF .deps/io_la-io.Tpo -c src/io.c  -fno-common -DPIC -o .libs/io_la-io.o
In file included from src/io.c:17:
In file included from /Users/mo/software/gap/hpcgap/gap/src/compiled.h:19:
In file included from /Users/mo/software/gap/hpcgap/gap/src/gasman.h:50:
/Users/mo/software/gap/hpcgap/gap/src/atomic.h:5:10: fatal error: 'atomic_ops.h' file not found
#include <atomic_ops.h>
         ^
1 error generated.
make[1]: *** [io_la-io.lo] Error 1
make: *** [all-recursive] Error 1

The proposal is to add the optional components for the repository, issue tracker and support email address for a package in order to be displayed on the automatically generated overview pages for packages:

• Repository := rec( URL := "http://FOO", Type := "hg/git/svn/cvs")
• IssueTrackerURL
• SupportEmail

Knowing the type of the repository would be useful if we ever want to use these fields in some automated procedure.

I will update the template for the PackageInfo.g file in the example package and ask package authors to update their PackageInfo.g files.