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

Rewrite DerivedSeriesOfGroup to check for abelian derived subgroups #585

Closed

Conversation

hungaborhorvath
Copy link
Contributor

DerivedSeriesOfGroup is rewritten to check for the derived subgroup being
abelian. In some cases this may terminate for fp-groups for which the old
method did not, in the expense of an extra IsAbelian check for each term.
Insted of while do, repeat until is used to decrease code repetition.

Test file is added.

This is supposed to fix #584, where some discussion is already started.

This attribute is one of the most basic in GAP, so please, everybody, (try to) tear it apart as long as it is not perfect.

DerivedSeriesOfGroup is rewritten to check for the derived subgroup being
abelian. In some cases this may terminate for fp-groups for which the old
method did not, in the expense of an extra IsAbelian check for each term.
Insted of while do, repeat until is used to decrease code repetition.

Test file is added.
@hulpke
Copy link
Contributor

hulpke commented Feb 5, 2016

But if the derived series does not descend to the trivial group, this PR will cause a calculation to be potentially not terminating that before was trivial. For example:
G:=F/ParseRelators(F,"a3,b8,c13,(ab)2,(bc)2,(ca)2,(abc)2");

has DerivedSeries terminate in the blink of an eye, but is likely never to finish under this PR.

@hungaborhorvath
Copy link
Contributor Author

@hulpke Thank you for the feedback, you are absolutely right. I see that you already have a working fix that you mention in the thread #584. Probably when you do a pull request about your fix, then this pull request should be closed without merging.

You are of course welcome to use the tst file from this PR if you like.

@hungaborhorvath
Copy link
Contributor Author

I am going to close this PR as #618 takes care of the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IsSolvableGroup cannot even verify solvability for infinite fp-groups?
2 participants