McAlister triples #271

ChristopherRussell · 2017-04-25T14:58:04Z

This pull request implements the function McAlisterTriple which creates McAlister triple semigroups, along with basic operations for McAlister triples and their elements.

A McAlister triple is a representation of an E-unitary inverse semigroup defined by a partial order and a join semilattice, a group and a group. Amongst several other conditions, the semilattice is a suborder of the partial order and the group acts on them both.

TODO: improve the method for GeneratorsOfSemigroup for these objects in the future.

wilfwilson · 2017-05-30T19:54:14Z

@ChristopherRussell You'll need to update the required version of Digraphs. Your stuff uses things like IsPartialOrderDigraph. However these don't exist yet in Digraphs 0.7.1, which is the version required by the Semigroups package currently. To fix this, you should open the file PackageInfo.g and replace the occurrences of 0.7.1 with a newer version of Digraphs - I think 0.8.0 should work, although 0.8.1 has been released since then too. Once you've done this, the Travis tests should pass (I hope).

wilfwilson

Some initial comments about your documentation.

wilfwilson · 2017-07-27T11:10:05Z

doc/semieunit.xml

+      A <E>McAlister triple semigroup</E> is an E-unitary inverse semigroup
+      <Ref Oper= "IsEUnitaryInverseSemigroup" BookName= "Semigroups" />
+      defined by a partial order, a semilattice and a group which acts by
+      order automorphisms on the partial order. All E-unitary inverse


I think "Any E-unitary inverse semigroup..." sounds better.

Are you referring to line 19 or line 16?

The way it is written now could be interpreted as "there exists a mcalister triple semigroup S such that every e-unitary inverse semigroup is isomorphic to S".

wilfwilson · 2017-07-27T11:11:57Z

doc/semieunit.xml

+      <List>
+        <Mark> M1 </Mark>
+        <Item>
+          <C>Y</C> is a semilattice which is a subdigraph of <C>X</C>.


Change "which is" to "that is"

wilfwilson · 2017-07-27T11:14:53Z

doc/semieunit.xml

+      <A>G</A>, and a function <A>act</A> defining an action of <A>G</A> on
+      <A>X</A>. <C>McAlisterTripleSemigroup</C> also has a three argument
+      version which assumes that  <A>act</A> is <Ref Func= "OnPoints"
+      BookName= "GAPDoc"/>. Furthermore there are three and four argument


Shouldn't be GAPDoc here. Probably should be ref. Change it to ref and see if the warnings go away when you compile the documentation.

wilfwilson · 2017-07-27T11:16:09Z

doc/semieunit.xml

+    <Returns><K>true</K> or <K>false</K>.</Returns>
+    <Description>
+      A <E>McAlister triple semigroup</E> is an E-unitary inverse semigroup
+      <Ref Oper= "IsEUnitaryInverseSemigroup" BookName= "Semigroups" />


Get rid of the book name bit here. You don't need to do it when you're referring to stuff in the current package. If you've got BookName = "Semigroups" elsewhere then get rid of it too.

wilfwilson · 2017-07-27T11:17:42Z

doc/semieunit.xml

+        <Mark> M2 </Mark> 
+        <Item>
+          For all <C>A</C> in <C>X</C> and for all <C>B</C> in <C>Y</C>:
+          if <C>A</C> <M>\leq</M> <C>B</C> then <C>A</C> in <C>Y</C>.


Add a comma before then

wilfwilson · 2017-07-27T11:28:54Z

doc/semieunit.xml

+
+<#GAPDoc Label="McAlisterTripleSemigroup">
+  <ManSection>
+    <Oper Name = "McAlisterTripleSemigroup" Arg = "G, act, X, Y"


I think the order of arguments in the 4-argument versions should be the same as in the 3-argument versions.

So you have G, X, Y, act or G, X, Y, and similarly G, X, sub_ver, act and G, X, sub_ver.

wilfwilson · 2017-07-27T11:33:14Z

doc/semieunit.xml

+    <Prop Name = "IsMcAlisterTripleSemigroup" Arg = "S"/>
+    <Returns><K>true</K> or <K>false</K>.</Returns>
+    <Description>
+      A <E>McAlister triple semigroup</E> is an E-unitary inverse semigroup


You should talk with James already about how to organise this documentation. My intuition would be to have this part of the doc explain literally what filter IsMcAlisterTripleSemigroup describes (technically), and for all this mathematical info to be in the documentation for the creation functions.

@james-d-mitchell will have better advice about this.

You also need to add doc for everything you defined, including IsMcAlisterTripleSemigroupElementRep etc.

I currently have the mathematical details in z-chap15.xml at the start of my section and some of it mentioned again later. Should I rearrange like this:

-Introduction without too much detail at start of the section
-Technical details in IsMcAlisterTripleSemigroup section
-Mathematical details in McAlisterTripleSemigroup section

Ah okay I hadn't realised. However it should work, it should be clear, when I type ?IsMcAlisterTripleSemigroup what the documentation means - if it relies on notions defined elsewhere, then it should say so and link to that place.

wilfwilson · 2017-07-27T11:35:16Z

doc/semieunit.xml

+      versions where the homogeneous list <A>sub_ver</A> of vertices of
+      <A>X</A> replaces <A>Y</A> as an argument. When <A>sub_ver</A> is
+      provided, a McAlister triple semigroup is created with semilattice
+      <C>InducedSubdigraph(<A>X</A>, <A>sub_ver</A>)</C>. <P/>


If I read this documentation, I have absolutely no idea what properties the arguments must satisfy.

Sorry this is too harsh! What I meant is that, from this bit of text, it's not clear how G, X, Y etc have to be related. If all you did was type ?McAlisterTripleSemigroup into the command line, it wouldn't be obvious where to get these details.

wilfwilson · 2017-07-27T11:36:36Z

doc/semieunit.xml

+      Returns the <E>McAlister triple semigroup</E> element of the McAlister
+      triple semigroup <A>S</A> which corresponds to the semilattice vertex
+      <A>A</A> and group element <A>g</A>, if this specifices an element of
+      <A>S</A>. The functions <Ref Prop= "MTE"/> and <Ref Prop= 


You've called them Props here - are they?

wilfwilson · 2017-07-27T11:36:52Z

doc/semieunit.xml

+      triple semigroup <A>S</A> which corresponds to the semilattice vertex
+      <A>A</A> and group element <A>g</A>, if this specifices an element of
+      <A>S</A>. The functions <Ref Prop= "MTE"/> and <Ref Prop= 
+        "McAlisterTripleSemigroupElement"/> are identicial.


Can you declare them as synonyms? You should if possible.

These should be referred to as operations. MTE is just:

function(S, A, g)
return McAlisterTripleSemigroupElement(S, A, g);
end);

Is that ok?

In the gd file one of them should be declared as an operation, yes (probably the one with the long name). The other one, MTE, should be declared using DeclareSynonym (see this chapter of the GAP manual) for how to do it - pretty sure that is the right way of doing it.

Since this is the piece of the documentation that is about these things, you don't need to have these bits as Refs here. This sentence can be replaced by: <C>MTE</C> is a synonym for <C>McAlisterTripleSemigroupElement</C>, or something similar.

You've also spelt identical wrongly.

wilfwilson · 2017-07-27T13:34:00Z

OneImmutable for a McAlister triple semigroup has no code coverage.

ChristopherRussell · 2017-07-31T14:43:50Z

Thank you @wilfwilson for all the good suggestions. I have acted on them all but I am still deciding on the organisation of the mathematical and technical details amongst the doc at start of the section as well as the doc for IsMcAlisterTripleSemigroup and McAlisterTripleSemigroup. Will post again when this is done.

wilfwilson · 2017-07-31T19:51:19Z

Thanks @ChristopherRussell, I'll take a look at it again sometime soon. James will be able to give you some guidance about how best to organise the documentation - it's certainly a good idea to give it some thought.

wilfwilson

I've done a bit more reviewing. Mostly focuses on missing/unlinked documentation, and questions that I have about your some of code (I can't work out what it all does/is meant to do).

wilfwilson · 2017-08-01T09:38:17Z

doc/semieunit.xml

+    <Oper Name = "AsMcAlisterTripleSemigroup" Arg = "S"/>
+    <Returns>A McAlister triple semigroup.</Returns>
+    <Description>
+      This function can be used to find a McAlister triple semigroup which is


You should say this this only works for e-unitary inverse semigroups, and have a reference to <Ref Prop="IsEUnitaryInverseSemigroup"/>.

I have just realised. IsomorphismMcAlisterTripleSemigroup should by implemented as IsomorphismSemigroup and I think AsSemigroup with filter IsMcAlisterTripleSemigroup will then work based on that. Does this sound right?

Oh yes, these should be intregrated into IsomorphismSemigroup and AsSemigroup, as you say.

wilfwilson · 2017-08-01T09:39:03Z

doc/semieunit.xml

+    <Description>
+      This function can be used to find an isomorphism from a given semigroup
+      <A>S</A> to an isomorphic McAlister triple semigroup, provided such an
+      isomorphism exists.


As above, you can be more specific here. You should say this this only works for e-unitary inverse semigroups, and have a reference to <Ref Prop="IsEUnitaryInverseSemigroup"/>

wilfwilson · 2017-08-01T09:42:06Z

doc/semieunit.xml

+      the partial order digraph <C>X</C> is a join-semilattice digraph. This
+      function uses <Ref Oper = "AsMcAlisterTripleSemigroup"/> to find a
+      McAlister triple semigroup isomorphic to <A>S</A> then checks if the
+      McAlister triple semigroup is F-inverse.


I think you should change this documentation - the last sentence in particular. We don't normally put implementational details in the doc, unless that information is particularly relevant to the user and might change the way they want to use the function.

I think it might be nicer if you can characterise this in a more elementary way. Is it true that an F-inverse semigroup is an E-unitary inverse semigroup in which the partial order of D-classes defines a join-semilattice? If so, that would be a shorter way of describing it, and you could include references to IsEUnitaryInverseSemigroup, PartialOrderOfDClasses, etc.

I hadn't thought of that characterisation and it seems much less complicated that the typical definition of F-Inverse semigroup. I will investigate and get back to you.

Actually this idea doesn't work.

McAlisterTripleSemigroup(Group((4,5)), Digraph([[1],[1,2],[1,3],[1,2,3,4],[1,2,3,5]]), [1 .. 4]);

is not an F-inverse semigroup since the 2nd argument is not a join-semilattice yet the partial order of D-classes

[ [ 1 ], [ 1, 2 ], [ 1, 3 ], [ 1, 2, 3, 4 ] ]

defines a join-semilattice digraph.

gap> Digraph([[1],[1,2],[1,2,3],[1,2,3,4],[1,2,3,5]]); <digraph with 5 vertices, 14 edges> gap> IsJoinSemilatticeDigraph(last); true

It says it's a join-semilattice for me

Sorry, the Digraph was meant to be Digraph([[1],[1,2],[1,3],[1,2,3,4],[1,2,3,5]]) with no edge from 3 to 2. Then there is no join for vertices 4 & 5.

Okay good work, you'll have to go back to your old method then.

wilfwilson · 2017-08-01T09:47:21Z

doc/z-chap15.xml

+    the subsemigroup of a semigroup consisting of its idempotents by <C>E</C>.
+    A semigroup <C>S</C> is said to be <E>E-unitary</E> if for all <C>e</C> in
+    <C>E</C> and for all <C>s</C> in 
+    <C>S</C> we have that:


You can get rid of 'we have that'

wilfwilson · 2017-08-01T09:50:18Z

gap/semigroups/semieunit.gd

+DeclareOperation("ELM_LIST", [IsMcAlisterTripleSemigroupElementRep, IsPosInt]);
+
+# F-inverse semigroup property
+DeclareOperation("IsFInverseSemigroup", [IsSemigroup]);


This should be a property rather than an operation

wilfwilson · 2017-08-01T11:11:17Z

gap/semigroups/semieunit.gi

+                  "the fourth argument must be a join semilattice digraph,");
+  fi;
+
+  # Check condition M2 (check that Y is an order ideal of X.)


M2 says that if x in X is less than some y in Y, then x is in Y. Could you explain to me what the next block of code is doing? I can't work it out.

wilfwilson · 2017-08-01T11:12:19Z

gap/semigroups/semieunit.gi

+  for x in DigraphVertexLabels(X) do
+    if not x in DigraphVertexLabels(Y) then
+      for y in DigraphVertexLabels(Y) do
+        if Intersection(out_nbrs[x], out_nbrs[y]) = out_nbrs[x] then


This bit is especially confusing to me.

For each vertex of X which does not correspond to a vertex of Y we check whether there are any vertices which are 'less' than it which correspond to a vertex of Y. Vertex a is less than vertex b when there is an edge from b to a.
The intersection of the out neighbours of two vertices is equal to their join. The join of a vertex in X with a vertex in Y is either less than or equal to a vertex of Y or is equal to that vertex in X. I should be using PartialOrderDigraphJoinOfVertices here!

So you're doing this by contrapositive basically: for each vertex of X that is not in Y, if the vertex is less than something in Y, then we fail.

Since X is a partial order, it is transitive. So if we want to know whether a is less than b, we can simply check whether b is in the out-neighbours of a, rather than doing intersections, right?

There's also another vertex number/label of X conundrum here: x is one of the vertex labels of X, not necessarily the actualy vertex number. However, out_nbrs[x] gives the out-neighbours (as vertex numbers, not labels) of the vertex whose number (not label) is x. Do you see the problem?

i.e x might be vertex 1, however it could have label 2. And out_nbrs[x] is out_nbrs[2], which might have no relation to out_nbrs[1], which describes the actual out-neighbours of x.

You are correct regarding just checking out-neighbours vs intersections.

I see that using the labels of X is a bit of an issue and I don't really recall why I chose to do so now. Using the labels of Y was necessary to know how Y embeds into X but perhaps I used them mistakingly here or thought the user might like to label their digraph! I think it should be changed to loop over DigraphVertices(X) instead of the labels.

I agree. Using vertex labels of Y makes sense, but using the vertex labels of X seems to complicate things. So the label of a vertex of Y should refer to a vertex number of a vertex of X. There might be a few parts of your code where you'll need to check that the right thing is being done, and of course the documentation needs to be precise.

wilfwilson · 2017-08-01T11:14:02Z

gap/semigroups/semieunit.gi

+  fi;
+
+  iso_x := IsomorphismDigraphs(McAlisterTripleSemigroupPartialOrder(S),
+                               McAlisterTripleSemigroupPartialOrder(T));


Just wondering: are you assuming that iso_x (an iso between the partial orders) restricts to an isomorphism between the respective semilattice sub digraphs?

You are right, I need to check that. Should I do this by using RestrictedMapping... then is there something existing which can check if this is an isomorphism?

Not sure how is best to do this. Certainly iso_x is an isomorphism from the X for S to the X for T, and iso_x embeds the Y for S into some copy in the X for T. However, perhaps it gets mapped to a different one than the actual Y for T. If there exists some iso_x however, then there must exist one that does what you want it to. The question is how do you get your hands on it? Not sure right now.

I will comment this function out until we can think of something.

wilfwilson · 2017-08-01T11:14:38Z

gap/semigroups/semieunit.gi

+[IsMcAlisterTripleSemigroup and HasGeneratorsOfSemigroup,
+IsMcAlisterTripleSemigroup and HasGeneratorsOfSemigroup],
+function(S, T)
+  if IsomorphismSemigroups(S, T) = fail then


this function could be shortened to return IsomorphismSemigroups(S, T) <> fail;

wilfwilson · 2017-08-01T11:17:23Z

gap/semigroups/semieunit.gi

+InstallMethod(IsFInverseMonoid, "for a McAlister triple semigroup",
+[IsMcAlisterTripleSemigroup],
+function(S)
+  return IsFInverseSemigroup(S) and IsMonoid(S);


Swap these conditions around. It's easier to check IsMonoid (in fact it's immediate) so it should come first.

wilfwilson · 2017-08-01T11:22:53Z

In the master branch, the required version of Digraphs has been increased to 0.10.0. This change will conflict with your change, which increased the version to 0.8.1. To resolve this conflict, go with 0.10.0.

ChristopherRussell · 2017-08-03T12:42:01Z

I made some commits but I'm not finished yet. I still need to:

-Fix IsomorphismSemigroups for two McAlister triple semigroups, as discussed above not sure how to do this.
-Document McAlisterTripleSemigroupElementReps

ChristopherRussell · 2017-08-07T10:29:50Z

I tried searching for other examples of Reps. There is IsEnumerableSemigroupRep but other examples I found by searching .gd files (ag DeclareRepresentation -G .gd) were undocumented:

IsEnumerableSemigroupGreensClassRep
IsEnumerableSemigroupGreensRelationRep
IsPlistRowBasisOverFiniteFieldRep
IsPlistMatrixOverFiniteFieldRep
IsCongruenceByGeneratingPairsRep
IsCongruenceClassByGeneratingPairsRep

I am not sure how to document this rep. The only thing I can think to say in the doc for McAlisterTripleSemigroupElementReps is that these are objects created by McAlisterTripleSemigroupElement. Anything about McAlister triple semigroups and their elements would be repeating things I have already covered elsewhere.

@wilfwilson @james-d-mitchell

ChristopherRussell · 2017-08-16T15:54:53Z

I have now pushed a fix to IsomorphismSemigroups for two McAlisterTripleSemigroup semigroups, added comments in the .gd file describing McAlisterTripleSemigroupElementRep and McAlisterTripleSemigroupDefaultRep (as opposed to writing documentation for them) and created a new chapter (now chapter 12) for McAlister triple semigroup documentation.

wilfwilson

Good work @ChristopherRussell, my comments now are mostly just correcting typos.

The are two serious things: there's still a problem with IsomorphismSemigroups. I suggested a fix on the relevant bit of code. This is an example of the problem:

gap> gr := DigraphFromDigraph6String("+H_A?GC_Q@G~wA?G");
<digraph with 9 vertices, 20 edges>
gap> G := Group((1,2,3)(4,5,6), (8,9));
Group([ (1,2,3)(4,5,6), (8,9) ])
gap> S1 := McAlisterTripleSemigroup(G, gr, [1, 4, 5, 7, 8]);
<McAlister triple semigroup over Group([ (1,2,3)(4,5,6), (8,9) ])>
gap> S2 := McAlisterTripleSemigroup(G, gr, [3, 6, 7, 8, 9]);
<McAlister triple semigroup over Group([ (1,2,3)(4,5,6), (8,9) ])>
gap> IsomorphismSemigroups(S1, S2);
Error, no method found! For debugging hints type ?Recovery from NoMethodFound
The 1st argument is 'fail' which might point to an earlier problem
Error, no 1st choice method found for `*' on 2 arguments at /Users/Wilf/GAP/lib/methsel2.g:241 called from
rep
* iso_x
  at /Users/Wilf/GAP/pkg/semigroups/gap/semigroups/semieunit.gi:260 called fro\
m
func( elm ) at /Users/Wilf/GAP/lib/coll.gi:1613 called from
ForAll( DigraphVertices( McAlisterTripleSemigroupPartialOrder( S ) ),
 function ( x )
      return McAlisterTripleSemigroupAction( S )( x, g ) ^ (rep * iso_x)
        = McAlisterTripleSemigroupAction( T )( x ^ iso_x, g ^ iso_g ) ^ rep;
  end
 ) at /Users/Wilf/GAP/pkg/semigroups/gap/semigroups/semieunit.gi:261

The problem is that you can have two MTS's with the same partial orders and groups, and with isomorphic semilattices, but where there is no automorphism of the whole digraph that takes the first semilattice to the second. Therefore the rep that you get by using RepresentativeAction could be fail.

The second problem is that Print doesn't seem to work (using the above semigroup S1) in all cases:

gap> Print(S1);
Semigroup(
[
  MTSE(McAlisterTripleSemigroup(Group( [ (1,2,3)(4,5,6), (8,9) ] ), Digraph( [ \
[ 1, 4, 7 ], [ 2, 5, 7 ], [ 3, 6, 7 ], [ 4, 7 ], [ 5, 7 ], [ 6, 7 ], [ 7 ], [ \
7, 8 ], [ 7, 9 ] ] ), Digraph( [ [ 1, 2, 4 ], [ 2, 4 ], [ 3, 4 ], [ 4 ], [ 4, \
5 ] ] )), 1, ()),
  MTSE(McAlisterTripleSemigroup(Group( [ (1,2,3)(4,5,6), (8,9) ] ), Digraph( [\
 [ 1, 4, 7 ], [ 2, 5, 7 ], [ 3, 6, 7 ], [ 4, 7 ], [ 5, 7 ], [ 6, 7 ], [ 7 ], [\
 7, 8 ], [ 7, 9 ] ] ), Digraph( [ [ 1, 2, 4 ], [ 2, 4 ], [ 3, 4 ], [ 4 ], [ 4,\
 5 ] ] )), 7, ()),
  MTSE(McAlisterTripleSemigroup(Group( [ (1,2,3)(4,5,6), (8,9) ] ), Digraph( [\
 [ 1, 4, 7 ], [ 2, 5, 7 ], [ 3, 6, 7 ], [ 4, 7 ], [ 5, 7 ], [ 6, 7 ], [ 7 ], [\
 7, 8 ], [ 7, 9 ] ] ), Digraph( [ [ 1, 2, 4 ], [ 2, 4 ], [ 3, 4 ], [ 4 ], [ 4,\
Error, List Element: <list>[7] must have an assigned value in
   5 ] ] )), 8, ()),
  return Concatenation( "MTSE(", String( x![3] ), ", ",
       String(
         DigraphVertexLabels( McAlisterTripleSemigroupSemilattice( x![3] ) )[
           x[1]] ), ", ", String( x[2] ), ")" )
     ; at /Users/Wilf/GAP/pkg/semigroups/gap/semigroups/semieunit.gi:323 called from
PrintString( o )

wilfwilson · 2017-08-17T11:12:23Z

doc/z-chap12.xml

+    For inverse semigroups these two conditions are equivalent. We are only
+    interested in <E>E-unitary inverse semigroups</E>. 
+    Before defining McAlister triple semigroups we define a McAlister triple.
+    A <E>McAlister triple</E> is a triple <C>(G,X,Y)</C> which consisting of:


which consisting of -> which consists of

wilfwilson · 2017-08-17T11:14:25Z

doc/z-chap12.xml

+
+    where <C>Join</C> is the natural join operation of the semilattice and
+    <C>Bg^-1</C> is <C>B</C> acted on by the inverse of <C>g</C>. With this
+    operation, <C>M(G,X,Y)</C> is semigroup which we call a <E>McAlister triple


is semigroup -> is a semigroup

wilfwilson · 2017-08-17T11:15:24Z

doc/semieunit.xml

+    <Description>
+
+    The following documentation covers the technical information needed to
+    create McAlister triple semigroups in GAP, the underlying theorey can be


theorey -> theory

wilfwilson · 2017-08-17T11:15:37Z

doc/semieunit.xml

+
+    The following documentation covers the technical information needed to
+    create McAlister triple semigroups in GAP, the underlying theorey can be
+    read in the introduction of Section


Section -> Chapter
of -> to

wilfwilson · 2017-08-17T11:16:34Z

doc/semieunit.xml

+    <Ref Chap = "McAlister triple semigroups"/>.
+    <P/>
+
+    In this implementation the partial order <C>x</C> of a McAlister triple is 


I think x should be capitalised as X here (since you use <C>X</C> in the introduction to Chapter 12)

wilfwilson · 2017-08-17T12:04:24Z

gap/semigroups/semieunit.gi

+  return fail;
+end);
+
+InstallMethod(IsIsomorphicSemigroup, "for two McAlister triple semigroups",


You can get rid of this method: the one in gap/attributes/isomorph.gi works perfectly fine for MTSs.

wilfwilson · 2017-08-17T12:08:59Z

gap/semigroups/semieunit.gi

+  return Concatenation("<McAlister triple semigroup over ", ViewString(G), ">");
+end);
+
+# Currently this does not work. iso_x restricted to the semilattice is not


Is this comment out of date now?

wilfwilson · 2017-08-17T12:30:34Z

gap/semigroups/semieunit.gi

+  # iso_x will restrict to an isomorphism from (the labels of) YS to YT.
+  if not im_YS = DigraphVertexLabels(YT) then
+    A := AutomorphismGroup(XT);
+    rep := RepresentativeAction(A, im_YS, DigraphVertexLabels(YT), OnSets);


It is possible that RepresentativeAction returns fail here: in this case, the method should return fail.
i.e

if rep = fail then return fail; fi;

I'll post an example below.

Thanks for spotting this and the example.

wilfwilson · 2017-08-17T12:31:07Z

gap/semigroups/semieunit.gi

+function(S, A, g)
+  if not A in DigraphVertexLabels(McAlisterTripleSemigroupSemilattice(S)) then
+    ErrorNoReturn("Semigroups: McAlisterTripleSemigroupElement: usage,\n",
+                  "second input should be a vertex label of the ",


input -> argument

wilfwilson · 2017-08-17T12:31:15Z

gap/semigroups/semieunit.gi

+                  "join-semilattice of the McAlister triple,");
+  elif not g in McAlisterTripleSemigroupGroup(S) then
+    ErrorNoReturn("Semigroups: McAlisterTripleSemigroupElement: usage,\n",
+                  "third input must an element of the group of the McAlister ",


input -> argument

ChristopherRussell · 2017-08-29T08:50:02Z

I have made the most recent suggested changes now @wilfwilson. Apologies for the delay, I was away and without my laptop.

wilfwilson · 2017-08-29T15:13:26Z

@james-d-mitchell Can you tell me what you think the correct behaviour should be here?

I'll create two McAlisterTripleSemigroup objects:

gap> S := McAlisterTripleSemigroup(Group([(1, 2, 3)(4, 5, 6), (8, 9)]),
>  DigraphFromGraph6String("+H_A?GC_Q@G~wA?G"), [1, 4, 5, 7, 8]);
gap> T := McAlisterTripleSemigroup(Group([(1, 2, 3)(4, 5, 6), (8, 9)]),
>  DigraphFromGraph6String("+H_A?GC_Q@G~wA?G"), [1, 4, 5, 7, 8]);

S and T are mathematically identical, because they're created in exactly the same way. But they're different objects, and their elements belong to different families, since each McAlisterTripleSemigroup has its own family. However, the equality method for these two objects returns true:

gap> S = T;
true

If I create two mathematically identical objects in each of these semigroups:

s := MTSE(S, 1, ());
t := MTSE(T, 1, ());

then, again, the equality method for McAlisterTripleSemigroupElements will say that they are equal, since their semigroups and parameters are equal:

gap> s = t;
true

But then the in method is not consistent with this:

gap> s in S and t in T;
true
gap> s in T or t in S;
false

This has knock-on implications, such as when it comes to re-creating something from its Print or String value. If String gives the following:

"Semigroup([
  MTSE(McAlisterTripleSemigroup(SymmetricGroup([2 .. 5]),
        Digraph([[1], [1, 2], [1, 3], [1, 4], [1, 5]]), [1 .. 4]), 3, (3, 4)),
  MTSE(McAlisterTripleSemigroup(SymmetricGroup([2 .. 5]),
        Digraph([[1], [1, 2], [1, 3], [1, 4], [1, 5]]), [1 .. 4]), 1, (2, 4))])"

then EvalString(last) fails for obvious reasons: the two generators are created relative to two semigroups that, although mathematically the same, have different families, and you can't create a semigroup out of objects in different families.

How should things be? Certainly I think that the behaviour should be consistent, and would preferably allow a McAlisterTripleSemigroup or subsemigroup to be re-created from its String, but I'm not sure if that's possible.

wilfwilson · 2017-08-29T15:34:55Z

@ChristopherRussell I made some minor changes following my most recent review, and I've put them in a pull request to your e-unitary branch in your fork of the repository. So you just need to merge that once to get those changes. They'll then appear as part of this pull request.

Once you merge in my PR, and once @james-d-mitchell and I have resolved the way that equality should work with MTS's and MTSE's, I'm happy for this to be merged in. I'm hoping to do it at the Wednesday meeting tomorrow.

james-d-mitchell · 2017-08-29T20:39:19Z

Semigroups consist of sets, and two sets are equal if and only if they have there same elements. Therefore the correct solution is for elements of distinct McAlister triples should never be equal. I think this is the case with Rees matrix semigroups, and it would be consistent if McAlister triples behaved the same. I don't see how to fix the string method. I'll write some more about this tomorrow

wilfwilson · 2017-08-29T20:52:45Z

I agree.

…

On 29 Aug 2017, at 21:39, James Mitchell ***@***.***> wrote: Semigroups consist of sets, and two sets are equal if and only if they have there same elements. Therefore the correct solution is for elements of distinct McAlister triples should never be equal. I think this is the case with Rees matrix semigroups, and it would be consistent if McAlister triples behaved the same. I don't see how to fix the string method. I'll write some more about this tomorrow — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

This commit moves the documentation for McAlister triple semigroups to their own chapter - Chapter 12. It also re-implements IsomorphismSemigroups, which previously was not working, and adds more tests for it.

ChristopherRussell · 2017-08-30T20:39:32Z

I made the fix to the equality methods and changed tests accordingly.

wilfwilson · 2017-08-30T21:16:31Z

@ChristopherRussell Can you push your changes please?

ChristopherRussell · 2017-08-31T07:55:14Z

I did push them. I put them in with my last commit (95e1d9f). It's the one before your fix to String. Sorry if that was confusing.

wilfwilson · 2017-08-31T11:47:52Z

Thanks @ChristopherRussell

james-d-mitchell · 2017-08-31T11:57:31Z

Hmm, I have some comments on this too. @ChristopherRussell can you make a new pull request to fix the one or two minor issues I am about to point out?

james-d-mitchell

Please review my two comments, and make a new pull request to fix these minor issues (if necessary).

james-d-mitchell · 2017-08-31T11:59:36Z

gap/semigroups/semieunit.gi

+#############################################################################
+# Methods for McAlister triple semigroups
+#############################################################################
+# InstallMethod(\=, "for two McAlister triple semigroups",


This commented out method should either be deleted, or you could include this in the isomorphism semigroup code if it would be worthwhile doing this.

james-d-mitchell · 2017-08-31T12:01:19Z

gap/semigroups/semieunit.gd

+# This is a representation for McAlister triple semigroup elements, which are
+# created via the function McAlisterTripleSemigroupElement.
+#
+# The components are:


This says that there are 3 components, it should really say that there are 3 positions, and what is stored in each position, and then in DeclareRepresentation you only declare 2 positions, so which is it, 2 or 3?

The components 'Vertex' and 'Group element' are accessible by [] but parent is not. Instead parent is accessed via McAlisterTripleSemigroupElementParent (at some point we decided it was not user friendly for it to be accessible by [] because these elements are printed as pairs, not triples).

Should there be 2 positions declared or 3?
(Is this number the positions accesible by [] or the ones accessible by ![]?)

It should be a description of the internal representation, so the ones accessible by ![], and so it should be 2 positions, according to your description.

There are 3 positions accessible by ![]. Did you mean to say it should be 3?

james-d-mitchell · 2017-08-31T13:01:46Z

Oh sorry I misunderstood what you wrote, yes, 3.

…

On Thu, 31 Aug 2017 at 13:58 ChristopherRussell ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In gap/semigroups/semieunit.gd <#271 (comment)> : > +## +#W semieunit.gd +#Y Copyright (C) 2016 Christopher Russell +## +## Licensing information can be found in the README file of this package. +## +############################################################################# +## + +DeclareCategory("IsMcAlisterTripleSemigroupElement", + IsAssociativeElement and IsMultiplicativeElementWithInverse); + +# This is a representation for McAlister triple semigroup elements, which are +# created via the function McAlisterTripleSemigroupElement. +# +# The components are: There are 3 positions accessible by ![]. Did you mean to say it should be 3? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#271 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGYFd7k9tkC6CmB1FBJ9ItQ7D2HenXvOks5sdq20gaJpZM4NHnKY> .[image: Web Bug from https://github.com/notifications/beacon/AGYFd7la1dHfVvbHi6Q-hmklIfhhZmboks5sdq20gaJpZM4NHnKY.gif] {"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/gap-packages/Semigroups","title":"gap-packages/Semigroups","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png ","avatar_image_url":" https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in ***@***.*** commented on #271"}],"action":{"name":"View Pull Request","url":" #271 (comment) "}}}

mtorpey added the 3.1 label May 29, 2017

ChristopherRussell force-pushed the e-unitary branch 2 times, most recently from 416b709 to 393e90b Compare June 5, 2017 15:57

mtorpey added the do not merge Label for PR that should not be merged label Jun 14, 2017

ChristopherRussell changed the title ~~McAlister triples~~ McAlister triples - work in progress Jun 14, 2017

ChristopherRussell force-pushed the e-unitary branch 7 times, most recently from 48821c4 to 33cc4cc Compare June 22, 2017 10:50

ChristopherRussell changed the title ~~McAlister triples - work in progress~~ McAlister triples - ready for review Jul 19, 2017

mtorpey removed the do not merge Label for PR that should not be merged label Jul 19, 2017

wilfwilson requested changes Jul 27, 2017

View reviewed changes

wilfwilson changed the title ~~McAlister triples - ready for review~~ McAlister triples Jul 27, 2017

wilfwilson requested changes Aug 1, 2017

View reviewed changes

ChristopherRussell force-pushed the e-unitary branch 2 times, most recently from 498a632 to 79ed059 Compare August 3, 2017 12:26

ChristopherRussell force-pushed the e-unitary branch from 79ed059 to 3a85299 Compare August 4, 2017 13:20

ChristopherRussell force-pushed the e-unitary branch 2 times, most recently from a972a7b to 6816c25 Compare August 7, 2017 14:10

wilfwilson requested changes Aug 17, 2017

View reviewed changes

ChristopherRussell added 6 commits August 28, 2017 16:53

semieunit: implement McAlister triple semigroups

d7f99d5

tests: add tests for McAlister triple semigroups

07a70e1

doc: document McAlister triple semigroups and their methods

78623f3

semieunit: let McAlister semigroups know if they are monoids

303fb42

semieunit: changes to doc, method names and argument orders.

7f10aa3

semieunit: improvements to doc and to McAlisterTripleSemigroup

cf7b780

ChristopherRussell force-pushed the e-unitary branch 2 times, most recently from 753090d to 2d1af62 Compare August 28, 2017 17:23

wilfwilson approved these changes Aug 29, 2017

View reviewed changes

ChristopherRussell force-pushed the e-unitary branch from e4ad1f2 to 44344f7 Compare August 30, 2017 14:24

ChristopherRussell and others added 2 commits August 30, 2017 15:59

semieunit: moved doc location, fix to IsomorphismSemigroups

95e1d9f

This commit moves the documentation for McAlister triple semigroups to their own chapter - Chapter 12. It also re-implements IsomorphismSemigroups, which previously was not working, and adds more tests for it.

semieunit: fix to String for an MTSE

471f45f

ChristopherRussell force-pushed the e-unitary branch from 44344f7 to 471f45f Compare August 30, 2017 14:59

wilfwilson merged commit f298a0c into semigroups:master Aug 31, 2017

james-d-mitchell reviewed Aug 31, 2017

View reviewed changes

McAlister triples #271

McAlister triples #271

Conversation

ChristopherRussell commented Apr 25, 2017

wilfwilson commented May 30, 2017

wilfwilson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherRussell Jul 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilfwilson Jul 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilfwilson commented Jul 27, 2017

ChristopherRussell commented Jul 31, 2017

wilfwilson commented Jul 31, 2017

wilfwilson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherRussell Aug 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherRussell Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilfwilson Aug 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilfwilson commented Aug 1, 2017

ChristopherRussell commented Aug 3, 2017 • edited Loading

ChristopherRussell commented Aug 7, 2017 • edited Loading

ChristopherRussell commented Aug 16, 2017

wilfwilson left a comment

Choose a reason for hiding this comment

wilfwilson Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherRussell Jul 27, 2017 •

edited

Loading

wilfwilson Jul 27, 2017 •

edited

Loading

ChristopherRussell Aug 7, 2017 •

edited

Loading

ChristopherRussell Aug 8, 2017 •

edited

Loading

wilfwilson Aug 1, 2017 •

edited

Loading

ChristopherRussell commented Aug 3, 2017 •

edited

Loading

ChristopherRussell commented Aug 7, 2017 •

edited

Loading

wilfwilson Aug 17, 2017 •

edited

Loading