-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make congruence poset object a digraph #385
Conversation
That's not what you've implemented:
Which do you want? |
My documentation for
which is what I intend. My description in the first comment of this PR is wrong. |
Thanks for clearing that up @mtorpey. My comments in this post are all about making sure that the behaviour of your posets is consistent with Digraphs. Here's the documentation:
You should make your posets reflexive, since a congruence contains itself. Furthermore, it makes sense to do this for the following reason. Your posets are currently antisymmetric and transitive, but since they're not reflexive,
I think it's natural to expect a poset of congruences to satisfy More fundamentally, you should change the definition of the partial order that is used for your posets. In your setup,
So, taking this all together, The join of two elements partially ordered by less-than-or-equal is their least unique upper bound. Therefore, with your congruences, the join of two elements
Let me know if you want more convincing about this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've review your PR, and it looks good. Most of my comments are things that will need to change once you've switched the direction of the partial order.
gap/congruences/conglatt.gi
Outdated
return poset; | ||
end; | ||
|
||
InstallMethod(MinimalCongruences, | ||
"for a congruence poset", | ||
[IsCongruencePoset], | ||
poset -> CongruencesOfPoset(poset){DigraphSinks(poset!.po)}); | ||
poset -> CongruencesOfPoset(poset){DigraphSinks(poset)}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you add loops at every vertex, you'll have to change this. Options: vertices with out-degree 1, or vertices with in-degree 1. Or remove loops and then get the sinks.
obtained using <Ref Attr = "CongruencesOfPoset"/>. | ||
A congruence poset is a digraph | ||
(see <Ref Filt = "IsDigraph" BookName = "Digraphs"/>) with a vertex for | ||
each congruence, and an edge from vertex <C>i</C> to vertex <C>j</C> if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swap this
doc/conglatt.xml
Outdated
gap> IsDigraph(poset); | ||
true | ||
gap> OutNeighbours(poset); | ||
[ [ ], [ 1 ], [ 1, 2, 4 ], [ 1, 2 ] ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will presumably change.
doc/conglatt.xml
Outdated
gap> LatticeOfRightCongruences(S); | ||
<poset of 5 congruences over <regular transformation monoid | ||
of size 3, degree 2 with 2 generators>> | ||
gap> OutNeighbours(LatticeOfRightCongruences(S)); | ||
[ [ ], [ 1 ], [ 1 ], [ 1 ], [ 1, 2, 3, 4 ] ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will presumably change too.
gap/congruences/conglatt.gi
Outdated
children := OutNeighboursMutableCopy(poset!.po); | ||
parents := InNeighboursMutableCopy(poset!.po); | ||
children := OutNeighboursMutableCopy(poset); | ||
parents := InNeighboursMutableCopy(poset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swap this
gap/congruences/conglatt.gi
Outdated
children := OutNeighboursMutableCopy(poset!.po); | ||
parents := InNeighboursMutableCopy(poset!.po); | ||
children := OutNeighboursMutableCopy(poset); | ||
parents := InNeighboursMutableCopy(poset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swap
gap/congruences/conglatt.gi
Outdated
SetInNeighbours(po, parents); | ||
poset := Objectify(NewType(FamilyObj(children), IsCongruencePoset), | ||
rec(po := po)); | ||
poset := Digraph(children); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same swap
gap/congruences/conglatt.gi
Outdated
SetInNeighbours(po, parents); | ||
poset := Objectify(NewType(FamilyObj(children), IsCongruencePoset), | ||
rec(po := po)); | ||
poset := Digraph(children); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same swap
gap/congruences/conglatt.gi
Outdated
fi; | ||
|
||
rel := List([1 .. Length(poset)], x -> Filtered(poset[x], y -> x <> y)); | ||
rel := List([1 .. Length(out_nbs)], x -> Filtered(out_nbs[x], y -> x <> y)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to rel := InNeighbours(DigraphReflexiveTransitiveReduction(poset));
The remove the line j := Difference(rel[i], Union(rel{rel[i]}));
.
gap/congruences/conglatt.gi
Outdated
@@ -510,14 +500,15 @@ InstallMethod(DotString, | |||
"for a congruence poset and a record", | |||
[IsCongruencePoset, IsRecord], | |||
function(poset, opts) | |||
local congs, S, symbols, i, nr, rel, str, j, k; | |||
local out_nbs, congs, S, symbols, i, nr, rel, str, j, k; | |||
out_nbs := OutNeighbours(poset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dont need this line. Throughout this method, change Length(out_nbs)
and Length(rel)
to Size(poset)
.
Added changes to reverse the ordering. I still need to add loops. |
This is done! |
e9b6777
to
c7aace5
Compare
This probably should've been the case when the object was first created. A poset of semigroup congruences is a digraph with vertices labelled by the congruences themselves. An edge
i -> j
exists if congruencei
j
is a subrelation (a refinement) of congruencej
i
.This is in reference to Issue #369.
EDIT:
i
andj
reversed.