HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Genetic search algorithm for TSP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tspsearchgeneticalgorithmfor

Problem

I made a genetic search algorithm in Python for the Travelling Salesman Problem for a midterm project. The grade was fine, but I was hoping to get some pointers on style and documentation. Please provie any feedback you have about how I can make my code more readable, consistent, and friendly.

```
"""
geneticTS.py

By Logan Davis | 10/25/15

Description: A genetic solution to the Traveling Saleman
problem for the AI Midterm.

Python 2.7 | Editor: Emacs | Distro: Ubuntu 15
"""
import random, math

class Citizen(object):
"""
Citizen is small object meant to be used by
an instance of a TravelingSalesmen object.
All the Citizen class can do is hold two
attributes: route (it's planned travelsal
through a series of locations) and fitness
(the distance of that route. The smaller,
the better).
"""
def __init__(self,route = None, fitness = 0):
self.route = route
self.fitness = fitness

class TravelingSalesmen(object):
"""
OVERVIEW:
A genetic algorithm class to generate answers to the
Traveling Salesmen problem.

HOW TO USE:
This simplist way to use this is by creating an instance
of this class, and then call self.generate() where self is the
instance you just created. However this method can be slow
because you have to enter each location in one at a time.
One of the optional arguements is to directly assign an
organized route to the route arguement. An organized is a list
of locations, each location is a list containing a name/identifier
as the first element, an x co-ord as the second, and a y co-ord
as the last. EXAMPLE OF LOCATION:
["Jutland", 45, 2000.3]
After either directly assigning these locations in a list, you still have
call the self.generate() method, but it will skip constructing a route and
just generate an answer. If you don't directly assign a route, after you
input the route

Solution

Code:

It appears to me that your code style is just fine! I have only a few comments:

Whenever you see something like for i in range(len(mylist)), try to see if you can use a "for-in" loop. An example of this is your function _mutate(). The whole function can be rewritten as:

for citizen in self.population:
    if random.random() <= self.mutationRate:
        index1 = random.randint(0,len(self.route)-1)
        index2 = random.randint(0,len(self.route)-1)
        copy = citizen.route[index1]
        citizen.route[index1] = citizen.route[index2]
        citizen.route[index2] = copy


There is even possible to rewrite the flip stage as

citizen.route[i1], citizen.route[i2] = citizen.route[i2], citizen.route[i1]


Index is in my opinion self explanatory, so there is no need to use the whole index word.

Another issue I see is that it is hard to tell if some functions mutate (that means change) the state of the model given the functions name and description. For example _fitness(). This function changes the state of your class, but neither the function name nor the description explains this. Consider changing the name to _update_fitness() or something similar.

Comments:

Everyone disagrees when it comes to how to comment code. When that is said, there is some consensus. Self documenting code is the best. Try to write your code as clear as possible. If you can do this, then there is not need for to many comments.

I like that you use block comments on function and classes, but some of the are not necessary in my opinion. The rule here is basically to write code that explains why you do something. It appears to me that you have a tendency to explain the implementation, though the implementation can be seen just fine by the code. An example:

def _mutate(self):
    """
    _mutate iterates through the entire
    self.population. If a random.random()
    call returns a value <= self.mutationRate,
    then two random locations in a single 
    citizen's route are flipped.
    """
    for i in xrange(0,len(self.population)):
        if random.random() <= self.mutationRate:
            index1 = random.randint(0,len(self.route)-1)
            index2 = random.randint(0,len(self.route)-1)
            copy = self.population[i].route[index1]
            self.population[i].route[index1] = self.population[i].route[index2]
            self.population[i].route[index2] = copy


In my opinion, this code explains itself very well. I would look into another Stack Overflow answer as well as I feel it explains things well.

There are probably more stuff i could mention, an someone else might also disagree with the things I have pointed out, but at least you have someone elses opinion.

I also know this is for a school project. In a real life project, I would probable prefer less and clearer comments ;)

Code Snippets

for citizen in self.population:
    if random.random() <= self.mutationRate:
        index1 = random.randint(0,len(self.route)-1)
        index2 = random.randint(0,len(self.route)-1)
        copy = citizen.route[index1]
        citizen.route[index1] = citizen.route[index2]
        citizen.route[index2] = copy
citizen.route[i1], citizen.route[i2] = citizen.route[i2], citizen.route[i1]
def _mutate(self):
    """
    _mutate iterates through the entire
    self.population. If a random.random()
    call returns a value <= self.mutationRate,
    then two random locations in a single 
    citizen's route are flipped.
    """
    for i in xrange(0,len(self.population)):
        if random.random() <= self.mutationRate:
            index1 = random.randint(0,len(self.route)-1)
            index2 = random.randint(0,len(self.route)-1)
            copy = self.population[i].route[index1]
            self.population[i].route[index1] = self.population[i].route[index2]
            self.population[i].route[index2] = copy

Context

StackExchange Code Review Q#109811, answer score: 3

Revisions (0)

No revisions yet.