-
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
Fix nice monomorphism dispatch for HallSubgroup #559
Fix nice monomorphism dispatch for HallSubgroup #559
Conversation
Could you also add the case when HallSubgroup returns a list of groups? This is the documented behaviour when there is more than one conjugacy class of Hall subgroups. SL(2,11) with π={2,3} is an example where this happens. |
@bh11 I just had to produce an example where |
97b1b77
to
68bf30d
Compare
Voila. Also added test cases. |
fail | ||
gap> G := PSL(4,2);; | ||
gap> HallSubgroup(G, [2,3]); | ||
<permutation group of size 576 with 8 generators> |
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 might be problem running this test with different packages loaded, because the presentation of the answer might change. Maybe ask for an IdGroup
, or something similar?
68bf30d
to
d4e897e
Compare
I did't realise that someone had started adjusting tests for possible isomorphisms. This is certainly not done in many places, and we do know that this is a problem, because sometimes changing algorithms gives different, but isomorphic, results. In any case, I added IdGroup, it even works for the matrix groups, presumably because they are tiny. |
@markuspf Thank you very much for adding this -- when I implemented the Hall subgroup code I only tested matrix groups when using the |
@markuspf As far As I know, two tests are run, one with all packages loaded and one with no packages loaded. Sometimes this can make differences in the output presentation. Probably @alex-konovalov could explain it better. Anyway, thanks for this. |
Fix nice monomorphism dispatch for HallSubgroup
Fixes issue #557
Will this work when
HallSubgroup
returns a list of groups?Also I will add some tests.