-
Notifications
You must be signed in to change notification settings - Fork 38
Mitsch Order #1024
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
base: main
Are you sure you want to change the base?
Mitsch Order #1024
Conversation
| deg := DegreeOfTransformationSemigroup(S); | ||
| return | ||
| function(x, y) | ||
| # Only returns the correct answer if y is a regular element |
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.
Add check that y is regular.
Also what happens if x or y not in S?
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
Co-authored-by: James Mitchell <james-d-mitchell@users.noreply.github.com>
Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Max Horn <max@quendi.de>
reiniscirpons
left a comment
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.
Overall new functionality looks good, I had some comments on particular implementations and docs. One of the big blockers for merging is the lack of tests, however. None of the Mitsch order functions have tests written for them.
I don't have the time to address these issues currently, but writing them down for the future.
| local E; | ||
| E := Idempotents(S); | ||
| return ForAny(E, e -> e * y = x) | ||
| and ForAny(E, f -> y * f = x); |
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 would like to see some benchmarks for this change. I can believe its faster for certain semigroups, but wouldn't this change make it slower for semigroups that have lots of idempotents but small R-classes, e.g. lattices?
| DeclareOperation("IsRefinementKernelOfTransformation", | ||
| [IsTransformation, IsTransformation, IsInt]); | ||
| DeclareOperation("IsRefinementKernelOfTransformation", | ||
| [IsTransformation, IsTransformation]); |
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 would suggest removing the 3-parameter version of IsRefinementKernelOfTransformation and just leaving the 2-parameter version. This is because GAP by default treats transformations as belonging to the same underlying infinite transformation monoid (see https://docs.gap-system.org/doc/ref/chap53.html). So I think automatically assuming that the degree is the maximum degree of either semigroup is the correct behavior and having the specify the degree manually is not necessary.
| q := []; | ||
| for class in KernelOfTransformation(a, n) do | ||
| m := Minimum(class); | ||
| for i in class do | ||
| q[i] := m; | ||
| od; | ||
| od; | ||
| for class in KernelOfTransformation(b, n) do | ||
| idx := q[class[1]]; | ||
| for i in class do | ||
| if q[i] <> idx then | ||
| return false; | ||
| fi; | ||
| od; | ||
| od; | ||
| return true; |
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.
Some code duplication between the two versions of IsRefinementKernelOfTransformation. I would recommend just calling the 2-parameter version from within the 3-parameter version.
| q := []; | |
| for class in KernelOfTransformation(a, n) do | |
| m := Minimum(class); | |
| for i in class do | |
| q[i] := m; | |
| od; | |
| od; | |
| for class in KernelOfTransformation(b, n) do | |
| idx := q[class[1]]; | |
| for i in class do | |
| if q[i] <> idx then | |
| return false; | |
| fi; | |
| od; | |
| od; | |
| return true; | |
| return IsRefinementKernelOfTransformation(a, b); |
| gap> S := BrauerMonoid(3); | ||
| <regular bipartition *-monoid of degree 3 with 3 generators> | ||
| gap> IsRegularSemigroup(S); | ||
| true | ||
| gap> Size(S); | ||
| 15 | ||
| gap> NambooripadPartialOrder(S); | ||
| [ [ ], [ ], [ ], [ ], [ ], [ ], [ ], [ ], [ ], | ||
| [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ], [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ], | ||
| [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ], [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ], | ||
| [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ], [ 1, 2, 3, 4, 5, 6, 7, 8, 9 ] ] | ||
| gap> NambooripadLeqRegularSemigroup(S)(Elements(S)[3], Elements(S)[9]); | ||
| false | ||
| gap> NambooripadLeqRegularSemigroup(S)(Elements(S)[2], Elements(S)[15]); |
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.
Example seems to be for NambooripadPartialOrder instead of the MitschOrderOfSemigroup.
| entry <C>i</C> in <C>MitschOrderOfSemigroup(<A>S</A>)</C> is the set of | ||
| positions in <C>Elements(<A>S</A>)</C> of elements which are less than | ||
| <C>Elements(<A>S</A>)[i]</C>. See also <Ref | ||
| Func="MitschLeqOfSemigroup"/>. |
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.
Missing documentation for MitschLeqOfSemigroup.
| higher than <C>n</C> and hence cannot belong to a semigroup of degree | ||
| <C>n</C>. |
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.
| higher than <C>n</C> and hence cannot belong to a semigroup of degree | |
| <C>n</C>. | |
| higher than <C>n</C> and hence cannot belong to the full transformation semigroup of degree | |
| <C>n</C>. |
|
|
||
| <#GAPDoc Label="IsRefinementKernelOfTransformation"> | ||
| <ManSection> | ||
| <Func Name = "IsRefinementKernelOfTransformation" Arg = "trans1, trans2, n"/> |
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.
The 2-parameter version of this function should also be documented.
| DeclareOperation("KernelContainment", | ||
| [IsTransformation, IsTransformation, IsPosInt]); |
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.
Operation KernelContainment declared but never defined.
| InstallMethod(DumbMitschOrderOfSemigroup, | ||
| "for a finite semigroup", | ||
| [IsSemigroup and IsFinite], | ||
| function(S) | ||
| local func, out, i, j, elts; | ||
| func := MitschLeqSemigroup(S); | ||
| elts := Elements(S); | ||
| out := List([1 .. Size(S)], x -> []); | ||
| for i in [1 .. Size(S)] do | ||
| for j in [1 .. Size(S)] do | ||
| if i = j then | ||
| continue; | ||
| elif func(elts[i], elts[j]) then | ||
| AddSet(out[j], i); | ||
| fi; | ||
| od; | ||
| od; | ||
| return out; | ||
| end); |
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 should probably not be a separate method. I would suggest adding an optional flag for MitschOrderOfSemigroup which indicates the method should use the "dumb" algorithm. Also, if the dumb algorithm is always slower then this bit of code can be removed. Needs some benchmarks.
Add methods for finding the Mitsch order of a semigroup