Code cleaned up.

Author: RuedigerLunde

aima-core/src/main/java/aima/core/logic/planning/Graph.java

@@ -14,17 +14,19 @@
  * @author samagra
  */
 public class Graph {
-    ArrayList<Level> levels;// Levels
-    Problem problem;// The planning problem
-    List<ActionSchema> propositionalisedActions;
+    private ArrayList<Level> levels;// Levels
+    private List<ActionSchema> propositionalisedActions;
 
-    public Graph(Problem problem, Level initialLevel) {
-        this.problem = problem;
+    public Graph(Problem problem) {
         levels = new ArrayList<>();
-        levels.add(initialLevel);
+        levels.add(new Level(null, problem));
         propositionalisedActions = problem.getPropositionalisedActions();
     }
 
+    public Level getLevel(int i) {
+        return levels.get(i);
+    }
+
     public int numLevels() {
         return levels.size();
     }
@@ -33,18 +35,19 @@ public ArrayList<Level> getLevels() {
         return levels;
     }
 
-    public Problem getProblem() {
-        return problem;
-    }
-
     public List<ActionSchema> getPropositionalisedActions() {
         return propositionalisedActions;
     }
 
-    public Graph addLevel() {
-        Level lastLevel = levels.get(levels.size() - 1);
-        Level level = new Level(lastLevel, this.problem);
-        levels.add(level);
-        return this;
+    /**
+     * This method adds levels (an action and a state level) for a new state
+     * to the planning graph.
+     */
+    public void expand(Problem problem) {
+        Level level0 = levels.get(levels.size() - 1);
+        Level level1 = new Level(level0, problem); // new action level
+        Level level2 = new Level(level1, problem); // new state level
+        levels.add(level1);
+        levels.add(level2);
     }
 }

aima-core/src/main/java/aima/core/logic/planning/GraphPlanAlgorithm.java

@@ -42,9 +42,9 @@ public class GraphPlanAlgorithm {
      */
     public List<List<ActionSchema>> graphPlan(Problem problem) {
         // graph ← INITIAL-PLANNING-GRAPH(problem)
-        Graph graph = initialPlanningGraph(problem);
+        Graph graph = new Graph(problem);
         // goals ← CONJUNCTS(problem.GOAL)
-        List<Literal> goals = conjuncts(problem.getGoalState());
+        List<Literal> goals = problem.getGoalState().getFluents();
         // nogoods ← an empty hash table
         Hashtable<Integer, List<Literal>> nogoods = new Hashtable<>();
         Level state;
@@ -64,7 +64,7 @@ public List<List<ActionSchema>> graphPlan(Problem problem) {
             if (levelledOff(graph) && leveledOff(nogoods))
                 return null;
             // graph ← EXPAND-GRAPH(graph, problem)
-            graph = expandGraph(graph);
+            graph.expand(problem);
         }
     }
 
@@ -129,6 +129,7 @@ private List<List<ActionSchema>> extractSolution(Graph graph, List<Literal> goal
                         if (!newGoals.contains(literal))
                             newGoals.add(literal);
                 }
+                newGoals.sort(Comparator.comparing(Literal::hashCode)); // defined order necessary for nogood test
                 List<List<ActionSchema>> solution = extractSolution(graph, newGoals, level-2, nogoods);
                 if (solution != null) {
                     solution.add(possibleSet);
@@ -141,10 +142,9 @@ private List<List<ActionSchema>> extractSolution(Graph graph, List<Literal> goal
     }
 
     public List<ActionSchema> asFlatList(List<List<ActionSchema>> solution) {
-        List<ActionSchema> result = solution.stream().flatMap(Collection::stream)
+        return solution.stream().flatMap(Collection::stream)
                 .filter(actionSchema -> !ActionSchema.NO_OP.equals(actionSchema.getName()))
                 .collect(Collectors.toList());
-        return result;
     }
 
     /**
@@ -174,20 +174,9 @@ private boolean checkAllGoalsNonMutex(Level level, List<Literal> goals) {
         return (!mutexCheck);
     }
 
-    /**
-     * This method adds a new state (a state level and an action level both) to the planning graph.
-     *
-     * @param graph The planning graph.
-     * @return The expanded graph.
-     */
-    private Graph expandGraph(Graph graph) {
-        return graph.addLevel().addLevel();
-    }
-
     /**
      * A graph is said to be levelled off if two consecutive levels are identical.
      *
-     * @param nogoods
      * @return Boolean stating if the hashtable is levelled off.
      */
     private boolean leveledOff(Hashtable<Integer, List<Literal>> nogoods) {
@@ -199,34 +188,30 @@ private boolean leveledOff(Hashtable<Integer, List<Literal>> nogoods) {
     /**
      * A graph is said to be levelled off if two consecutive levels are identical.
      *
-     * @param graph
      * @return Boolean stating if the graph is levelled off.
      */
     private boolean levelledOff(Graph graph) {
         if (graph.numLevels() < 2)
             return false;
-        return graph.levels.get(graph.numLevels() - 1).equals(graph.levels.get(graph.numLevels() - 2));
+        return graph.getLevel(graph.numLevels() - 1).equals(graph.getLevel(graph.numLevels() - 2));
     }
 
-    /**
-     * Returns a list of literals in a state.
-     *
-     * @param goalState
-     * @return List of literals.
-     */
-    private List<Literal> conjuncts(State goalState) {
-        return goalState.getFluents();
-    }
-
-    /**
-     * This method initialises the planning graph for a particular problem.
-     *
-     * @param problem The planning problem.
-     * @return Graph for the planning problem.
-     */
-    private Graph initialPlanningGraph(Problem problem) {
-        Level initialLevel = new Level(null, problem);
-        return new Graph(problem, initialLevel);
+    public List<List<ActionSchema>> generateCombinations(List<List<ActionSchema>> actionLists) {
+        List<List<ActionSchema>> result = new ArrayList<>();
+        if (actionLists.size() == 1) {
+            result.add(actionLists.get(0));
+        } else if (actionLists.size() > 1) {
+            result = combineTwoLists(actionLists.get(0), actionLists.get(1));
+            if (actionLists.size() > 2) {
+                for (int i = 2; i < actionLists.size(); i++)
+                    result = combineExtraList(result, actionLists.get(i));
+            }
+        }
+        // sorting by increasing number of actions might reduce execution costs of first found solution.
+        result =  result.stream().sorted
+                (Comparator.comparing(x -> x.stream().filter(y -> !ActionSchema.NO_OP.equals(y.getName())).count()))
+                .collect(Collectors.toList());
+        return result;
     }
 
     // Helper methods for combinations and permutations.
@@ -251,21 +236,4 @@ public List<List<ActionSchema>> combineExtraList(List<List<ActionSchema>> combin
         }
         return result;
     }
-
-    public List<List<ActionSchema>> generateCombinations(List<List<ActionSchema>> actionLists) {
-        List<List<ActionSchema>> result = new ArrayList<>();
-        if (actionLists.size() == 1) {
-            result.add(actionLists.get(0));
-            return result;
-        }
-        if (actionLists.size() == 2) {
-            return combineTwoLists(actionLists.get(0), actionLists.get(1));
-        } else {
-            result = combineTwoLists(actionLists.get(0), actionLists.get(1));
-            for (int i = 2; i < actionLists.size(); i++) {
-                result = combineExtraList(result, actionLists.get(i));
-            }
-            return result;
-        }
-    }
 }

aima-core/src/main/java/aima/core/logic/planning/Level.java

@@ -8,27 +8,24 @@
  * The data structure for calculating and holding the levels of a planning graph.
  *
  * @author samagra
+ * @author Ruediger Lunde
  */
 public class Level {
     List<Object> levelObjects;
-    HashMap<Object, List<Object>> mutexLinks;//can be planned alternatively
+    HashMap<Object, List<Object>> mutexLinks; //can be planned alternatively
     HashMap<Object, List<Object>> nextLinks;
     HashMap<Object, List<Object>> prevLinks;
-    Problem problem;
     Level prevLevel;
 
     public Level(Level prevLevel, Problem problem) {
-        this.prevLevel = prevLevel;
-        this.problem = problem;
+        levelObjects = new ArrayList<>();
+        prevLinks = new HashMap<>();
         if (prevLevel != null) {
+            this.prevLevel = prevLevel;
             HashMap<Object, List<Object>> linksFromPreviousLevel = prevLevel.getNextLinks();
-            this.problem = problem;
-            levelObjects = new ArrayList<>();
-            prevLinks = new HashMap<>();
             for (Object node : linksFromPreviousLevel.keySet()) {
                 List<Object> thisLevelObjects = linksFromPreviousLevel.get(node);
-                for (Object nextNode :
-                        thisLevelObjects) {
+                for (Object nextNode : thisLevelObjects) {
                     if (levelObjects.contains(nextNode)) {
                         List<Object> tempPrevLink = prevLinks.get(nextNode);
                         tempPrevLink.add(node);
@@ -37,38 +34,27 @@ public Level(Level prevLevel, Problem problem) {
                         levelObjects.add(nextNode);
                         prevLinks.put(nextNode, new ArrayList<>(Collections.singletonList(node)));
                     }
-
                 }
-
             }
-            addNoPrecondActions();
-            calculateNextLinks();
-            calculateMutexLinks(prevLevel);
         } else {
-            levelObjects = new ArrayList<>();
-            prevLinks = new HashMap<>();
             levelObjects.addAll(problem.getInitialState().getFluents());
-            for (Object obj : levelObjects) {
+            for (Object obj : levelObjects)
                 prevLinks.put(obj, new ArrayList<>());
-            }
-            addNoPrecondActions();
-            calculateNextLinks();
-            calculateMutexLinks(null);
         }
+        addNoPrecondActions(problem);
+        calculateNextLinks(problem);
+        calculateMutexLinks(prevLevel);
         addPersistenceActions();
     }
 
+    // for testing only...
     public Level(Level prevLevel, Problem problem, String extraLiterals) {
         this(prevLevel, problem);
-        this.addExtraLiterals(extraLiterals);
-    }
-
-    public void addExtraLiterals(String s) {
-        for (Literal literal : Utils.parse(s)) {
+        for (Literal literal : Utils.parse(extraLiterals)) {
             if(!levelObjects.contains(literal))
                 levelObjects.add(literal);
         }
-        calculateNextLinks();
+        calculateNextLinks(problem);
         calculateMutexLinks(getPrevLevel());
         addPersistenceActions();
     }
@@ -89,35 +75,34 @@ public HashMap<Object, List<Object>> getPrevLinks() {
         return prevLinks;
     }
 
-    public Problem getProblem() {
-        return problem;
-    }
-
-    private void addPersistenceActions() {
-       if(getLevelObjects().get(0) instanceof Literal) {
-           for (Object literal : getLevelObjects()) {
-               ActionSchema action = new ActionSchema(ActionSchema.NO_OP, null,
-                       Collections.singletonList((Literal) literal),
-                       Collections.singletonList((Literal) literal));
-               addToHashMap(literal, action, nextLinks);
-           }
-       }
+    public Level getPrevLevel() {
+        return prevLevel;
     }
 
-    public void addNoPrecondActions(){
-        if(getLevelObjects().get(0) instanceof ActionSchema) {
+    private void addNoPrecondActions(Problem problem) {
+        if(levelObjects.get(0) instanceof ActionSchema) {
             for (ActionSchema action : problem.getPropositionalisedActions()) {
                 if (action.getPrecondition().size()==0)
                     levelObjects.add(action);
             }
         }
     }
 
-
+    private void addPersistenceActions() {
+        if(levelObjects.get(0) instanceof Literal) {
+            for (Object literal : getLevelObjects()) {
+                ActionSchema action = new ActionSchema(ActionSchema.NO_OP, null,
+                        Collections.singletonList((Literal) literal),
+                        Collections.singletonList((Literal) literal));
+                addToHashMap(literal, action, nextLinks);
+            }
+        }
+    }
 
     private void calculateMutexLinks(Level prevLevel) {
         mutexLinks = new HashMap<>();
-        if(prevLevel == null) return;
+        if (prevLevel == null)
+            return;
         if (levelObjects.get(0) instanceof Literal) {
             Literal firstLiteral, secondLiteral;
             List<Object> possibleActionsFirst, possibleActionsSecond;
@@ -221,18 +206,7 @@ private boolean checkInterference(List<Literal> firstActionPreconditions, List<L
         return checkMutex;
     }
 
-    private void addToHashMap(Object firstObject, Object secondObject, HashMap<Object, List<Object>> map) {
-        List<Object> tempList;
-        if (map.containsKey(firstObject)) {
-            tempList = map.get(firstObject);
-            tempList.add(secondObject);
-            map.put(firstObject, tempList);
-        } else {
-            map.put(firstObject, new ArrayList<>(Collections.singletonList(secondObject)));
-        }
-    }
-
-    private void calculateNextLinks() {
+    private void calculateNextLinks(Problem problem) {
         nextLinks = new HashMap<>();
         if (levelObjects.get(0) instanceof Literal) {
             for (ActionSchema action : problem.getPropositionalisedActions()) {
@@ -252,20 +226,20 @@ private void calculateNextLinks() {
             }
         } else if (levelObjects.get(0) instanceof ActionSchema) {
             for (Object action : levelObjects) {
-                Object[] effects =  ((ActionSchema) action).getEffects().toArray();
-                nextLinks.put(action, new ArrayList<>(Arrays.asList(effects)));
+                List<Literal> effects = ((ActionSchema) action).getEffects();
+                nextLinks.put(action, new ArrayList<>(new ArrayList<Object>(effects)));
             }
         }
-
     }
 
-    public Level getPrevLevel() {
-        return prevLevel;
+    private void addToHashMap(Object firstObject, Object secondObject, HashMap<Object, List<Object>> map) {
+        List<Object> list = map.computeIfAbsent(firstObject, k -> new ArrayList<>());
+        list.add(secondObject);
     }
 
     @Override
     public boolean equals(Object obj) {
-        if (!(obj instanceof Level))
+        if (obj == null || getClass() != obj.getClass())
             return false;
         return this.levelObjects.containsAll(((Level) obj).levelObjects)
                 && ((Level) obj).levelObjects.containsAll(this.levelObjects)

aima-core/src/test/java/aima/test/core/unit/logic/planning/GraphTest.java

@@ -10,6 +10,7 @@
 
 /**
  * @author samagra
+ * @author Ruediger Lunde
  */
 public class GraphTest {
     Problem problem;
@@ -18,17 +19,12 @@ public class GraphTest {
     @Before
     public void setup() {
         problem = PlanningProblemFactory.spareTireProblem();
-        firstLevel = new Level(null, problem, "At(Spare,Trunk)^At(Flat,Axle) ^~At(Spare,Axle)^~At(Flat,Ground)^~At(Spare,Ground)");
-        secondLevel = new Level(firstLevel, problem);
-        thirdLevel = new Level(thirdLevel, problem);
     }
 
     @Test
-    public void addLevelTest() {
-        Graph graph = new Graph(problem, firstLevel);
-        graph.addLevel();
-        Assert.assertEquals(2, graph.getLevels().size());
-        graph.addLevel();
+    public void expandTest() {
+        Graph graph = new Graph(problem);
+        graph.expand(problem);
         Assert.assertEquals(3, graph.getLevels().size());
     }
 }