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

FpGroup: a bug in an elimination technique #12973

Merged
merged 1 commit into from Jul 15, 2017

Conversation

Projects
None yet
2 participants
@valglad
Contributor

valglad commented Jul 14, 2017

This PR fixes a bug in the code of one of the elimination techniques: when a relator is used to eliminate a generator by replacing it with an equivalent expression (e.g. with generators a, b, c, if c*a**2*b is a relator, c is a redundant generator as all its occurrences can be replaced with b**-1**a**-2), the latter shouldn't be cyclically reduced.

For example, consider a group on the generators a, b, c and use the relator c*a*b**-1*a**-1 to eliminate c. Then c should end up being equivalent to a**-1*b*a, however, at the moment the code reduces it to b which is wrong in general.

Additionally, the _finite_index_subgroup() method is modified to not adjoin a random element to the list of generators when the number of generators is 2. This is to prevent the returned subgroup from being equal to the whole group which is likely to happen in the 2 generator case and result in an infinite loop.

i += 1
s = [gen, rand] + [g for g in self.generators if g != gen]
if len(self.generators) == 2:
s = [gen] + [g for g in self.generators if g != gen]

This comment has been minimized.

@jksuom

jksuom Jul 15, 2017

Member

Will s be the same as self.generators with gen moved to the first place?

@jksuom

jksuom Jul 15, 2017

Member

Will s be the same as self.generators with gen moved to the first place?

@jksuom jksuom merged commit 751bfce into sympy:master Jul 15, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@valglad valglad deleted the valglad:order_bug branch Jul 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment