Issue1072

src/search/landmarks/landmark_graph.cc

@@ -57,6 +57,11 @@ LandmarkNode &LandmarkGraph::get_disjunctive_landmark(const FactPair &fact) cons
     return *(disjunctive_landmarks_to_nodes.find(fact)->second);
 }
 
+const vector<LandmarkNode *> &LandmarkGraph::get_conjunctive_landmarks(
+    const FactPair &fact) const {
+    assert(contains_conjunctive_landmark(fact));
+    return conjunctive_landmarks_to_nodes.find(fact)->second;
+}
 
 bool LandmarkGraph::contains_simple_landmark(const FactPair &lm) const {
     return simple_landmarks_to_nodes.count(lm) != 0;
@@ -66,6 +71,10 @@ bool LandmarkGraph::contains_disjunctive_landmark(const FactPair &lm) const {
     return disjunctive_landmarks_to_nodes.count(lm) != 0;
 }
 
+bool LandmarkGraph::contains_conjunctive_landmark(const FactPair &lm) const {
+    return conjunctive_landmarks_to_nodes.count(lm) != 0;
+}
+
 bool LandmarkGraph::contains_overlapping_disjunctive_landmark(
     const set<FactPair> &lm) const {
     // Test whether ONE of the facts is present in some disjunctive landmark.
@@ -118,6 +127,15 @@ LandmarkNode &LandmarkGraph::add_landmark(Landmark &&landmark) {
         }
         ++num_disjunctive_landmarks;
     } else if (lm.conjunctive) {
+        for (const FactPair &lm_fact : lm.facts) {
+            auto it = conjunctive_landmarks_to_nodes.find(lm_fact);
+            if (it == conjunctive_landmarks_to_nodes.end()) {
+                conjunctive_landmarks_to_nodes.emplace(
+                    lm_fact, vector<LandmarkNode *>{new_node_p});
+            } else {
+                (it->second).push_back(new_node_p);
+            }
+        }
         ++num_conjunctive_landmarks;
     } else {
         simple_landmarks_to_nodes.emplace(lm.facts.front(), new_node_p);
@@ -144,6 +162,14 @@ void LandmarkGraph::remove_node_occurrences(LandmarkNode *node) {
         }
     } else if (landmark.conjunctive) {
         --num_conjunctive_landmarks;
+        for (const FactPair &lm_fact : landmark.facts) {
+            vector<LandmarkNode *> *conjunctive_landmarks_vector =
+                &(conjunctive_landmarks_to_nodes.find(lm_fact)->second);
+            auto it = find(conjunctive_landmarks_vector->begin(),
+                           conjunctive_landmarks_vector->end(), node);
+            assert(it != conjunctive_landmarks_vector->end());
+            conjunctive_landmarks_vector->erase(it);
+        }
     } else {
         simple_landmarks_to_nodes.erase(landmark.facts[0]);
     }

src/search/landmarks/landmark_graph.h

@@ -77,6 +77,7 @@ class LandmarkGraph {
 
     utils::HashMap<FactPair, LandmarkNode *> simple_landmarks_to_nodes;
     utils::HashMap<FactPair, LandmarkNode *> disjunctive_landmarks_to_nodes;
+    utils::HashMap<FactPair, std::vector<LandmarkNode *>> conjunctive_landmarks_to_nodes;
     Nodes nodes;
 
     void remove_node_occurrences(LandmarkNode *node);
@@ -118,13 +119,18 @@ class LandmarkGraph {
     /* This is needed only by landmark graph factories and will disappear
        when moving landmark graph creation there. */
     LandmarkNode &get_disjunctive_landmark(const FactPair &fact) const;
+    // TODO: comment?
+    const std::vector<LandmarkNode *> &get_conjunctive_landmarks(
+        const FactPair &fact) const;
 
     /* This is needed only by landmark graph factories and will disappear
        when moving landmark graph creation there.  It is not needed by
        HMLandmarkFactory*/
     bool contains_simple_landmark(const FactPair &lm) const;
     /* Only used internally. */
     bool contains_disjunctive_landmark(const FactPair &lm) const;
+    // TODO: comment?
+    bool contains_conjunctive_landmark(const FactPair &lm) const;
     /* This is needed only by landmark graph factories and will disappear
        when moving landmark graph creation there.  It is not needed by
        HMLandmarkFactory*/

src/search/landmarks/landmark_heuristic.cc

@@ -118,12 +118,7 @@ void LandmarkHeuristic::compute_landmark_graph(
 
 void LandmarkHeuristic::generate_preferred_operators(
     const State &state, ConstBitsetView &future) {
-    /*
-      Find operators that achieve future landmarks.
-      TODO: Conjunctive landmarks are ignored in *lm_graph->get_node(...)*, so
-       they are ignored when computing preferred operators. We consider this
-       a bug and want to fix it in issue1072.
-    */
+    // Find operators that achieve future landmarks.
     assert(successor_generator);
     vector<OperatorID> applicable_operators;
     successor_generator->generate_applicable_ops(state, applicable_operators);
@@ -132,13 +127,27 @@ void LandmarkHeuristic::generate_preferred_operators(
         OperatorProxy op = task_proxy.get_operators()[op_id];
         EffectsProxy effects = op.get_effects();
         for (EffectProxy effect : effects) {
-            if (!does_fire(effect, state))
+            if (!does_fire(effect, state)) {
+                continue;
+            }
+            FactPair fact = effect.get_fact().get_pair();
+            if (state[fact.var].get_value() == fact.value) {
                 continue;
-            FactProxy fact_proxy = effect.get_fact();
-            LandmarkNode *lm_node = lm_graph->get_node(fact_proxy.get_pair());
+            }
+            LandmarkNode *lm_node = lm_graph->get_node(fact);
             if (lm_node && future.test(lm_node->get_id())) {
                 set_preferred(op);
             }
+            if (lm_graph->contains_conjunctive_landmark(fact)) {
+                vector<LandmarkNode *> conjunctive_landmarks =
+                    lm_graph->get_conjunctive_landmarks(fact);
+                for (auto conj_lm : conjunctive_landmarks) {
+                    if (future.test(conj_lm->get_id())) {
+                        set_preferred(op);
+                        break;
+                    }
+                }
+            }
         }
     }
 }