Semigroups created before package is loaded cause errors, incorrect results

[ManuelAFDelgado]

This issue is related to the examples I gave in a comment to Issue #537 started by Max (@fingolfin)
Hopefully these new examples help to find the problem (the main issue is that the result in the second example is wrong).

(The command "gapOF -b" below is used to start GAP 4.9.3 without banner; the package "semigroups" is compiled.)

The examples below were obtained from three different GAP sessions (as I just did copy/paste, the examples should be easily reproduced).

#Example1
This example gives an unexpected error...

mdelgado@mdelgado-dell:~$ gapOF -b
gap> g0:=Transformation([4,1,2,4]);;
gap> g1:=Transformation([1,3,4,4]);;
gap> g2:=Transformation([2,4,3,4]);;
gap> poi3:= Monoid(g0,g1,g2);;
gap> el := Transformation([1,2,4,4]);;
gap> LoadPackage("semi");
true
gap> dc := GreensDClassOfElement(poi3, el);;
gap> DisplayEggBoxOfDClass(dc);
Error, Record: '<rec>.data' must have an assigned value in
  comps := GreensXRelation( S )!.data.comps
 ; at /usr/local/lib/gap-4.9.3/pkg/semigroups-3.0.16/gap/greens/gren.gi:79 called from 
..........

#Example2
This example is similar to the previous one; the difference stays in the insertion of "DegreeOfTransformationSemigroup(poi3)". (Possibly other functions produce the same effect.)
Here there is a huge problem: a wrong result is silently displayed!! (Note that poi3 is an inverse semigroup (see the third example), thus the eggbox picture can not be the one presented)

mdelgado@mdelgado-dell:~$ gapOF -b
gap> g0:=Transformation([4,1,2,4]);;
gap> g1:=Transformation([1,3,4,4]);;
gap> g2:=Transformation([2,4,3,4]);;
gap> poi3:= Monoid(g0,g1,g2);;
gap> el := Transformation([1,2,4,4]);;
gap> LoadPackage("semi");
true
gap> DegreeOfTransformationSemigroup(poi3);;
gap> dc := GreensDClassOfElement(poi3, el);;
gap> DisplayEggBoxOfDClass(dc);
[ [  0,  0,  1 ],
  [  0,  0,  1 ],
  [  0,  0,  1 ] ]
gap> 

#Example3
This example is similar to the previous one; the difference stays in the order the "semigroups" package is loaded.

mdelgado@mdelgado-dell:~$ gapOF -b
gap> LoadPackage("semi");
true
gap> g0:=Transformation([4,1,2,4]);;
gap> g1:=Transformation([1,3,4,4]);;
gap> g2:=Transformation([2,4,3,4]);;
gap> poi3:= Monoid(g0,g1,g2);;
gap> el := Transformation([1,2,4,4]);;
gap> DegreeOfTransformationSemigroup(poi3);;
gap> dc := GreensDClassOfElement(poi3, el);;
gap> DisplayEggBoxOfDClass(dc);
[ [  1,  0,  0 ],
  [  0,  0,  1 ],
  [  0,  1,  0 ] ]
gap> IsInverseSemigroup(poi3);
true

[james-d-mitchell]

Thanks for the report @ManuelAFDelgado!

This is a bug, but I am not sure how to resolve it.

The problem is that if you create a semigroup S in GAP and then later load the Semigroups package, the internal data structures used by Semigroups are not present in S (how could they be, they didn't exist when S was created), that's what is happening in your Example 1. In Example 2, I'm not sure what is going on, I will investigate. In Example 3, everything seems to be working as expected.

Certainly, in Example 1, a more meaningful error could be given, or better still this method should just not be applied since your semigroup was created before Semigroups was loaded. Essentially, the Semigroups package was not designed to cope with the situation where a semigroup is created and then the package is loaded, this is an oversight that I will attempt to rectify.

[james-d-mitchell]

Postscript: I think there might be a warning in the manual that things might not work properly in precisely the situation you describe @ManuelAFDelgado, but I can't immediately find it (the manual is rather long these days!). So, it might be argued that this is a feature not a bug, but on the other hand, it seems that it would be best to remove this particular feature!

[ManuelAFDelgado]

I faced the kind of problems reported while I was trying to keep the sgpviz package working... I was warned by @alex-konovalov that the test file showed some differences depending on whether the semigroups package was loaded. I first thought that these differences were completely harmless (that they only depended on the way the factorizations were performed). But after other tests I realized that this was not the case and I thought that I should report the problems found, which I did.
In what concerns the sgpviz package I think that the problem is solved. (I fave to thank @fingolfin, who I am visiting, for several very fruitful discussions.) The way it was solved is: each function that has a semigroup as input, creates a new semigroup with the same generators... this way, when the semigroups package is loaded, the semigroup always becomes a "semigroups" semigroup. Of couse, this can be done in a package like sgpviz, which is a small package for which the performance is not an issue, but can not be taken as a general suggestion.
A naive question/sugestion: Why not create the semigroups in GAP as they are created in the semigroups package?

[james-d-mitchell]

A naive question/sugestion: Why not create the semigroups in GAP as they are created in the semigroups package?

Because we have specialised methods in Semigroups that don't exist in GAP itself, and these require special data structures that are not available in GAP.

This is definitely a bug, the methods in Semigroups should not be applied to any semigroup created before the package is loaded and your Example 1 shows that this is not what is happening.

For you, though this should be easy to resolve, you could make Semigroups a dependency of Sgpviz, that way Semigroups would always be loaded before Sgpviz; you could add Semigroups to your list of packages loaded automatically at startup in GAP; or you could just load it before loading Sgpviz.

Just out of interest, I believe that most of the functionality of sgpviz is already in Semigroups, is there something that sgpviz provides that is not in Semigroups?

[ManuelAFDelgado]

I resisted for now to put Semigroups as a required package for SgpViz in part because the (few) SgpViz users possibly do not feel comfortable with the need of compiling the package. Anyway, my resistance is mainly due to the fact that it would not solve my problem: the semigroups could be created before loading SgpViz...
Surprisingly, the GUIs Xsemigroup and XAutomaton (which are based in TcL/TK) are still working and there are at least two people using it...
In the first page of the SgpViz manual I wrote "The present package is superseded by the GAP package semigroups, by James Mitchel, in what concerns some aspects of semigroup visualisation. We strongly recommend the usage of that package, unless you find useful specific tools available in SgpViz but not in semigroups."... I have to check more carefully (hopefully in St Andrews, later this year) and then propose to include in Semigroups some features that are not there, but you may see as interesting. I think that one of them is (to be able) to display (draw) D-classes with the H-classes containing an idempotent disposed into blocks around the diagonal. (For that, I plan to provide a new implementation Graham's algorithm, since the one in SgpViz should be completely rewritten.)
It was not my plan to make any other release of SgpViz, but as some problems arise, I thought that I should let you know them, just to try to clearly identify its origin and to make sure that nothing serious is hidden.

[james-d-mitchell]

I understand your rationale, Semigroups isn't a small package to depend on. Just wanted to say that we have had D-classes in "Graham normal form" in Semigroups for some time now, so that feature at least doesn't necessarily need to be reimplemented.

[fingolfin]

Just a small remark from the sidelines: from my perspective as a GAP developer, I'd be hesitant to call this a bug in the Semigroups packages. I might call it a design issue with GAP, though even there I'd be careful and would like to think and debate more about this: Arguably, we want packages to use existing "infrastructure" from GAP, and e.g. provide their "own" semigroups in the GAP IsSemigroup filter. And it is very common that packages introduce new mathematical properties, and new implications (e.g. via InstallTrueMethod). Yet at the same time, GAP never tries to retroactively apply such newly introduced implications to existing objects (and it'd be difficult to do that, and arguably might cause its own set of issues, even if it was feasible).

So, while of course I agree it would be nice if the Semigroups package were to "do something" about this (by e.g. somehow detecting if a semigroup was created before, and then taking "appropriate steps", whatever that means: refuse to accept that input; update the input with any missing filters and attributes; etc.), I personally would also accept if you just said "this is a fundamental problem with the way things work in GAP; if you want to avoid that, and are working with semigroups regularly, then just add semigroups to your list of autoloaded GAP packages, and you'll never experience any such problems).

Anyway, that's really only meant to be my two cents, of course @james-d-mitchell and the rest of the Semigroup team will know best how they can and want to designate and handle this.

[ManuelAFDelgado]

The Semigroups package grew very much and much faster than I have been able to keep up with. In particular, I did not realize that of the Graham normal form. Sorry for that.
The two recent releases of SgpViz have been motivated by the need of not causing problems in GAP (last year it was the "SuperFail" that had to be removed and this year because of the test file showed some differences when the Semigroups package was loaded). As long as the package does not cause problems, I would like to keep it until taking some time to check whether there is some interesting feature not present in the Semigroups package and, if any, suggest to include it there. (I thought that the Graham normal for could be the case... Sorry, again.) It remains the GUI part, which I do not know what to do with. It is perhaps not very useful but, until it works... A couple of months ago, Alfredo Costa, when warned by me that SgpViz would most probably disappear soon, replayed that he would keep religiously a working version because he loved the idea of giving an automaton or a semigroup without having to care about the syntax...
Anyway, this is another topic for possible turther discussion. Here, the examples I gave do not depend on SgpViz... I just wanted to call the attention to something that I did not expect (and felt dangerous, because it seemed that all I was doing was legal and ended up with a wrong result). I accompany @fingolfin in the hesitation of whether it is a bug in the Semigroups package.
Many thanks @james-d-mitchell and @fingolfin for your patience.