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

Add is_symmetric for permutation groups #17677

Merged
merged 12 commits into from
Oct 4, 2019

Conversation

sylee957
Copy link
Member

@sylee957 sylee957 commented Oct 1, 2019

References to other Issues or PRs

Brief description of what is fixed or changed

I think that is_symmetric predicate is missing even if _is_sym cache is used in permutation groups.
I've used a naive test that the symmetric group is the largest permutation group that can be created from a base set of cardinality n, and have an order of n!.

Other comments

I think that permutation group is using custom cache for assumptions. But if there is a way to directly manipulate the global LRU cache or assumptions0 cache if some predicates are revealed during the computation, it may have to use those.

Release Notes

  • combinatorics
    • Added a new predicate is_symmetric to test out if a permutation group is a symmetric group.
    • Added a new predicate is_alternating to test out if a permutation group is a alternating group.

@sympy-bot
Copy link

sympy-bot commented Oct 1, 2019

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
    • Added a new predicate is_symmetric to test out if a permutation group is a symmetric group. (#17677 by @sylee957)

    • Added a new predicate is_alternating to test out if a permutation group is a alternating group. (#17677 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

I think that `is_symmetric` predicate is missing even if `_is_sym` cache is used in permutation groups.
I've used a naive test that the symmetric group is the largest permutation group that can be created from a base set of cardinality `n`, and have an order of `n!`.

#### Other comments

I think that permutation group is using custom cache for assumptions. But if there is a way to directly manipulate the global LRU cache or assumptions0 cache if some predicates are revealed during the computation, it may have to use those.

#### 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 a new predicate `is_symmetric` to test out if a permutation group is a symmetric group.
  - Added a new predicate `is_alternating` to test out if a permutation group is a alternating group.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@jksuom
Copy link
Member

jksuom commented Oct 1, 2019

Permutation.is_alt_sym is related. It is intended for large groups whose order is not easily found.

@sylee957
Copy link
Member Author

sylee957 commented Oct 1, 2019

One problem of using is_alt_sym for quicker test for True is that it treats symmetric group and alternating group as same.
I'm not sure how the algorithm behind _check_cycles_alt_sym can be improved to target only one predicate specifically. But I'll take a look.

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #17677 into master will increase coverage by 0.003%.
The diff coverage is 61.176%.

@@             Coverage Diff              @@
##           master    #17677       +/-   ##
============================================
+ Coverage   74.68%   74.684%   +0.003%     
============================================
  Files         634       634               
  Lines      165598    165715      +117     
  Branches    38961     38977       +16     
============================================
+ Hits       123670    123764       +94     
- Misses      36442     36464       +22     
- Partials     5486      5487        +1

@sylee957
Copy link
Member Author

sylee957 commented Oct 3, 2019

Unfortunately, I don’t think that the jordan’s theorem can discriminate between an alternating group or a symmetric group, so there would not be able to modify the algorithm.

is_alternating may also need to be added for the reason.

@jksuom
Copy link
Member

jksuom commented Oct 3, 2019

It is not hard to distinguish between Alt and Sym: If all generators are even, the group is Alt, otherwise it is Sym.

@sylee957
Copy link
Member Author

sylee957 commented Oct 3, 2019

I have some questions that if G.is_transistive() == False can also be a sufficient condition for is_alt == False and is_sym == False, and the cache can be updated.
I believe it is true from http://mathworld.wolfram.com/TransitiveGroup.html

I also think that the monte-carlo test can use None instead of False some uncertain false.
I would just want the direction to be more clear that

  1. Add is_alternating and is_symmetric helpers.
  2. Modify is_alt_sym to give None instead of some uncertain False.

@jksuom
Copy link
Member

jksuom commented Oct 4, 2019

Looks good.

@jksuom jksuom merged commit abfeccb into sympy:master Oct 4, 2019
@sylee957 sylee957 deleted the add_group_symmetry_test branch October 28, 2019 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants