runState inside a State Monadic function not working

I am trying to solve the problem 2.8 of "AI - A Modern Approach" book which involves a grid of cells and choosing random moves to navigate the grid.

2.7 Implement an environment for a n X m rectangular room, where each square has a 5% chance of containing dirt, and n and m are chosen at random from the range 8 to 15, inclusive.

2.8 Design and implement a pure reflex agent for the environment of Exercise 2.7, ignoring the requirement of returning home, and measure its performance.

So I have used two state monads - one with Grid as the state and another with StdGen as the state. The code compile without any error but when I run it from GHCi, it gets stuck and does not return.

The relevant part of the code:

Supporting code

type RandomState = State StdGen

makeGrid :: (Int, Int) -> (Int, Int) -> Float -> RandomState Grid

doAction :: Action -> Cleaner -> State Grid Cleaner

getRandomR :: Random a => (a, a) -> RandomState a
getRandomR limits = do
  gen <- get
  let (val, gen') = randomR limits gen
  put gen'
  return val

chooseAction :: Percepts -> RandomState Action
chooseAction percepts
  | PhotoSensor `elem` percepts = return SuckDirt
  | InfraredSensor `elem` percepts = return TurnOff
  | TouchSensor `elem` percepts = return TurnLeft
  | otherwise = do
    r <- getRandomR ((1, 3) :: (Int, Int))
    case r of
      1 -> return GoForward
      2 -> return TurnRight
      3 -> return TurnLeft

Main code

runCleaner :: Int -> Cleaner -> StateT Grid RandomState Cleaner
runCleaner turnsLeft cleaner@(Cleaner _ _ _ ph _) =
  if turnsLeft == 0
    then return cleaner
    else do
      grid <- get
      gen <- lift $ get
      cleaner <- case ph of
        [] -> do
          let (cleaner, grid) = runState (doAction GoForward cleaner) grid
          put grid
          return cleaner
        _ -> do
          let (action, gen) = runState (chooseAction (head ph)) gen
          lift $ put gen

          let (cleaner, grid) = runState (doAction action cleaner) grid
          put grid
          return cleaner

      case clState cleaner of
        Off -> return cleaner
        On -> runCleaner (turnsLeft - 1) cleaner

simulateOnGrid :: Int -> Grid -> StdGen -> (Cleaner, Grid)
simulateOnGrid maxTurns grid gen = 
  evalState (runStateT (runCleaner maxTurns cleaner) grid) gen
  where cleaner = createCleaner (fromJust $ cell (0,0) grid) East

I invoke the simulateOnGrid function from GHCi like this:

> gen <- newStdGen
> let grid = evalState (makeGrid (8,15) (8,15) 0.05) gen
> simulateOnGrid 5 grid gen

and code gets stuck at the line:

let (cleaner, grid) = runState (doAction GoForward cleaner) grid

which I have confirmed by putting traces in the code. The call to the doAction function never hap开发者_开发问答pens.

The issue seems to be the use of runState inside the runCleaner function, but I am unable to find any reason for it.

Please explain the reason and if there is a way to solve this issue.

Also, using runState inside the a monadic function feels wrong to me. Please suggest if there is a better way to do it.

In the right hand side of a let binding, the names being bound are in scope, so when you write

let (cleaner, grid) = runState (doAction GoForward cleaner) grid

the cleaner and grid on the right hand side of the = are the same ones as the ones on the left hand side. This will probably cause an infinite loop as you're feeding the output of the action back as its input! To avoid this, use different names for the output.

let (cleaner', grid') = runState (doAction GoForward cleaner) grid

That aside, you're absolutely right that using runState like this is odd. I think you can simplify things greatly if you change the type of doAction to

doAction :: Monad m => Action -> Cleaner -> StateT Grid m Cleaner

You didn't provide the body for this function, but I'm guessing it will still work with this less constrained type signature.

Now you don't have to fiddle around with getting and putting the state manually anymore, since doAction can be run directly in your monad, and chooseAction can be run by lifting it first. Using this, your case expression can be written much more succinctly:

cleaner <- case ph of
    [] -> doAction GoForward cleaner
    _  -> do action <- lift $ chooseAction (head ph)
             doAction action cleaner