-
Notifications
You must be signed in to change notification settings - Fork 112
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
Improve describe(::GAPGroup) and other group related changes #1137
Conversation
test/Groups/describe.jl
Outdated
|
||
@testset "describe() for some groups in examples" begin | ||
|
||
@test describe(fundamental_group(torus())) == "a finitely presented infinite group" |
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.
To be clear, this is not what @micjoswig requested in this case (he'd like something like Z x Z
).
And I can computed that -- in this case. The problem is to write the code so that it won't get stuck in an infinite loop for other examples. Gotta think a bit about that.
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.
Actually I've now added a cheap check that will detect some "obviously" abelian FPGroups, so this example now is recognized as Z x Z
.
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 looks good.
Perhaps the documentation could mention that the function is useful for infinite abelian groups (this is mentioned in the code but not in the documentation), provided that the group in question stores already that it is abelian; and it could be emphasized that describe
does not try to compute such properties (and thus calling describe
again later in the session may yield a different output).
P = SimplicialComplex([[1,2,4,9],[1,2,4,15],[1,2,6,14],[1,2,6,15],[1,2,9,14],[1,3,4,12],[1,3,4,15],[1,3,7,10],[1,3,7,12],[1,3,10,15],[1,4,9,12],[1,5,6,13],[1,5,6,14],[1,5,8,11],[1,5,8,13],[1,5,11,14],[1,6,13,15],[1,7,8,10],[1,7,8,11],[1,7,11,12],[1,8,10,13],[1,9,11,12],[1,9,11,14],[1,10,13,15],[2,3,5,10],[2,3,5,11],[2,3,7,10],[2,3,7,13],[2,3,11,13],[2,4,9,13],[2,4,11,13],[2,4,11,15],[2,5,8,11],[2,5,8,12],[2,5,10,12],[2,6,10,12],[2,6,10,14],[2,6,12,15],[2,7,9,13],[2,7,9,14],[2,7,10,14],[2,8,11,15],[2,8,12,15],[3,4,5,14],[3,4,5,15],[3,4,12,14],[3,5,10,15],[3,5,11,14],[3,7,12,13],[3,11,13,14],[3,12,13,14],[4,5,6,7],[4,5,6,14],[4,5,7,15],[4,6,7,11],[4,6,10,11],[4,6,10,14],[4,7,11,15],[4,8,9,12],[4,8,9,13],[4,8,10,13],[4,8,10,14],[4,8,12,14],[4,10,11,13],[5,6,7,13],[5,7,9,13],[5,7,9,15],[5,8,9,12],[5,8,9,13],[5,9,10,12],[5,9,10,15],[6,7,11,12],[6,7,12,13],[6,10,11,12],[6,12,13,15],[7,8,10,14],[7,8,11,15],[7,8,14,15],[7,9,14,15],[8,12,14,15],[9,10,11,12],[9,10,11,16],[9,10,15,16],[9,11,14,16],[9,14,15,16],[10,11,13,16],[10,13,15,16],[11,13,14,16],[12,13,14,15],[13,14,15,16]]) | ||
pi_1 = fundamental_group(P) | ||
@test describe(pi_1) == "SL(2,5)" |
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.
Added another example by @micjoswig which initially took 150 seconds on his computer, but now is down to 75 milliseconds on my computer. I am using an isomorphism to a permutation group, as @ThomasBreuer also suggested, but in addition the fp presentation is first simplified using Tietze transformations
88e0006
to
4cdc0f0
Compare
@ThomasBreuer I've updated the docstring, please have another look at it |
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.
Thanks.
Perhaps change the phrase "information ... become known", otherwise now the description of describe
is good.
@test describe(special_linear_group(2,4)) == "A5" # FIXME: correct but not ideal | ||
@test describe(special_linear_group(3,2)) == "PSL(3,2)" # FIXME: correct but not ideal | ||
@test describe(special_linear_group(3,3)) == "PSL(3,3)" # FIXME: correct but not ideal | ||
@test describe(special_linear_group(4,2)) == "A8" # FIXME: correct but not ideal |
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.
What would be ideal?
When describe
finds out that the group is a finite nonabelian simple group then one name for the (isomorphism type of the) group is chosen, independent of the context.
I would find it irritating if describe
would return, depending on the representation, one of "A5", "SL(2,4)", "PGL(2,4)", "PSL(2,4)", "PSL(2,5)", "PSU(2,5)", "PGU(2,4)", "SU(2,4)", "PSU(2,4)", "GO(3,4)", "PGO(3,4)", "SO(3,4)", "PSO(3,4)", "Omega(3,4)", "Omega(3,5)", etc.
The situation would be different for a function that collects data about the given group, until either the user stops this process or GAP runs out of ideas which information might be interesting.
For A5 given as some matrix group, a possible list of pieces could contain the entries "a subgroup of GL(2,4)", "the simple group of order 60", "SL(2,4)", "A5"; this list would look differently if the group is given as a permutation group.
Super! Thanks, everyone. |
simplified_fp_group(G::FPGroup)
hasgens(G::GAPGroup)
isfinitelygenerated(G::GAPGroup)
describe(G::GAPGroup)
Resolves #1133
Resolves #987