Skip to content
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

Powerful p-Groups Feature #894

Merged
merged 4 commits into from
Sep 20, 2016
Merged

Powerful p-Groups Feature #894

merged 4 commits into from
Sep 20, 2016

Conversation

grouptheoryenthusiast
Copy link
Contributor

Please make sure that this pull request:

  • is submitted to the correct branch (the stable branch is only for bugfixes)
  • contains an accurate description of changes for the release notes below
  • provides new tests or relies on existing ones
  • correctly refers to other issues and related pull requests

Description of changes (for the release notes)

This feature adds the propertyIsPowerfulPGroup for groups, and the method IsPowerfulPGroup(G) .
A finite p-group G is said to be a Powerful p-group if the commutator subgroup [G,G] is contained in
G^{p} if the prime p is odd, or if [G,G] is contained in G^{4} if p=2.
In the case a p- group is powerful, then FrattiniSubgroup(G)=G^{p} . Therefore, if GAP knows a group is powerful and has the computed Agemos (which is most likely the case if it has tested if G is powerful), then using this feature it can quickly return the FrattiniSubgroup without doing any extra work.

This is my first time uploading a GAP feature, I hope everything seems okay.

(Note: For more details on Powerful p-Groups, see:
Powerful p-groups. I. Finite groups, Alexander Lubotzky, Avinoam Mann )

…s commit I have included tests, which in particular test the true methods for abelian p-groups, test that powerfulness is inherited to quotient groups, that the function works for 2-groups and odd p groups. Documentation is improved. As computing IsPowerfulPGroup will involve computing the agemos, I have installed a high priority method for finding the frattini subgroup of powerful p-groups, in the case that G is a group which is powerful and with computed agemos, which will almost always be the case if IsPowerfulPGroup has been called, then GAP now just returns Agemo(G,p) instead of computing FrattiniSubgroup(G).
<#Include Label="PrimePGroup">
<#Include Label="PClassPGroup">
<#Include Label="RankPGroup">
<#Include Label="IsPSolvable">
<#Include Label="IsPNilpotent">


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to avoid random whitespace changes (I know it's annoying to do so)

@ChrisJefferson
Copy link
Contributor

I don't really like the ErrorNoReturn in IsPowerfulPGroup. Would it make more sense to just say false (if something isn't a prgroup, it isn't a powerfulpgroup)?

@ChrisJefferson
Copy link
Contributor

Don't worry about the problems github is reporting about 'conflicts'. Basically you, and someone else, added a test to prgroups.tst. The solution is probably to 'rebase' your code, but if you aren't happy doing that (it can be a little tricky if you aren't happy with git), then one of us can do it later.

…t case. Also removed white space at end of groups.xml . This is as per the suggestions.
@grouptheoryenthusiast
Copy link
Contributor Author

Hi @ChrisJefferson ,
Thank you for your suggestions. I have removed the white space and also changed the error to instead return false as you suggested . I have pushed these commits to the branch.
As I am still learning git I think on this occasion I would prefer one of you to 'rebase' the code.
Thank you
James

Copy link
Member

@olexandr-konovalov olexandr-konovalov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready to be merged. Comments are addressed, and the merge conflict is just because more tests were added to the test file changed in the meantime.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.9.0 milestone Sep 19, 2016
@markuspf markuspf merged commit 13f720c into gap-system:master Sep 20, 2016
@olexandr-konovalov olexandr-konovalov added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants