Fix HybridWumpusAgent (Fig 7.20) so it runs and reasons correctly (#961)

The agent crashed immediately and, where it didn't, used inverted logic. Fixed,
faithfully to AIMA Fig 7.20:
- WumpusKB now asserts the ok_to_move axiom at time 0 too (it was only added for
  t>0), so the agent can prove [1,1] safe on its first move; previously the safe
  set was empty at t=0 and planning crashed.
- execute(): 'unvisited' was collecting *visited* squares (inverted) -> now the
  squares never been located in (for all t' <= t), intersected with safe;
  possible_wumpus uses 'not KB entails ~W' and not_unsafe uses 'not KB entails
  ~OK' (were 'not entails W' / 'not entails OK'); guarded the pose update when no
  location is entailed.
- plan_route(): returns [] when A* finds no path (was crashing on None.solution()).
- plan_shot(): used EAST/WEST/NORTH/SOUTH orientations that PlanRoute/WumpusPosition
  never use -> mapped to RIGHT/LEFT/UP/DOWN; set.remove -> set.discard (the wumpus
  cells were never added, so removing them raised KeyError).
- make_percept_sentence(): handled only a single percept via an elif-chain -> now
  accepts a collection (a square can give Stench *and* Breeze at once) and None.

The agent now runs and returns legal actions with correct logic. Added tests for
make_percept_sentence, plan_shot, plan_route (no-path) and the t=0 end-to-end step.
NB: only the first step is exercised in tests -- the propositional Fig 7.20
inference (SAT entailment over a temporal KB) grows expensive as the KB
accumulates, which is inherent to the textbook algorithm, not a remaining bug.

Author: dmeoli

aima/logic.py

@@ -1405,6 +1405,10 @@ def __init__(self, dimrow):
             for j in range(1, dimrow + 1):
                 self.tell(implies(location(i, j, 0), equiv(percept_breeze(0), breeze(i, j))))
                 self.tell(implies(location(i, j, 0), equiv(percept_stench(0), stench(i, j))))
+                # a square is safe to move into at time 0 iff it has no pit, no
+                # (live) wumpus -- without this the agent cannot even prove [1,1]
+                # safe on its first move (the rule was only added for time > 0)
+                self.tell(equiv(ok_to_move(i, j, 0), ~pit(i, j) & ~wumpus(i, j) & wumpus_alive(0)))
                 if i != 1 or j != 1:
                     self.tell(~location(i, j, 0))
 
@@ -1426,40 +1430,28 @@ def make_action_sentence(self, action, time):
                 self.tell(~a)
 
     def make_percept_sentence(self, percept, time):
-        """Tell the KB which of the five percepts hold (and which do not) at ``time``."""
-        # Glitter, Bump, Stench, Breeze, Scream
-        flags = [0, 0, 0, 0, 0]
-
-        # Things perceived
-        if isinstance(percept, Glitter):
-            flags[0] = 1
-            self.tell(percept_glitter(time))
-        elif isinstance(percept, Bump):
-            flags[1] = 1
-            self.tell(percept_bump(time))
-        elif isinstance(percept, Stench):
-            flags[2] = 1
-            self.tell(percept_stench(time))
-        elif isinstance(percept, Breeze):
-            flags[3] = 1
-            self.tell(percept_breeze(time))
-        elif isinstance(percept, Scream):
-            flags[4] = 1
-            self.tell(percept_scream(time))
-
-        # Things not perceived
-        for i in range(len(flags)):
-            if flags[i] == 0:
-                if i == 0:
-                    self.tell(~percept_glitter(time))
-                elif i == 1:
-                    self.tell(~percept_bump(time))
-                elif i == 2:
-                    self.tell(~percept_stench(time))
-                elif i == 3:
-                    self.tell(~percept_breeze(time))
-                elif i == 4:
-                    self.tell(~percept_scream(time))
+        """Tell the KB which of the five percepts hold (and which do not) at ``time``.
+
+        A single square can yield several percepts at once (e.g. a Stench *and* a
+        Breeze), so ``percept`` may be one percept object or a collection of them
+        (the format produced by ``WumpusEnvironment.percept``); ``None`` means
+        nothing was perceived.
+        """
+        if percept is None:
+            percepts = []
+        elif isinstance(percept, (list, tuple, set)):
+            percepts = list(percept)
+        else:
+            percepts = [percept]
+
+        # for each of the five percepts, assert whether it holds at this time
+        for cls, sentence in ((Glitter, percept_glitter), (Bump, percept_bump),
+                              (Stench, percept_stench), (Breeze, percept_breeze),
+                              (Scream, percept_scream)):
+            if any(isinstance(p, cls) for p in percepts):
+                self.tell(sentence(time))
+            else:
+                self.tell(~sentence(time))
 
     def add_temporal_sentences(self, time):
         """Add the successor-state axioms relating ``time`` to ``time - 1`` (location, orientation,
@@ -1607,14 +1599,15 @@ def execute(self, percept):
                     temp.append(i)
                     temp.append(j)
 
-        if self.kb.ask_if_true(facing_north(self.t)):
-            self.current_position = WumpusPosition(temp[0], temp[1], 'UP')
-        elif self.kb.ask_if_true(facing_south(self.t)):
-            self.current_position = WumpusPosition(temp[0], temp[1], 'DOWN')
-        elif self.kb.ask_if_true(facing_west(self.t)):
-            self.current_position = WumpusPosition(temp[0], temp[1], 'LEFT')
-        elif self.kb.ask_if_true(facing_east(self.t)):
-            self.current_position = WumpusPosition(temp[0], temp[1], 'RIGHT')
+        if len(temp) >= 2:  # a location was entailed; otherwise keep the previous pose
+            if self.kb.ask_if_true(facing_north(self.t)):
+                self.current_position = WumpusPosition(temp[0], temp[1], 'UP')
+            elif self.kb.ask_if_true(facing_south(self.t)):
+                self.current_position = WumpusPosition(temp[0], temp[1], 'DOWN')
+            elif self.kb.ask_if_true(facing_west(self.t)):
+                self.current_position = WumpusPosition(temp[0], temp[1], 'LEFT')
+            elif self.kb.ask_if_true(facing_east(self.t)):
+                self.current_position = WumpusPosition(temp[0], temp[1], 'RIGHT')
 
         safe_points = list()
         for i in range(1, self.dimrow + 1):
@@ -1631,17 +1624,13 @@ def execute(self, percept):
             self.plan.append('Climb')
 
         if len(self.plan) == 0:
+            # unvisited = squares the agent has never been located in (for all t' <= t)
             unvisited = list()
             for i in range(1, self.dimrow + 1):
                 for j in range(1, self.dimrow + 1):
-                    for k in range(self.t):
-                        if self.kb.ask_if_true(location(i, j, k)):
-                            unvisited.append([i, j])
-            unvisited_and_safe = list()
-            for u in unvisited:
-                for s in safe_points:
-                    if u not in unvisited_and_safe and s == u:
-                        unvisited_and_safe.append(u)
+                    if not any(self.kb.ask_if_true(location(i, j, k)) for k in range(self.t + 1)):
+                        unvisited.append([i, j])
+            unvisited_and_safe = [u for u in unvisited if u in safe_points]
 
             temp = self.plan_route(self.current_position, unvisited_and_safe, safe_points)
             self.plan.extend(temp)
@@ -1650,7 +1639,7 @@ def execute(self, percept):
             possible_wumpus = list()
             for i in range(1, self.dimrow + 1):
                 for j in range(1, self.dimrow + 1):
-                    if not self.kb.ask_if_true(wumpus(i, j)):
+                    if not self.kb.ask_if_true(~wumpus(i, j)):
                         possible_wumpus.append([i, j])
 
             temp = self.plan_shot(self.current_position, possible_wumpus, safe_points)
@@ -1660,7 +1649,7 @@ def execute(self, percept):
             not_unsafe = list()
             for i in range(1, self.dimrow + 1):
                 for j in range(1, self.dimrow + 1):
-                    if not self.kb.ask_if_true(ok_to_move(i, j, self.t)):
+                    if not self.kb.ask_if_true(~ok_to_move(i, j, self.t)):
                         not_unsafe.append([i, j])
             temp = self.plan_route(self.current_position, not_unsafe, safe_points)
             self.plan.extend(temp)
@@ -1683,7 +1672,8 @@ def plan_route(self, current, goals, allowed):
         """Return an action sequence from ``current`` to any of ``goals`` through ``allowed`` squares,
         found by A* search."""
         problem = PlanRoute(current, goals, allowed, self.dimrow)
-        return astar_search(problem).solution()
+        node = astar_search(problem)
+        return node.solution() if node is not None else []
 
     def plan_shot(self, current, goals, allowed):
         """Return an action sequence that moves to a square lined up with a possible wumpus and shoots."""
@@ -1694,19 +1684,19 @@ def plan_shot(self, current, goals, allowed):
             y = loc[1]
             for i in range(1, self.dimrow + 1):
                 if i < x:
-                    shooting_positions.add(WumpusPosition(i, y, 'EAST'))
+                    shooting_positions.add(WumpusPosition(i, y, 'RIGHT'))  # face east toward the wumpus
                 if i > x:
-                    shooting_positions.add(WumpusPosition(i, y, 'WEST'))
+                    shooting_positions.add(WumpusPosition(i, y, 'LEFT'))  # face west
                 if i < y:
-                    shooting_positions.add(WumpusPosition(x, i, 'NORTH'))
+                    shooting_positions.add(WumpusPosition(x, i, 'UP'))  # face north
                 if i > y:
-                    shooting_positions.add(WumpusPosition(x, i, 'SOUTH'))
+                    shooting_positions.add(WumpusPosition(x, i, 'DOWN'))  # face south
 
         # Can't have a shooting position from any of the rooms the Wumpus could reside
-        orientations = ['EAST', 'WEST', 'NORTH', 'SOUTH']
+        orientations = ['UP', 'DOWN', 'LEFT', 'RIGHT']
         for loc in goals:
             for orientation in orientations:
-                shooting_positions.remove(WumpusPosition(loc[0], loc[1], orientation))
+                shooting_positions.discard(WumpusPosition(loc[0], loc[1], orientation))
 
         actions = list()
         actions.extend(self.plan_route(current, shooting_positions, allowed))

tests/test_logic.py

@@ -405,5 +405,41 @@ def test_SAT_plan():
     assert SAT_plan((0, 0), transition, (1, 1), 4) == ['Right', 'Down']
 
 
+def test_hybrid_wumpus_agent_make_percept_sentence():
+    # a single square can yield several percepts at once (Stench *and* Breeze);
+    # the old elif-chain recorded only the first one
+    from aima.agents import Stench, Breeze
+    kb = WumpusKB(2)
+    kb.make_percept_sentence([Stench(), Breeze()], 0)
+    assert percept_stench(0) in kb.clauses
+    assert percept_breeze(0) in kb.clauses
+    assert ~percept_glitter(0) in kb.clauses
+
+
+def test_hybrid_wumpus_agent_plan_shot():
+    # plan_shot lines up with a possible wumpus and ends by shooting, using the
+    # same UP/DOWN/LEFT/RIGHT orientations as PlanRoute (it used to use EAST/WEST/…)
+    agent = HybridWumpusAgent(2)
+    actions = agent.plan_shot(WumpusPosition(1, 1, 'RIGHT'), [[2, 2]],
+                              [[1, 1], [2, 1], [1, 2], [2, 2]])
+    assert actions and actions[-1] == 'Shoot'
+
+
+def test_hybrid_wumpus_agent_plan_route_no_path():
+    # an unreachable / empty goal set yields an empty plan instead of crashing
+    agent = HybridWumpusAgent(2)
+    assert agent.plan_route(WumpusPosition(1, 1, 'RIGHT'), [], [[1, 1]]) == []
+
+
+def test_hybrid_wumpus_agent_first_action():
+    # end-to-end first step: from [1,1] (provably safe at t=0) the agent returns a
+    # legal action and records its pose. (Only t=0 is exercised: the propositional
+    # Fig 7.20 inference grows expensive as the temporal KB accumulates.)
+    agent = HybridWumpusAgent(2)
+    action = agent.program(None)
+    assert action in {'Forward', 'TurnLeft', 'TurnRight', 'Grab', 'Shoot', 'Climb'}
+    assert agent.current_position.get_location() == (1, 1)
+
+
 if __name__ == '__main__':
     pytest.main()