Breadth first search inefficiencies

Question

I'm writing a simple breadth first search algorithm is Scala and I feel like it should be pretty efficient. However when I run this one some relatively small problems I'm managing to run out of memory.

def search(start: State): Option[State] = {
    val queue: mutable.Queue[State] = mutable.Queue[State]()
    queue.enqueue( start )
    while( queue.nonEmpty ){
      val node = queue.dequeue()
      if( self.isGoal(node) )
        return Some(node)
      self.successors(node).foreach( queue.enqueue )
    }
    None
}

I believe the enqueue and dequeue methods on a mutable queue were constant time and for each is implemented efficiently. The methods isGoal and successors I know are as efficient as they as they can be. I don't understand how I can be running out of memory so quickly. Are there any inefficiencies in this code that I'm missing?

I do not know Scala but I think you are missing one thing: check if a node has already been tested, before enqueuing it. Otherwise you may have endless loops. For example if node `a` is a neighbor of `b`, testing `a` should enqueue `b`. Later when you test `b` node `a` will be enqueue again, because it is a neighbor of `b`, — c0der, Apr 08 '20 at 04:58

score 1 · Answer 1 · answered Apr 08 '20 at 05:35

I think c0der's comment nailed it: you may be getting caught in an infinite loop re-checking nodes that you've already visited. Consider the following changes:

def search(start: State): Option[State] = {
    var visited: Set[State] = Set() // change #1
    val queue: mutable.Queue[State] = mutable.Queue[State]()
    queue.enqueue( start )
    while( queue.nonEmpty ){
      val node = queue.dequeue()
      if (!visited.contains(node)) { // change #2
        visited += node // change #3
        if( self.isGoal(node) )
          return Some(node)
        self.successors(node).foreach( queue.enqueue )
      }
    }
    None
}

Initialize a new Set, visited, to keep track of which Nodes you've been to.
Immediately after dequeueing a Node, check if you've visited it before. If not, continue checking this Node. Otherwise, ignore it.
Make sure to add this Node to the visited Set so it's not checked again in the future.

Hope that helps :D

score 1 · Answer 2 · answered Apr 08 '20 at 09:25

You have some Java, not Scala code there. For Scala vars and while is something that you should not use at all. Here is my suggestion how you could solve this.

 class State(val neighbours: List[State]) // I am not sure how your State class looks like, but it could look something like this

  val goal = new State(List())

  def breathFirst(start: State): Option[State] = {

    @scala.annotation.tailrec
    def recursiveFunction(visited: List[State], toVisit: List[State]): Option[State] = { // So we will create recursive function with visited nodes and nodes that we should visit
      if (toVisit.isEmpty) return None // If toVisit is empty that means that there is no path from start to goal, return none
      else {
        val visiting = toVisit.head // Else we should take first node from toVisit
        val visitingNeighbours = visiting.neighbours // Take all neighbours from node that we are visiting
        val visitingNeighboursNotYetVisited = visitingNeighbours.filter(x => !visited.contains(x)) //Filter all neighbours that are not visited
        if (visitingNeighboursNotYetVisited.contains(goal)) { //if we found goal, return it
          return Some(goal)
        } else {
          return recursiveFunction(visited :+ visiting, toVisit.tail ++ visitingNeighboursNotYetVisited) // Otherwise add node that we visited in this iteration to list of visited nodes that does not have visited node - it was head so we take toVisit.tail
          // and also we will take all neighbours that are not visited and add them to toVisit list for next iteration
        }
      }
    }

    if (start == goal) { // If goal is start, return start
      Some(start)
    } else { // else call our recursive function with empty visited list and with toVisit list that has start node
      recursiveFunction(List(), List(start))
    }
  }

NOTE: You could change:

val visitingNeighboursNotYetVisited = visitingNeighbours.filter(x => !visited.contains(x)) //Filter all neighbours that are not visited

with

val visitingNeighboursNotYetVisited = visitingNeighbours

and check if you will go out of memory, and, as probably you wont it will show you why you should use tailrec.

Breadth first search inefficiencies

2 Answers2