This is based on a very similar question I asked, found here.
The intent of this program is to start with a set number of random strings of zeros and ones, and eventually evolve them to find one specific string ('111111'
).
It fulfils its purpose, but what do you think, and how can I improve it?
import random as r
import operator
import time
import os
target = '111111'
class String(object):
def __init__(self, chromo):
self.chromo = chromo
self.fitness = 5
self.age = 0
def Fitness(string):
for char in string.chromo:
if char == '1':
string.fitness += 1
else:
pass
def SelectParents(strings):
parents = []
parents.append(max(strings, key=operator.attrgetter('fitness')))
strings.remove(parents[0])
parents.append(max(strings, key=operator.attrgetter('fitness')))
strings.append(parents[0])
return parents
def PartStringRand():
string_complete = ''
for _ in range(4):
rand = r.randint(0, 1)
string_element = str(rand)
string_complete = ''.join([string_complete, string_element])
return string_complete
def CreateChild(strings, parents):
for children in range(2):
string_complete = PartStringRand()
string_complete += (parents[0].chromo[r.randint(0, 5)] + parents[1].chromo[r.randint(0, 5)])
child = String(string_complete)
strings.append(child)
def KillOld(strings):
for _ in strings:
if _.age > 2:
strings.remove(_)
#First strings
strings = []
for _ in range(5):
string_complete = ''
for _ in range(6):
rand = r.randint(0, 1)
string_element = str(rand)
string_complete = ''.join([string_complete, string_element])
string = String(string_complete)
strings.append(string)
if string.chromo == "111111":
strings.remove(string)
for string in strings:
print string.chromo
raw_input()
#Start incremental mutation
c = 0
for _ in range(5000000):
#os.system("cls")
for string in strings:
Fitness(string)
parents = SelectParents(strings)
CreateChild(strings, parents)
for organism in strings:
organism.age += 1
print organism.chromo
if organism.chromo == '111111':
print "::...Successfully evolved specifc string '111111' from random strings...::"
time.sleep(4)
sys.exit()
KillOld(strings)
c += 1
print "Generation: %s" %c
#time.sleep(0.3)
-
\$\begingroup\$ "I believe it fulfills [sic] its purpose" - have you tested it? \$\endgroup\$jonrsharpe– jonrsharpe2014年05月16日 15:31:02 +00:00Commented May 16, 2014 at 15:31
-
\$\begingroup\$ Yes, it does work. Sorry for being too ambiguous. \$\endgroup\$FrigidDev– FrigidDev2014年05月16日 15:34:05 +00:00Commented May 16, 2014 at 15:34
2 Answers 2
One thing that immediately jumped out:
def KillOld(strings):
for _ in strings:
if _.age > 2:
strings.remove(_)
_
is used by convention to indicate "I won't be using this variable", e.g.:
for _ in range(5):
do_something()
If you are using the variable, give it a better name (even just s
). Also, functions' names should be lowercase_with_underscores
per PEP-0008.
Next, your mutation seems odd, as it generates four new random characters then picks two from the parents. Instead, I would have done:
def create_child(parents):
return String("".join(r.choice(c) for c in zip(*(p.chromo for p in parents))))
i.e. for each position, pick randomly between the two parents.
Note that the function returns the new String
, rather than adding it to strings
. I would say that:
strings.append(create_child(parents))
is clearer than adding to strings
as a side-effect. Similarly, I would probably rewrite KillOld
to return a new list, rather than mutate its argument (and call it kill_old
).
You can simplify
string_complete = ''
for _ in range(6):
rand = r.randint(0, 1)
string_element = str(rand)
string_complete = ''.join([string_complete, string_element])
string = String(string_complete)
to:
s = String("".join(map(str, (r.randint(0, 1) for _ in range(6)))))
(don't use string
as a variable name; you may want to use that module later). And rather than append
then remove
it, check first:
if s.chromo != target:
strings.append(s)
(Note that you define target
then don't use it - replace all '111111'
with target
and you can change it much more easily in future!)
Rather than calculate the s.fitness
for each s
, then have a complex key
:
parents.append(max(strings, key=operator.attrgetter('fitness')))
make the function fitness
that takes a single String
and returns the fitness score:
def fitness(s):
return sum(c == t for c, t in zip(s.chromo, target))
Your key
is then e.g.:
parents.append(max(strings, key=fitness))
This also decouples the fitness from the String
object itself; you no longer need the s.fitness
attribute.
You could also generally improve SelectParents
; remove
and append
again is not efficient.
def select_parents(strings):
return sorted(strings, key=fitness)[-2:]
This sorts strings
in ascending fitness, then returns a sliced list of the last (up to) two items.
-
\$\begingroup\$ Good answer, I will take into consideration your suggestions. \$\endgroup\$FrigidDev– FrigidDev2014年05月16日 15:51:53 +00:00Commented May 16, 2014 at 15:51
-
\$\begingroup\$ @FrigidDev no problem; I've just added another comment \$\endgroup\$jonrsharpe– jonrsharpe2014年05月16日 15:56:02 +00:00Commented May 16, 2014 at 15:56
I second jonrsharpe's answer. In addition...
Python function names conventionally begin with lower case: fitness
, select_parents
, etc. UpperCase
is for classes.
Calling a class String
is confusing, because of the built-in string type. How about Organism
?
Fitness
should be a function (or a method Organism.fitness
) returning a fitness. It should not modify the organism! Anyway, there's a simpler way to do it: return self.chromo.count('1')
.
SelectParents
should not modify its argument. In general, don't use state unless you mean state.
What's the point of sleep
before exit
? Just finish.
In the main loop, c
duplicates the loop variable. They can be combined: for generation in range(1000000):
...