-
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
Add ListWreathProductElement
and WreathProductElementList
#4410
Add ListWreathProductElement
and WreathProductElementList
#4410
Conversation
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.
Do you really want to have the element test x in G
in ListWreathProductElement
? This requires dividing off a stabilizer chain each time. I'd expect that the test would actually be cheaper to do once one obtained the decomposition as tuple.
@hulpke You are right. So probably I will redo this safety check in
I still think that it might be useful if the method |
OK. Then you might want to rename the |
The idea sounds good! I haven't looked at your code though.
I've had a look now. It looks like you don't need 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.
I've got a few minor remarks. I've marked a few lines where it's not clear to me what they do. Could you add explanations there?
So, in my opinion the product action is constructed unintuitively. It seems to me, that the action of the top elements is renamed in reverse order via the isomorphism
I implemented a new version for computing the 'Projection' in this case. We can discuss, if the top action should be changed to something more intuitive. |
55a552a
to
184f0c6
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!
@fingolfin, I have added some new tests and applied all of your suggestions. I left the comments open that may require further discussion. Do you know what to do about the failed tests in the environments, where |
@FriedrichRober sorry, didn't see your question before... Do the tests pass locally for you? Do they still pass after rebasing your PR on the latest master? Do they pass if GAP is loaded without packages ( |
2b6a2fa
to
2c57a77
Compare
@fingolfin, the tests do not run without packages ( |
bde3d1f
to
73f723c
Compare
@fingolfin I think the pull request is now ready. I changed the option I would squash the commits into three commits once you are ready with the review: |
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.
My comments were adressed. I haven't been able to read the newest version, though.
73f723c
to
19cc940
Compare
This is a hack taken from the recog package.
- Add `Projection` for perm wreath product in product action - Add `Projection` for matrix wreath product
Add implementations of these operations for the following wreath products: - perm wreath product in imprimitive action - perm wreath product in product action - matrix wreath product - generic wreath product Add documentation and tests for these operations.
19cc940
to
8950fb5
Compare
@fingolfin I rebased onto master and squashed the commits. |
ListWreathProductElement
and WreathProductElementList
ListWreathProductElement
and WreathProductElementList
ListWreathProductElement
and WreathProductElementList
Description
Notation
I am working on a package that aims to improve the performance of certain operations in wreath products.$W = K \wr H$ be a wreath product and $H \leq \Sym(m)$ . (I am not really interested in top actions that are not faithful or actions on an infinite set.)$w = (f, h) \in W$ by a tuple $(f_1, ..., f_m; h)$ where $f_i \in K$ is the $i$ -th base component and $h \in H$ is the top component of $w$ .
Let
We identify elements
My Package and Computational Advantages of this Tuple Notation
This generic representation of an element can be exploited to solve the following problems (hopefully efficiently) by using a wreath cycle decomposition and breaking the problems down to the groups$K$ and $H$ :
In my package, I aim to provide implementations to solve these problems and hope that they will perform quite well in comparison with generic methods that do not exploit the wreath product structure.
Package WreathProductElements
The theory behind this will be described in a paper that is work in progress, by Alice C. Niemeyer, Sebastian Krammer, @DominikBernhardt, @FriedrichRober and @wucas.
Interface for Wreath Product Elements in GAP
For my package I need an interface that allows me to work with elements of a wreath product in this tuple notation efficiently.$F = (f, 1)$ and $h$ via the current interface of a wreath product, namely by $f$ .
Note that it is easy to access
h := w ^ Projection(W)
andF := w * (h ^ (-1)) ^ Embedding(W, m + 1)
, but there is no convenient way known to me to access the components of@hulpke suggested that this interface should be implemented on the
GAP
-side, which has the advantage that we could use the (undocumented) internal data stored in wreath products for the implementation. There are several ways how to realise such an interface, I include an implementation of such an interface in this pull request:In analogy to
PermList
andListPerm
, I add two methods that translate between elements of wreath products in various representations to a list/tuple representation:WreathProductElementList
ListWreathProductElement
In a follow-up pull request I suggest to document the record returned by the function
WreathProductInfo
partially and thus to provide some of the stored data that is shared by all wreath product representations to users and package authors.I am very interested in other suggestions how such an interface can be realized and improved.
Further details
I fixed
Projection
for perm wreath products in product action,but there are still other issues with
Projection
, which I have to investigate further.For example
Projection
is not implemented yet for matrix wreath products.I also need to add tests, where the top domain or the base domain gets
compressed/renamed
by callingWreathProduct
, for example I observe the following behavior which is expected to result in issues with my implementation:There are also issues where generators of wreath products are inconvenient, for example we have in the above example:
But it is only necessary to store base generators for each orbit under the top action, so in this case
[ (1,2,3), (1,2), (1,4,7,10,13)(2,5,8,11,14)(3,6,9,12,15), (1,4)(2,5)(3,6) ]
is a more convenient generating set ofG
.I will try to solve these issues (maybe in a separate pull request).
Checklist for pull request reviewers
If your code contains kernel C code, run
clang-format
on it; thesimplest way is to use
git clang-format
, e.g. like this (don'tforget to commit the resulting changes):
usage of relevant labels
release notes: not needed
orrelease notes: to be added
bug
orenhancement
ornew feature
stable-4.X
add thebackport-to-4.X
labelbuild system
,documentation
,kernel
,library
,tests
runnable tests
lines changed in commits are sufficiently covered by the tests
adequate pull request title
well formulated text for release notes
relevant documentation updates
sensible comments in the code