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

groups: fix FpSubgroup's "contains" method #13028

Merged
merged 3 commits into from Jul 26, 2017

Conversation

Projects
None yet
2 participants
@valglad
Contributor

valglad commented Jul 22, 2017

This PR extends the __contains__ method of the FpSubgroup class to handle subgroups of free groups generated by a set containing elements that are not cyclically reduced, i.e. of the form r*w*r**-1
for some words r and w. Previously they would lead to an infinite loop as the method would try to add
all words of the form r*w**n*r**-1 to a list. Now, this infinite family is handled by checking for cyclic reduction and appending the tuple (r, w) to the list instead.

Two new methods were added for FreeGroupElements.

  • cyclic_reduction is similar to identity_cyclic_reduction but doesn't cyclically permute the reduced part of the word. For example, (x**-3*y*x**5).identity_cyclic_reduction() == x**2*y while (x**-3*y*x**5).cyclic_reduction() == y*x**2. Also, it has a keyword argument for returning the removed part (in this case x**-3).
  • power_of checks if the word is a power of some other word.
Show outdated Hide outdated sympy/combinatorics/tests/test_fp_groups.py
@@ -158,3 +158,14 @@ def test_order():
F, a, b, c = free_group("a, b, c")
f = FpGroup(F, [a**250, b**2, c*b*c**-1*b, c**4, c**-1*a**-1*c*a, a**-1*b**-1*a*b])
assert f.order() == 2000
def test_fp_subgroup():
from sympy import S

This comment has been minimized.

@jksuom

jksuom Jul 25, 2017

Member

The imported S is not needed.

@jksuom

jksuom Jul 25, 2017

Member

The imported S is not needed.

@jksuom

This comment has been minimized.

Show comment
Hide comment
@jksuom

jksuom Jul 26, 2017

Member

There could be more tests, for the construction of kernels, in particular. But they can be added later. I'll merge this now to see if the random timeouts will continue.

Member

jksuom commented Jul 26, 2017

There could be more tests, for the construction of kernels, in particular. But they can be added later. I'll merge this now to see if the random timeouts will continue.

@jksuom jksuom merged commit 11acd9b into sympy:master Jul 26, 2017

1 check passed

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

@valglad valglad deleted the valglad:kernel_test branch Aug 23, 2017

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