Skip to content
This repository

Permutation groups: Random Schreier-Sims, fundamental tools for backtracking, minor additions #1406

Merged
merged 9 commits into from over 1 year ago
 +2,338 1,232

4 participants

Aleksandar Makelov Stefan Krastanov Don't Add Me To Your Organization a.k.a The Travis Bot david joyner
Aleksandar Makelov

Implemented the randomized version of the Schreier-Sims algorithm. Implemented a depth-first search visiting all group elements (to be used in backtracking algorithms) and a function for swapping two consecutive points in the base of a permutation group.
Added some more attributes to the PermutationGroup class so that the output of the deterministic version of Schreier-Sims is more easily usable.
Implemented the direct product of several groups at once (faster than applying * repeatedly), and from that a function constructing an arbitrary abelian group by a cyclic decomposition.
Added a function that finds the degree of transitivity of a permutation group.

Aleksandar Makelov

As it turned out, the number of LoC is sort of unreasonably high. If this makes review unnecessarily hard, I can remove some of the stuff and save them for the next pull request.

Stefan Krastanov
Collaborator

SymPy Bot Summary: :eight_spoked_asterisk: All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY5YMeDA

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 48f42f8
branch hash: 78c1e3e

Automatic review by SymPy Bot.

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYjs0dDA

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 48f42f8
branch hash: 78c1e3e

Automatic review by SymPy Bot.

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYoZseDA

Interpreter: /usr/bin/python3 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 48f42f8
branch hash: 78c1e3e

Automatic review by SymPy Bot.

Aleksandar Makelov

@testfailures: it wasn't the combinatorics module

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged 9200741f into fb35324).

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test command: setup.py test
master hash: fb35324
branch hash: 9200741

Interpreter 1: :eight_spoked_asterisk: All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY_sYfDA

Interpreter 2: :red_circle: There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY26QgDA

Interpreter 3: :red_circle: There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY2f0fDA

Build HTML Docs: :red_circle: There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYlZUgDA

Automatic review by SymPy Bot.

Aleksandar Makelov

OK, the errors above are not related to the combinatorics module

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 7b5d062a into b33c1ed).

Aleksandar Makelov

The failure the travisbot detected comes from the quantum physics module and is probably related to the recently merged #1396

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test command: setup.py test
master hash: b33c1ed
branch hash: 7b5d062

Interpreter 1: :eight_spoked_asterisk: All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYr40gDA

Interpreter 2: :red_circle: There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY38sgDA

Interpreter 3: :red_circle: There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY3KQgDA

Build HTML Docs: :red_circle: There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYxogfDA

Automatic review by SymPy Bot.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 700101ac into b33c1ed).

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 526a7827 into b33c1ed).

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test command: setup.py test
master hash: b33c1ed
branch hash: 526a782

Interpreter 1: :eight_spoked_asterisk: All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYl5UgDA

Interpreter 2: :red_circle: There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYlpUgDA

Interpreter 3: :red_circle: There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1K8fDA

Build HTML Docs: :red_circle: There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYsY0gDA

Automatic review by SymPy Bot.

Aleksandar Makelov

Again, it wasn't the combinatorics module.

Aleksandar Makelov

Is anyone reviewing this, and if yes, could you provide some feedback :)

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged 5166caaf into c84e5df).

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test command: setup.py test
master hash: c84e5df
branch hash: 5166caa

Interpreter 1: :eight_spoked_asterisk: All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY9ZkhDA

Interpreter 2: :red_circle: There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYq7EhDA

Interpreter 3: :red_circle: There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYstghDA

Build HTML Docs: :red_circle: There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY3aEhDA

Automatic review by SymPy Bot.

david joyner

Generaly, the code looks good but docstrings must be added. I understand some methods are random. In that case, if it is easier to write docstrings which are not tested, that is fine. There should be docstrings which describe the input and output, as well as an example or two (even if the example is written in a way to escape testing).
Can this be done while fixing the doc build errors methodes above?

Stefan Krastanov
Collaborator
added some commits June 22, 2012
Aleksandar Makelov Randomized Schreier Sims, abelian groups cf890b5
Aleksandar Makelov Backtrack search printing all group elements
	Implemented a procedure that prints all group elements
	in the lexicographical ordering induced by a base (i.e.,
	according to the images of the base under the action
	of the group elements).

	This basic procedure that visits all elements will be
	used with some modifications for other backtracking
	algorithms for permutation groups.
06a0494
Aleksandar Makelov Implemented baseswap (needs debugging)
	The algorithm baseswap is used to swap two consecutive
	elements in the base of a permutatation group. The
	current version breaks somewhere and needs some debugging.
b670ff0
Aleksandar Makelov Fixed the bug in BASESWAP, refactored the code
The reason the implementation of BASESWAP suggested
in the "Handbook..." was failing is that (I believe)
there is a mistake in the book. Now that I noticed
it, it seems to work fine. This will be further
discussed in my blog.

Also, implemented a randomized version of BASESWAP
that is supposed to run faster than the deterministic
one.

Introduced some new utility functions to help
handling computations using a BSGS, and refactored
PRINTELEMENTS and BASESWAP accordingly.

Moved the current utility functions to a new file,
util.py, and moved the constructor for an abelian
group to the named_groups.py file.
00a819d
Aleksandar Makelov Docstrings, tests, refactoring
Wrote docstrings and tests for the new functionality,
and did some refactoring in the new functions.

Also, made possible the interaction with the
deterministic version of the Schreier-Sims algorithm
by introducing some new attributes.

Reorganized the util.py file containing helper functions for
the combinatorics module.

Altered _strip in sympy/combinatorics/util.py
so that it accepts basic transversals as an argument, instead of
basic stabilizers, so that finding transversal elements is done
in constant time. Altered _distribute_gens_by_base in the same
file slightly.
c9f9808
Aleksandar Makelov

By the way: the function list_lex_by_base is not very useful - there are other functions available to list all group elements (and that are faster than list_lex_by_base). It was like a stepping stone for me in the implementation of more sophisticated backtracking algorithms like subgroup searching (which will appear in my next PR). I'm just going to remove it from this PR and keep the code somewhere just in case.

Aleksandar Makelov Removed list_lex_by_base and related tests/references.
The function list_lex_by_base served as a stepping stone in
my understanding of more complex backtracking routines, so
I'm removing it now. There are other (and faster) functions
that list all group elements.
547b393
Aleksandar Makelov

@Krastanov : Thanks, this is really useful!

Aleksandar Makelov

Hi again,
I improved several docstrings with paremeters/returns/examples stuff. If you're happy with them, I can then reorder the methods in the files in alphabetical order (after that happens, the diff is going to be terrible, so it's best to do it just before merging).

david joyner

Thanks, the docstrings look much better. Please do reorder the methods aphabetically. It makes it easier to read your code, as the module is getting quite big. Hopefuly it can be merged soon afterwards.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged 3ad1c91 into 861a066).

Aleksandar Makelov Ordered the methods in several files alphabetically.
These were: the standalone methods in named_groups.py and
util.py, and the class methods for the classes _JGraph
and PermutationGroup in perm_groups.py.
0fbc00d
Aleksandar Makelov

Done :)

Stefan Krastanov
Collaborator

SymPy Bot Summary: :red_circle: There were test failures.

@amakelov: Please fix the test failures.

Test command: setup.py test
master hash: 861a066
branch hash: 0fbc00d

Interpreter 1: :eight_spoked_asterisk: All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY4MUiDA

Interpreter 2: :red_circle: There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYjrYiDA

Interpreter 3: :red_circle: There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYqK4iDA

Build HTML Docs: :red_circle: There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYiY8iDA

Automatic review by SymPy Bot.

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged 0fbc00d into 861a066).

Stefan Krastanov
Collaborator
david joyner

+1 from me.

Stefan Krastanov Krastanov commented on the diff July 27, 2012
sympy/combinatorics/named_groups.py
((89 lines not shown))
36 70
 
37 71
     """
38  
-    if n == 1:
39  
-        G = PermutationGroup([Permutation([0])])
40  
-    elif n == 2:
41  
-        G = PermutationGroup([Permutation([1, 0])])
42  
-    else:
43  
-        a = range(1,n)
  72
+    # small cases are special
  73
+    if n == 1 or n == 2:
1
Stefan Krastanov Collaborator
Krastanov added a note July 27, 2012

just mentioning that you can write this as n in (1, 2). It is not "better", however I personally prefer it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Stefan Krastanov Krastanov commented on the diff July 27, 2012
sympy/combinatorics/named_groups.py
((89 lines not shown))
36 70
 
37 71
     """
38  
-    if n == 1:
39  
-        G = PermutationGroup([Permutation([0])])
40  
-    elif n == 2:
41  
-        G = PermutationGroup([Permutation([1, 0])])
42  
-    else:
43  
-        a = range(1,n)
  72
+    # small cases are special
  73
+    if n == 1 or n == 2:
  74
+        return PermutationGroup([Permutation([0])])
  75
+
  76
+    a = range(n)
  77
+    a[0], a[1], a[2] = a[1], a[2], a[0]
  78
+    gen1 = _new_from_array_form(a)
  79
+    if n % 2 == 1:
1
Stefan Krastanov Collaborator
Krastanov added a note July 27, 2012

you can write just if n%2: blah; else: blah;

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

My comments above were just mentioning how I would have written it. They are not related to correctness. As the mentor has reviewed this, and as I have not found any style issues I will merge this.

Concerning test failures:

  • travis: OK
  • without hash randomization: OK
  • with hash randomizations: No failures in this module
  • docs: no errors related to this module
Stefan Krastanov Krastanov merged commit ab87ae7 into from July 27, 2012
Stefan Krastanov Krastanov closed this July 27, 2012
Stefan Krastanov
Collaborator

By the way, do you use ./bin/coverage_report.py?

Aleksandar Makelov

Thanks, your suggestions about the style are appreciated - I'll get them in in my next pull request since I also find them more suitable.

Aleksandar Makelov

I have not; what is this?

Stefan Krastanov
Collaborator
Aleksandar Makelov

OK, I'm running it right now. The tests are taking ages though.

Aleksandar Makelov

I see. Cool :)

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

Showing 9 unique commits by 1 author.

Jul 26, 2012
Aleksandar Makelov Randomized Schreier Sims, abelian groups cf890b5
Aleksandar Makelov Backtrack search printing all group elements
	Implemented a procedure that prints all group elements
	in the lexicographical ordering induced by a base (i.e.,
	according to the images of the base under the action
	of the group elements).

	This basic procedure that visits all elements will be
	used with some modifications for other backtracking
	algorithms for permutation groups.
06a0494
Aleksandar Makelov Implemented baseswap (needs debugging)
	The algorithm baseswap is used to swap two consecutive
	elements in the base of a permutatation group. The
	current version breaks somewhere and needs some debugging.
b670ff0
Aleksandar Makelov Fixed the bug in BASESWAP, refactored the code
The reason the implementation of BASESWAP suggested
in the "Handbook..." was failing is that (I believe)
there is a mistake in the book. Now that I noticed
it, it seems to work fine. This will be further
discussed in my blog.

Also, implemented a randomized version of BASESWAP
that is supposed to run faster than the deterministic
one.

Introduced some new utility functions to help
handling computations using a BSGS, and refactored
PRINTELEMENTS and BASESWAP accordingly.

Moved the current utility functions to a new file,
util.py, and moved the constructor for an abelian
group to the named_groups.py file.
00a819d
Aleksandar Makelov Docstrings, tests, refactoring
Wrote docstrings and tests for the new functionality,
and did some refactoring in the new functions.

Also, made possible the interaction with the
deterministic version of the Schreier-Sims algorithm
by introducing some new attributes.

Reorganized the util.py file containing helper functions for
the combinatorics module.

Altered _strip in sympy/combinatorics/util.py
so that it accepts basic transversals as an argument, instead of
basic stabilizers, so that finding transversal elements is done
in constant time. Altered _distribute_gens_by_base in the same
file slightly.
c9f9808
Aleksandar Makelov Removed list_lex_by_base and related tests/references.
The function list_lex_by_base served as a stepping stone in
my understanding of more complex backtracking routines, so
I'm removing it now. There are other (and faster) functions
that list all group elements.
547b393
Aleksandar Makelov Improved some of the docstrings. 565e34a
Jul 27, 2012
Aleksandar Makelov Removed unnecessary blank lines from perm_groups.py 3ad1c91
Aleksandar Makelov Ordered the methods in several files alphabetically.
These were: the standalone methods in named_groups.py and
util.py, and the class methods for the classes _JGraph
and PermutationGroup in perm_groups.py.
0fbc00d
Something went wrong with that request. Please try again.