-
Notifications
You must be signed in to change notification settings - Fork 134
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 all_perfect_groups
#3434
Add all_perfect_groups
#3434
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.
Thanks, good new feature.
(Perhaps remove two of the supported functions, see the comments.)
- `intval` selects groups whose order equals `intval`; this is equivalent to `order => intval` | ||
- `intlist` selects groups whose order is in `intlist`; this is equivalent to `order => intlist` | ||
|
||
The following functions are currently supported as values for `func`: |
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 that I do not actually restrict which functions are used in the code; my reading of "supported" here is "we promise these will work, anything else can lead to errors. I could explicitly reject other functions, but there seems no point in this right now.
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.
No problem with the code; if someone asks for supersolvable perfect groups then this is fine.
But it is irritating to read is_supersolvable
here.
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 was exactly what I expected from the wording.
Is there a reason why this list is so much shorter than e.g. for the transitive groups?
Oscar.jl/src/Groups/libraries/transitivegroups.jl
Lines 162 to 179 in 83f0575
The following functions are currently supported as values for `func`: | |
- `degree` | |
- `is_abelian` | |
- `is_almost_simple` | |
- `is_cyclic` | |
- `is_nilpotent` | |
- `is_perfect` | |
- `is_primitive` | |
- `is_quasisimple` | |
- `is_simple` | |
- `is_sporadic_simple` | |
- `is_solvable` | |
- `is_supersolvable` | |
- `is_transitive` | |
- `number_of_conjugacy_classes` | |
- `number_of_moved_points` | |
- `order` | |
- `transitivity` |
I suppose this is too computationally expensive?
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.
No, just most of these filters make no sense: the only perfect group which is abelian/cyclic/nilpotent/solvable/super-solvable is the trivial group.
And primitive/transitive/degree etc. don't really make sense here -- while we return permutation groups, (a) in a future version we may give the user a choice to also get fp groups, (b) which permutation group representations we use is arbitrary. I.e.: the "degree" of a transitive group is well-defined; the "degree" of a perfect group is not.
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.
Ah okay, thanks for clarifying this.
64d6c81
to
14c876a
Compare
14c876a
to
52b1d36
Compare
@ThomasBreuer @lgoettgens thank you both for your great feedback. I hope I've addressed it all for now, please let me know if there is anything else. |
No more objections from me (assuming CI succeeds) |
- Add QQBar docs to the manual #3423 - do not show the OscarInterface banner #3422 - fix bugs in all_OD_infos #3419 - Ep/ Rename Spec to AffineScheme #3345 #3425 - Remove two mentions of Arb_jll #3431 - Tweak epimorphism_from_free_group #3430 - CI: re-enable nightly #3435 - support gen(G::GAPGroup, 0) #3332 - Align all_*_groups methods some more #3433 - Add all_perfect_groups #3434 - Add all_primitive_groups and all_transitive_groups variants taking a single int or int range #3404 - fix a docstring #3436 - Fixes multivariate division #3396 - Docu invariants tori #3428 - Improve docstrings for is_conjugate/is_conjugate_with_data. #3384 - Fix ambient_module(M::SubquoModule) #3448 - Bugfix for printing of affine schemes #3437 - Bugfix for bugfix for printing of affine schemes #3445 - Update OSCAR banner #3410 - Docu invariants lin. red. groups (Lakshmi Ramesh and Wolfram Decker) #3443 - add od_from_atlas_group, od_from_p_subgroup, and helpers #3444 - Unexport normalise #3453 - support group properties for character tables #3449 - add docstrings for acting_group and action_function #3432 (exports are used in new groups code for the book) - Adjust to renaming of rank(A::FinGenAbGroup) to torsion_free_rank(A::FinGenAbGroup) #3457 - Ensure fp_group(G) transfers group attributes #3464 - Added comment on convention #3467 - Export weierstrass_chart_on_minimal_model and patch transform_to_weierstrass #3458 - Fix a doc signature #3466 - Grading + caching for affine algebra of torus invariants #3469
Resolves #3376