Added function to check if a group is Cyclic or not #16522

divyanshu132 · 2019-03-31T17:55:05Z

Brief description of what is fixed or changed

Added method is_cyclic for both PermutationGroup and FpGroup. Also, added a method to compute the exponent of a PermutationGroup.

Other comments

Release Notes

combinatorics
- added functions is_cyclic and exponent

sympy-bot · 2019-03-31T17:55:06Z

✅

Hi, I am the SymPy bot (v145). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

combinatorics
- added functions is_cyclic and exponent (#16522 by @divyanshu132)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.5.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed

Added method `is_cyclic` for both `PermutationGroup` and FpGroup. Also, added a method to compute the exponent of a PermutationGroup.
#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
*  combinatorics
   *  added functions is_cyclic and exponent
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

divyanshu132 · 2019-03-31T17:57:35Z

@jksuom please review!

codecov · 2019-03-31T20:28:38Z

Codecov Report

Merging #16522 into master will increase coverage by 0.079%.
The diff coverage is 65.714%.

@@              Coverage Diff              @@
##            master    #16522       +/-   ##
=============================================
+ Coverage   73.677%   73.757%   +0.079%     
=============================================
  Files          618       619        +1     
  Lines       158567    158656       +89     
  Branches     37224     37185       -39     
=============================================
+ Hits        116828    117020      +192     
+ Misses       36322     36217      -105     
- Partials      5417      5419        +2

jksuom · 2019-04-01T09:54:10Z

sympy/combinatorics/fp_groups.py

+        False
+
+        """
+        if len(self.generators) == 1:


How about the trivial case (no generators)?

I'll include that.

It seems like that will also have length of 1 for permutation group but I have added for Fp group.

>>> g=PermutationGroup() >>> g.generators [Permutation()] >>> len(g.generators) 1 >>> g=PermutationGroup([Permutation(1,2,3)]) >>> len(g.generators) 1

jksuom · 2019-04-01T09:58:50Z

sympy/combinatorics/fp_groups.py

+            return True
+        if not self.is_abelian:
+            return False
+        for p in primefactors(self.order()):


The order should probably be finite.

Yes, for the current implementation order is finite, For infinite order we probably have to implement Abelian Invariants which I have included in my GSoC proposal!

jksuom · 2019-04-01T10:17:00Z

Would it be possible to implement is_cyclic first for permutation groups and then extend that to finitely presented groups using the methods of #13119? That way a single copy of the algorithm would suffice.

divyanshu132 · 2019-04-01T11:16:10Z

@jksuom I'll look into it.

divyanshu132 · 2019-04-01T12:06:59Z

sympy/combinatorics/fp_groups.py

+            pgens = []
+            for g in self.generators:
+                pgens.append(g**p)
+            K, T = self.subgroup(pgens, homomorphism=True)


Actually both the is_cyclic methods are not completely same there is a slight difference so, I doubt it if a single method would be suffice or not.

Do you mean that is_cyclic has different meanings for permutation groups and finitely presented groups?

Actually we are using the method .subgroup which has been defined separately for both the permutation groups and fp groups also we are keeping homomorphism=True in the subgroup method so that we can have a list of generators which can be passed to the index method below. Otherwise the generators will simply produce the free variables and we will not be able to compute the index below.

But this is not the case in permutation group where the subgroup method directly returns a group via which index can be calculated easily.

I guess is_cyclic is same for both but it actually depends on the implementation of subgroup method which works differently.

Would it be possible to apply PermutationGroup.is_cyclic also for an instance of FpGroup by transforming it to a permutation group using _to_perm_group? One could first check the 0-1 generator cases.

Yes, I think we can do this.

>>> F, x, y = free_group("x, y") >>> f = FpGroup(F, [x*y*x**-1*y**-1, y**5, x**4]) >>> g, t = f._to_perm_group() >>> g.is_cyclic() True >>> g PermutationGroup([ (0 1 5 2)(3 6 12 8)(4 7 13 9)(10 14 18 16)(11 15 19 17), (0 3 10 11 4)(1 6 14 15 7)(2 8 16 17 9)(5 12 18 19 13)])

divyanshu132 · 2019-04-02T15:52:01Z

@jksuom can this be merged.

jksuom · 2019-04-02T16:28:18Z

sympy/combinatorics/fp_groups.py

+        False
+
+        """
+        if self.order() == S.Infinity:


An infinite group could be cyclic if there is only one generator.

jksuom · 2019-04-02T16:33:32Z

A finite group can handled in two ways. Which method is more efficient, FpGroup.is_cyclic or PermutationGroup.is_cyclic.

divyanshu132 · 2019-04-02T16:48:27Z

PermutationGroup.is_cyclic is far more efficient than FpGroup.is_cyclic.

divyanshu132 · 2019-04-02T16:50:08Z

>>> F, x, y = free_group("x, y")
>>> f = FpGroup(F, [x*y*x**-1*y**-1, y**5, x**4])
>>> f.is_cyclic()
elapsed_time =  0.23461808000000017
True

>>> g, t = f._to_perm_group()
>>> g.is_cyclic()
elapsed_time =  0.0021614229999999957
True

jksuom · 2019-04-02T17:15:41Z

PermutationGroup.is_cyclic is far more efficient than FpGroup.is_cyclic.

That is what I expected. Therefore I suggested that the FpGroup method would apply _to_perm_group after it has been checked that the group is finite.

divyanshu132 · 2019-04-04T03:48:32Z

@jksuom thanks, for pointing it out, you were right we can easily use _to_perm_group to reduce the execution time of is_cyclic for Fp groups. I've updated the PR please have a look.

Should I convert is_cyclic to property?

jksuom · 2019-04-04T07:02:26Z

It seems that is_cyclic could be a property like the other is_... methods.

jksuom · 2019-04-04T07:03:11Z

sympy/combinatorics/fp_groups.py

+        Return ``True`` if group is Cyclic.
+
+        """
+        if len(self.generators) == 0 or len(self.generators) == 1:


This could be len(self.generators) <= 1

jksuom · 2019-04-04T07:11:56Z

sympy/combinatorics/fp_groups.py

+        """
+        if len(self.generators) == 0 or len(self.generators) == 1:
+            return True
+        P, T = self._to_perm_group()


_to_perm_group will raise NotImplementedError if the group is infinite. That can be raised also here but the error message should probably be changed. Maybe something like this:

try: P, _ = self._to_perm_group() except NotImplementedError: raise NotImplementedError("suitable message")

divyanshu132 · 2019-04-04T13:13:15Z

@jksuom I've done the above changes please have a look.

jksuom · 2019-04-04T15:17:04Z

The purpose of self._is_cyclic is to act as a cache. Initially, it is None but it should be set when the question is decided for the first time. Next time it would suffice to check if it has been changed.

divyanshu132 · 2019-04-05T09:11:19Z

@jksuom can this be merged now!

jksuom · 2019-04-05T09:39:48Z

It looks good otherwise but I am not sure about exponent if the generators do not commute. Maybe that should be left to another PR.

divyanshu132 · 2019-04-05T09:43:45Z

Okay I'll remove it for this PR.

jksuom · 2019-04-05T11:46:01Z

sympy/combinatorics/fp_groups.py

@@ -1,6 +1,8 @@
 """Finitely Presented Groups and its algorithms. """

 from __future__ import print_function, division
+from sympy.ntheory import primefactors


This is probably not needed.

jksuom · 2019-04-05T11:46:40Z

sympy/combinatorics/perm_groups.py

@@ -2,6 +2,8 @@

 from random import randrange, choice
 from math import log
+from sympy.ntheory import primefactors
+from sympy.polys import lcm


Not needed.

oh I forget to remove it!

divyanshu132 · 2019-04-06T04:31:30Z

@jksuom I think now this can be merged.

jksuom · 2019-04-06T06:09:32Z

Thanks.

add-iscyclic-exponent

ddb3d97

oscargus added the combinatorics label Mar 31, 2019

jksuom reviewed Apr 1, 2019

View reviewed changes

divyanshu132 commented Apr 1, 2019

View reviewed changes

Divyanshu added 2 commits April 2, 2019 00:11

infinite-order-fpgroup

f8294a7

update

c7152a9

jksuom reviewed Apr 2, 2019

View reviewed changes

shift

f990dec

fp to perm

aac0a69

jksuom reviewed Apr 4, 2019

View reviewed changes

update

b65d987

cache

5359f19

remove-exponent

b003dcb

jksuom reviewed Apr 5, 2019

View reviewed changes

Divyanshu added 2 commits April 5, 2019 18:21

remove-lcm-import

7783cf9

update

46960c4

jksuom merged commit fa19fc7 into sympy:master Apr 6, 2019

divyanshu132 deleted the is-cyclic branch April 6, 2019 08:16

Added function to check if a group is Cyclic or not #16522

Added function to check if a group is Cyclic or not #16522

Conversation

divyanshu132 commented Mar 31, 2019

Brief description of what is fixed or changed

Other comments

Release Notes

sympy-bot commented Mar 31, 2019 • edited Loading

divyanshu132 commented Mar 31, 2019

codecov bot commented Mar 31, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshu132 Apr 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jksuom commented Apr 1, 2019

divyanshu132 commented Apr 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshu132 Apr 1, 2019 • edited Loading

Choose a reason for hiding this comment

divyanshu132 Apr 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshu132 commented Apr 2, 2019

Choose a reason for hiding this comment

jksuom commented Apr 2, 2019

divyanshu132 commented Apr 2, 2019

divyanshu132 commented Apr 2, 2019

jksuom commented Apr 2, 2019

divyanshu132 commented Apr 4, 2019

jksuom commented Apr 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshu132 commented Apr 4, 2019

jksuom commented Apr 4, 2019

divyanshu132 commented Apr 5, 2019

jksuom commented Apr 5, 2019

divyanshu132 commented Apr 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshu132 commented Apr 6, 2019

jksuom commented Apr 6, 2019

sympy-bot commented Mar 31, 2019 •

edited

Loading

codecov bot commented Mar 31, 2019 •

edited

Loading

divyanshu132 Apr 1, 2019 •

edited

Loading

divyanshu132 Apr 1, 2019 •

edited

Loading

divyanshu132 Apr 1, 2019 •

edited

Loading