patternpythonMinor
Cycle decomposition in Python
Viewed 0 times
decompositioncyclepython
Problem
Is there an algorithmically better way of doing this?
import numpy as np
l=np.random.permutation(10)
p=list(range(10))
print(l,p)
i=0
k=0
prod=1
z=0
def gcd(a, b):
while b:
a, b = b, a % b
return a
def lcm(a, b):
return a * b // gcd(a, b)
while z<10:
"""count how many times we have crossed (-1-ed) a number from p"""
k=0
while p[k]==-1 and k<9 :
"""find the first non -1 element in p (it's here that I feel that the code is especially clumsy) """
k=k+1
i=k
"""k is the first element in the cycle """
print(l[i])
p[k]=-1
z=z+1
j=1
while l[i]!=k:
"""go from i to l[i] until we get back to k, closing the cycle """
p[l[i]]=-1
z=z+1
i=l[i]
j+=1
"""g is just used to find the lcm """
print(l[i])
prod=lcm(prod,j)
print('------')
print(prod)Solution
Use the built in
In Python 3, you can just do:
Consider keeping with the standard library
Instead of using
Just use
Comments such as:
Are usually used for docstrings, you are using them to elaborate on certain steps in an algorithm, I would use
Place the cycle decomposition into a function
The while loop introduces a lot of global variables and makes it hard to use, I would put the contents into a function. Also explain that this function displays a permutation as disjoint cycles.
Please use better names.
I'm still not sure what every variable does. Like why is
gcdIn Python 3, you can just do:
from math import gcdConsider keeping with the standard library
Instead of using
np.random.permutation, consider just using random.shuffle to generate a random permutation. Removes the numpy dependency altogether.Just use
# instead of """Comments such as:
"""count how many times we have crossed (-1-ed) a number from p"""Are usually used for docstrings, you are using them to elaborate on certain steps in an algorithm, I would use
# for this.Place the cycle decomposition into a function
The while loop introduces a lot of global variables and makes it hard to use, I would put the contents into a function. Also explain that this function displays a permutation as disjoint cycles.
Please use better names.
I'm still not sure what every variable does. Like why is
p=list(range(10))? I would think p is short for permutation, but l seems to be what the actual permutation is. This program is really hard to understand simply because of your name choice.Code Snippets
from math import gcd"""count how many times we have crossed (-1-ed) a number from p"""Context
StackExchange Code Review Q#161704, answer score: 7
Revisions (0)
No revisions yet.