Improve is_cyclic

[sylee957]

References to other Issues or PRs

Brief description of what is fixed or changed

1. There were some wrong results for is_cyclic

>>> PermutationGroup(
...     Permutation(0, 1, 2, 3),
...     Permutation(0, 2)(1, 3)
... ).is_cyclic
False

because of if not self._is_abelian: which gives a false positive for None

2. I've made is_abelian cache to update whenever is_cyclic holds True.

3. I've also added an additional lemma when the order is square-free.

Other comments

I've also updated the documentation

Release Notes

• combinatorics
  • Fixed PermutationGroup.is_cyclic giving wrong result for the cyclic groups created with explicitly specified generators.
  • PermutationGroup.is_cyclic automatically updates the cache for is_abelian.
  • Improved the algorithm of PermutationGroup.is_cyclic to detect some trivial cases like the group order of 15, 35, etc.

[sympy-bot]

✅

Hi, I am the SymPy bot (v149). 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

  • Fixed PermutationGroup.is_cyclic giving wrong result for the cyclic groups created with explicitly specified generators. (#17781 by @sylee957)

  • PermutationGroup.is_cyclic automatically updates the cache for is_abelian. (#17781 by @sylee957)

  • Improved the algorithm of PermutationGroup.is_cyclic to detect some trivial cases like the group order of 15, 35, etc. (#17781 by @sylee957)

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. -->

#### References to other Issues or PRs
<!-- 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

1. There were some wrong results for `is_cyclic`
```python3
>>> PermutationGroup(
...     Permutation(0, 1, 2, 3),
...     Permutation(0, 2)(1, 3)
... ).is_cyclic
False
```
because of `if not self._is_abelian:` which gives a false positive for `None`

2. I've made `is_abelian` cache to update whenever `is_cyclic` holds ``True``. 

3. I've also added an additional lemma when the order is square-free.

#### Other comments

I've also updated the documentation
![image](https://user-images.githubusercontent.com/34944973/67360735-9fbcc880-f5a1-11e9-9941-9c47d2ae8468.png)

#### 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
  - Fixed `PermutationGroup.is_cyclic` giving wrong result for the cyclic groups created with explicitly specified generators.
  - `PermutationGroup.is_cyclic` automatically updates the cache for `is_abelian`.
  - Improved the algorithm of `PermutationGroup.is_cyclic` to detect some trivial cases like the group order of `15`, `35`, etc.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

[jksuom]

Would it be expensive to check first that the group is abelian? Then, I think, _distinct_primes_lemma would not be needed. It would suffice that the exponents are 1.

[sylee957]

is_abelian can take a lot of time if there are many generators.
I think that it even takes more time to test is_abelian first than using the default check when _distinct_primes_lemma fails, like

G = AbelianGroup(7, 29)
G = PermutationGroup(*G.generate())
G._is_abelian = None
G._is_cyclic = None

This case takes 236888 function calls in 0.100 seconds for is_cyclic, but takes 791745 function calls in 0.211 seconds for is_abelian.

I just added the suggestion as a trivial case when the cache is warm.

[jksuom]

Then this looks like a good method. Still, I wonder why checking commutativity is so expensive. Maybe that could be improved.