-
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
Add Translations #720
Add Translations #720
Conversation
f77aa82
to
2f73bcd
Compare
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.
A few initial comments mostly about the documentation.
doc/translat.xml
Outdated
gap> mat := [[G.1, G.2], [G.1, G.1], [G.2, G.3]];; | ||
gap> S := ReesMatrixSemigroup(G, mat);; | ||
gap> R := Range(RMSNormalization(S));; | ||
gap> G := UnderlyingSemigroup(R);; |
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.
gap> G := UnderlyingSemigroup(R);; |
You can delete this line because normalising a RMS/RZMS doesn't change its underlying (semi)group. So here G
is still defined as it was a few lines above.
tst/extreme/translat.tst
Outdated
# | ||
gap> SEMIGROUPS.StartTest(); | ||
|
||
#T# RZMS Translational Hull |
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.
In the intervening years we've got rid of these #T#
and #E#
tags, so we may as well not reintroduce them now. So please just use #
. 🙂 Same with the standard test file.
doc/z-chap14.xml
Outdated
<Heading> | ||
Translations and translational hulls | ||
</Heading> | ||
In this section we describe the functionality in Semigroups for working with |
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.
In this section we describe the functionality in Semigroups for working with | |
In this section we describe the functionality in &Semigroups; for working with |
doc/translat.xml
Outdated
<Func Name="RightTranslations" Arg="S"/> | ||
<Func Name="LeftTranslationsSemigroup" Arg="S"/> | ||
<Func Name="RightTranslationsSemigroup" Arg="S"/> | ||
<Returns>The semigroup of left or right translations of the given semigroup</Returns> |
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.
<Returns>The semigroup of left or right translations of the given semigroup</Returns> | |
<Returns>A semigroup of left or right translations.</Returns> |
doc/translat.xml
Outdated
<Func Name="RightTranslation" Arg="R, map"/> | ||
<Returns>A left or right translation.</Returns> | ||
<Description> | ||
For the semigroup of left of right translations and a Mapping on the |
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.
For the semigroup of left of right translations and a Mapping on the | |
For the semigroup of left of right translations and a mapping on the |
? Or do you deliberately want the capital letter for some reason?
doc/translat.xml
Outdated
For the semigroup of left of right translations and a Mapping on the | ||
underlying semigroup, or a transformation acting on the indices of the | ||
canonical list of elements of the underlying semigroup, | ||
LeftTranslation and RightTranslation return the corresponding translations. |
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.
LeftTranslation and RightTranslation return the corresponding translations. | |
<C>LeftTranslation</C> and <C>RightTranslation</C> return the corresponding translations. |
doc/translat.xml
Outdated
<Returns>The element of the translational hull corresponding to the given | ||
left and right translations</Returns> |
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.
Just to point out, I could be wrong, but we usually use the <Returns>
tag to say what kind of GAP object is returned, rather than its mathematical meaning. So this might be more in keeping as:
<Returns>The element of the translational hull corresponding to the given | |
left and right translations</Returns> | |
<Returns>A translational hull element.</Returns> |
doc/translat.xml
Outdated
<Filt Name="IsTranslationalHull" Type="synonym"/> | ||
<Description> | ||
<C>IsTranslationalHull</C> is a synonym for <C>IsSemigroup</C> and | ||
<Ref Filt="IsBitranslationCollection"/> |
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.
<Ref Filt="IsBitranslationCollection"/> | |
<Ref Filt="IsBitranslationCollection"/>. |
doc/translat.xml
Outdated
|
||
<#GAPDoc Label="UnderlyingSemigroup"> | ||
<ManSection> | ||
<Attr Name="UnderlyingSemigroup" Arg="S"/> |
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 <Returns>
tag here.
## | ||
# W rms-translat.gd | ||
# Y Copyright (C) 2015-17 Finn Smith | ||
## |
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.
Might want update the years here!
4f78843
to
c961b97
Compare
3b08ac4
to
d06bdcb
Compare
The linting on this PR will fail until james-d-mitchell/gaplint#13 is merged/released (or some other solution). |
@james-d-mitchell I think this is ready to be looked at |
0593c0f
to
7eaf553
Compare
7eaf553
to
3c09408
Compare
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.
Nice one @flsmith! Subject to the comments in this review, I'm happy with this
gap/attributes/rms-translat.gd
Outdated
############################################################################# | ||
## | ||
|
||
# The declarations in this file are purposefully undocumented. |
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.
With what purpose?
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.
They are internal, which I've noted in the file, but should I also preface them with SEMIGROUPS? I was avoiding doing so just because of how annoying that looks when they are used as filters.
This adds functionality for translations; see the description of #373.
Remaining tasks: