optimizing project euler #83 solution

Question

Project Euler problem 83 asks:

In the 5 by 5 matrix below,

131 673 234 103 18
201 96 342 965 150
630 803 746 422 111
537 699 497 121 956
805 732 524 37 331

the minimal path sum from the top left to the bottom right, by moving left, right, up, and down, is

131 → 201 → 96 → 342 → 234 → 103 → 18 → 150 → 111 → 422 → 121 → 37 → 331

and is equal to 2297.

Find the minimal path sum, in matrix.txt (right click and 'Save Link/Target As...'), a 31K text file containing a 80 by 80 matrix, from the top left to the bottom right by only moving right and down.

I have solved the project euler problem 83 using uniform cost search. This solution takes about 0.6s to solve. I want to know if anyone can get the code to run relatively faster without changing the general outline of the program.

import bisect
f = open('matrix.txt')
matrix = [[int(i) for i in j.split(',')] for j in f]
def uniformCostSearch(startNode):
 frontier = []
 frontier.append(startNode)
 closedSet = set()
 while not len(frontier) == 0:
 currentNode = frontier.pop(0)
 if currentNode.x == 79 and currentNode.y == 79:
 return currentNode.priority
 else:
 if not (currentNode.x, currentNode.y) in closedSet:
 closedSet.add((currentNode.x, currentNode.y))
 possibleMoves = currentNode.neighbors()
 for move in possibleMoves:
 if not (move.x, move.y) in closedSet:
 try:
 index = frontier.index(move)
 if move.priority < frontier[index].priority:
 frontier.pop(index)
 bisect.insort_left(frontier, move)
 except ValueError:
 # move is not in frontier so just add it
 bisect.insort_left(frontier, move)
 return -1
class Node:
 def __init__(self, x, y, priority=0):
 self.x = x
 self.y = y
 self.priority = priority
 def neighbors(self):
 tmp = [Node(self.x + 1, self.y), Node(self.x, self.y + 1),
 Node(self.x - 1, self.y), Node(self.x, self.y - 1),]
 childNodes = []
 for node in tmp:
 if node.x >= 0 and node.y >= 0 and node.x <= 79 and node.y <= 79:
 node.priority = self.priority + matrix[node.y][node.x]
 childNodes.append(node)
 return childNodes
 def __eq__(self, node):
 return self.x == node.x and self.y == node.y
 def __ne__(self, node):
 return not self.x == node.x and self.y == node.y
 def __cmp__(self, node):
 if self.priority < node.priority:
 return -1
 elif self.priority > node.priority:
 return 1
 else:
 return 0

• cat_baxter: Why not use a classical Dijkstra?
• Jeremy K: I turned the matrix into a graph and used a shortest path algorithm for this problem which got me the solution in 0.069s. I'll take a look through yours though and see if I can optimize it in anyway.

Answer 1

import bisect
f = open('matrix.txt')
matrix = [[int(i) for i in j.split(',')] for j in f]

You could consider using the numpy library, it'll handle multidimensional arrays more neatly and efficiently.

def uniformCostSearch(startNode):

Python convention is to use underscore_with_letters for function names and parameters

 frontier = []
 frontier.append(startNode)

Use frontier = [startNode], it'll be a touch faster

 closedSet = set()
 while not len(frontier) == 0:

Use while frontier:, it does the same thing and due to less operations is going to be a bit faster. You should at least use != rather then not ==

 currentNode = frontier.pop(0)

Popping from the front of a list will be inefficient. Actually, what you really want is a priority queue, see python's deque module.

 if currentNode.x == 79 and currentNode.y == 79:
 return currentNode.priority
 else:
 if not (currentNode.x, currentNode.y) in closedSet:

Move the not over next to in I think it reads clearer. I might also use a 2D array of bools rather then a set here.

 closedSet.add((currentNode.x, currentNode.y))

There isn't really any reason to check if the node is in the set here, you can add the node multiple times. Just add it unconditionally.

 possibleMoves = currentNode.neighbors()
 for move in possibleMoves:

No reason to store it in a local, just combine the previous two lines

 if not (move.x, move.y) in closedSet:
 try:
 index = frontier.index(move)
 if move.priority < frontier[index].priority:
 frontier.pop(index)
 bisect.insort_left(frontier, move)

Move the last three lines into an else block. That'll make sure only your index line can actually throw things that'll get caught.

 except ValueError:
 # move is not in frontier so just add it
 bisect.insort_left(frontier, move)

You do this in either case, I'd move it after the try/except block. return -1

class Node:
 def __init__(self, x, y, priority=0):
 self.x = x
 self.y = y
 self.priority = priority
 def neighbors(self):
 tmp = [Node(self.x + 1, self.y), Node(self.x, self.y + 1),
 Node(self.x - 1, self.y), Node(self.x, self.y - 1),]
 childNodes = []
 for node in tmp:
 if node.x >= 0 and node.y >= 0 and node.x <= 79 and node.y <= 79:

I suggest moving 79 into a constant. I'd also do < 80 rather then <= 79.

 node.priority = self.priority + matrix[node.y][node.x]
 childNodes.append(node)

I'd use a yield here, and make this a generator

 return childNodes
 def __eq__(self, node):
 return self.x == node.x and self.y == node.y

This bothers me because multiple nodes will compare equal despite having different priorities.

 def __ne__(self, node):
 return not self.x == node.x and self.y == node.y
 def __cmp__(self, node):
 if self.priority < node.priority:
 return -1
 elif self.priority > node.priority:
 return 1
 else:
 return 0

This really bothers me because you aren't defining your comparisons in a consistent way.