-
Notifications
You must be signed in to change notification settings - Fork 160
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
Collection of miscellaneous small changes ("cleanup") #4341
Conversation
7618312
to
85d3237
Compare
# | ||
gap> SetUserPreference("PartialPermDisplayLimit", 1); | ||
gap> PartialPerm([1], [2]); | ||
<partial perm on 1 pts with degree 1, codegree 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.
Note: if (when) this PR is merged, #4050 will need to be rebased and updated to change this line to
<partial perm on 1 pts with degree 1, codegree 2> | |
<partial perm on 1 pt with degree 1, codegree 2> |
fi; | ||
end ); | ||
G -> IsTrivial(G) or | ||
ForAll(GeneratorsOfGroup(G),i->SignPerm(i)=1) and PermgpContainsAn(G)=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.
Perhaps also like this?
ForAll(GeneratorsOfGroup(G),i->SignPerm(i)=1) and PermgpContainsAn(G)=true); | |
ForAll(GeneratorsOfGroup(G),i->SignPerm(i)=1) and PermgpContainsAn(G)); |
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.
PermgpContainsAn
can return either true
or false
or the return value of PermNatAnTestDetect
which can be a set of points, so I can't make that change (as much as I'd like to).
lib/semitran.gd
Outdated
@@ -17,8 +17,6 @@ DeclareSynonym("IsTransformationMonoid", IsMonoid and | |||
IsTransformationCollection); | |||
|
|||
DeclareProperty("IsFullTransformationSemigroup", IsSemigroup); | |||
InstallTrueMethod(IsSemigroup, IsFullTransformationSemigroup); |
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.
Why is this unnecessary?
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.
Perhaps I misunderstand how things work.
Since IsFullTransformationSemigroup
is only declared for IsSemigroup
, that means anything that could know it satisfies IsFullTransformationSemigroup
already knows that it is a semigroup.
Is perhaps the idea that: this InstallTrueMethod
is useful because perhaps someone in a package could (for example) do DeclareProperty("IsFullTransformationSemigroup", IsMagma);
and then install methods for IsMagma
(which would have to somehow test the magma for associativity, but perhaps not directly, and this could therefore mean that IsFullTransformationSemigroup
gets set but not IsSemigroup
)?
But actually it seems that any magma of transformations already knows that it is associative, and therefore a semigroup:
gap> S := Magma(GeneratorsOfMagma(FullTransformationSemigroup(3)));;
gap> HasIsSemigroup(S);
true
I can't think of any use for this true method, but perhaps I am not imaginative enough 🙂
I notice that there are similar true methods for natural symmetric and alternating groups (e.g. in lib/gpprmsya.gd
):
InstallTrueMethod( IsPermGroup, IsNaturalSymmetricGroup );
But things seem to work differently for groups:
gap> G := Semigroup(GeneratorsOfMagma(SymmetricGroup(4)));;
gap> IsGroup(G);
false
gap> SetIsNaturalSymmetricGroup(G, true);
gap> IsGroup(G);
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.
There is no automatic implication and there cannot be, as indeed GAP cannot predict the future and so can't know whether that property might in the future also be applicable to, say, algebras or strings or graphs or... Sure, as a human you know this is unlikely because of the name, but GAP does not known this (and anyway.
Somewhat related: issue #2336
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.
(and anyway.
Were you going to add something?
Anyway, I'll be happy to remove I have happily removed this commit. I'll read #2336 too.
Although the new text is longer, I think it's more understandable in English, in general, than the more German notation "i.".
This is a collection of several 'random' cleanup-style commits that I've made over the last year, but that have never made their way into another PR. I decided to just collect them into one PR here.
This is split into 7 commits with descriptive commit messages to help with reviewing.