Issue1036

src/search/landmarks/landmark_cost_assignment.cc

@@ -19,18 +19,17 @@ using namespace std;
 namespace landmarks {
 LandmarkCostAssignment::LandmarkCostAssignment(
     const vector<int> &operator_costs, const LandmarkGraph &graph)
-    : empty(), lm_graph(graph), operator_costs(operator_costs) {
+    : lm_graph(graph), operator_costs(operator_costs) {
 }
 
 const set<int> &LandmarkCostAssignment::get_achievers(
-    int lmn_status, const Landmark &landmark) const {
+    const Landmark &landmark, bool past) const {
     // Return relevant achievers of the landmark according to its status.
-    if (lmn_status == lm_not_reached)
-        return landmark.first_achievers;
-    else if (lmn_status == lm_needed_again)
+    if (past) {
         return landmark.possible_achievers;
-    else
-        return empty;
+    } else {
+        return landmark.first_achievers;
+    }
 }
 
 
@@ -44,23 +43,27 @@ LandmarkUniformSharedCostAssignment::LandmarkUniformSharedCostAssignment(
 
 
 double LandmarkUniformSharedCostAssignment::cost_sharing_h_value(
-    const LandmarkStatusManager &lm_status_manager) {
+    const LandmarkStatusManager &lm_status_manager,
+    const State &ancestor_state) {
     vector<int> achieved_lms_by_op(operator_costs.size(), 0);
     vector<bool> action_landmarks(operator_costs.size(), false);
 
     const LandmarkGraph::Nodes &nodes = lm_graph.get_nodes();
+    ConstBitsetView past =
+        lm_status_manager.get_past_landmarks(ancestor_state);
+    ConstBitsetView future =
+        lm_status_manager.get_future_landmarks(ancestor_state);
 
     double h = 0;
 
     /* First pass:
        compute which op achieves how many landmarks. Along the way,
        mark action landmarks and add their cost to h. */
     for (auto &node : nodes) {
-        int lmn_status =
-            lm_status_manager.get_landmark_status(node->get_id());
-        if (lmn_status != lm_reached) {
+        int id = node->get_id();
+        if (future.test(id)) {
             const set<int> &achievers =
-                get_achievers(lmn_status, node->get_landmark());
+                get_achievers(node->get_landmark(), past.test(id));
             if (achievers.empty())
                 return numeric_limits<double>::max();
             if (use_action_landmarks && achievers.size() == 1) {
@@ -90,11 +93,10 @@ double LandmarkUniformSharedCostAssignment::cost_sharing_h_value(
        an action landmark; decrease the counters accordingly
        so that no unnecessary cost is assigned to these landmarks. */
     for (auto &node : nodes) {
-        int lmn_status =
-            lm_status_manager.get_landmark_status(node->get_id());
-        if (lmn_status != lm_reached) {
+        int id = node->get_id();
+        if (future.test(id)) {
             const set<int> &achievers =
-                get_achievers(lmn_status, node->get_landmark());
+                get_achievers(node->get_landmark(), past.test(id));
             bool covered_by_action_lm = false;
             for (int op_id : achievers) {
                 assert(utils::in_bounds(op_id, action_landmarks));
@@ -118,10 +120,10 @@ double LandmarkUniformSharedCostAssignment::cost_sharing_h_value(
        count shared costs for the remaining landmarks. */
     for (const LandmarkNode *node : relevant_lms) {
         // TODO: Iterate over Landmarks instead of LandmarkNodes
-        int lmn_status =
-            lm_status_manager.get_landmark_status(node->get_id());
+        int id = node->get_id();
+        assert(future.test(id));
         const set<int> &achievers =
-            get_achievers(lmn_status, node->get_landmark());
+            get_achievers(node->get_landmark(), past.test(id));
         double min_cost = numeric_limits<double>::max();
         for (int op_id : achievers) {
             assert(utils::in_bounds(op_id, achieved_lms_by_op));
@@ -174,10 +176,16 @@ lp::LinearProgram LandmarkEfficientOptimalSharedCostAssignment::build_initial_lp
 }
 
 double LandmarkEfficientOptimalSharedCostAssignment::cost_sharing_h_value(
-    const LandmarkStatusManager &lm_status_manager) {
+    const LandmarkStatusManager &lm_status_manager,
+    const State &ancestor_state) {
     /* TODO: We could also do the same thing with action landmarks we
              do in the uniform cost partitioning case. */
 
+
+    ConstBitsetView past =
+        lm_status_manager.get_past_landmarks(ancestor_state);
+    ConstBitsetView future =
+        lm_status_manager.get_future_landmarks(ancestor_state);
     /*
       Set up LP variable bounds for the landmarks.
       The range of cost(lm_1) is {0} if the landmark is already
@@ -186,10 +194,10 @@ double LandmarkEfficientOptimalSharedCostAssignment::cost_sharing_h_value(
     */
     int num_cols = lm_graph.get_num_landmarks();
     for (int lm_id = 0; lm_id < num_cols; ++lm_id) {
-        if (lm_status_manager.get_landmark_status(lm_id) == lm_reached) {
-            lp.get_variables()[lm_id].upper_bound = 0;
-        } else {
+        if (future.test(lm_id)) {
             lp.get_variables()[lm_id].upper_bound = lp_solver.get_infinity();
+        } else {
+            lp.get_variables()[lm_id].upper_bound = 0;
         }
     }
 
@@ -207,10 +215,9 @@ double LandmarkEfficientOptimalSharedCostAssignment::cost_sharing_h_value(
     }
     for (int lm_id = 0; lm_id < num_cols; ++lm_id) {
         const Landmark &landmark = lm_graph.get_node(lm_id)->get_landmark();
-        int lm_status = lm_status_manager.get_landmark_status(lm_id);
-        if (lm_status != lm_reached) {
+        if (future.test(lm_id)) {
             const set<int> &achievers =
-                get_achievers(lm_status, landmark);
+                get_achievers(landmark, past.test(lm_id));
             if (achievers.empty())
                 return numeric_limits<double>::max();
             for (int op_id : achievers) {

src/search/landmarks/landmark_cost_assignment.h

@@ -1,6 +1,8 @@
 #ifndef LANDMARKS_LANDMARK_COST_ASSIGNMENT_H
 #define LANDMARKS_LANDMARK_COST_ASSIGNMENT_H
 
+#include "../task_proxy.h"
+
 #include "../lp/lp_solver.h"
 
 #include <set>
@@ -15,20 +17,20 @@ class LandmarkNode;
 class LandmarkStatusManager;
 
 class LandmarkCostAssignment {
-    const std::set<int> empty;
 protected:
     const LandmarkGraph &lm_graph;
     const std::vector<int> operator_costs;
 
-    const std::set<int> &get_achievers(int lmn_status,
-                                       const Landmark &landmark) const;
+    const std::set<int> &get_achievers(
+        const Landmark &landmark, bool past) const;
 public:
     LandmarkCostAssignment(const std::vector<int> &operator_costs,
                            const LandmarkGraph &graph);
     virtual ~LandmarkCostAssignment() = default;
 
     virtual double cost_sharing_h_value(
-        const LandmarkStatusManager &lm_status_manager) = 0;
+        const LandmarkStatusManager &lm_status_manager,
+        const State &ancestor_state) = 0;
 };
 
 class LandmarkUniformSharedCostAssignment : public LandmarkCostAssignment {
@@ -39,7 +41,8 @@ class LandmarkUniformSharedCostAssignment : public LandmarkCostAssignment {
                                         bool use_action_landmarks);
 
     virtual double cost_sharing_h_value(
-        const LandmarkStatusManager &lm_status_manager) override;
+        const LandmarkStatusManager &lm_status_manager,
+        const State &ancestor_state) override;
 };
 
 class LandmarkEfficientOptimalSharedCostAssignment : public LandmarkCostAssignment {
@@ -62,7 +65,8 @@ class LandmarkEfficientOptimalSharedCostAssignment : public LandmarkCostAssignme
         lp::LPSolverType solver_type);
 
     virtual double cost_sharing_h_value(
-        const LandmarkStatusManager &lm_status_manager) override;
+        const LandmarkStatusManager &lm_status_manager,
+        const State &ancestor_state) override;
 };
 }
 

src/search/landmarks/landmark_cost_partitioning_heuristic.cc

@@ -30,11 +30,6 @@ void LandmarkCostPartitioningHeuristic::check_unsupported_features(
     const plugins::Options &opts) {
     shared_ptr<LandmarkFactory> lm_graph_factory =
         opts.get<shared_ptr<LandmarkFactory>>("lm_factory");
-    if (lm_graph_factory->computes_reasonable_orders()) {
-        cerr << "Reasonable orderings should not be used for "
-             << "admissible heuristics." << endl;
-        utils::exit_with(utils::ExitCode::SEARCH_INPUT_ERROR);
-    }
 
     if (task_properties::has_axioms(task_proxy)) {
         cerr << "Cost partitioning does not support axioms." << endl;
@@ -65,10 +60,11 @@ void LandmarkCostPartitioningHeuristic::set_cost_assignment(
 }
 
 int LandmarkCostPartitioningHeuristic::get_heuristic_value(
-    const State & /*state*/) {
+    const State &ancestor_state) {
     double epsilon = 0.01;
 
-    double h_val = lm_cost_assignment->cost_sharing_h_value(*lm_status_manager);
+    double h_val = lm_cost_assignment->cost_sharing_h_value(
+        *lm_status_manager, ancestor_state);
     if (h_val == numeric_limits<double>::max()) {
         return DEAD_END;
     } else {

src/search/landmarks/landmark_cost_partitioning_heuristic.h

@@ -12,7 +12,7 @@ class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic {
     void check_unsupported_features(const plugins::Options &opts);
     void set_cost_assignment(const plugins::Options &opts);
 
-    int get_heuristic_value(const State &state) override;
+    int get_heuristic_value(const State &ancestor_state) override;
 public:
     explicit LandmarkCostPartitioningHeuristic(const plugins::Options &opts);
 

src/search/landmarks/landmark_heuristic.cc

@@ -8,12 +8,13 @@
 #include "../task_utils/successor_generator.h"
 #include "../tasks/cost_adapted_task.h"
 #include "../tasks/root_task.h"
+#include "../utils/markup.h"
 
 using namespace std;
 
 namespace landmarks {
 static bool landmark_is_interesting(
-    const State &state, const BitsetView &reached,
+    const State &state, ConstBitsetView &past,
     const landmarks::LandmarkNode &lm_node, bool all_lms_reached) {
     /*
       We consider a landmark interesting in two (exclusive) cases:
@@ -26,10 +27,10 @@ static bool landmark_is_interesting(
         const Landmark &landmark = lm_node.get_landmark();
         return landmark.is_true_in_goal && !landmark.is_true_in_state(state);
     } else {
-        return !reached.test(lm_node.get_id()) &&
+        return !past.test(lm_node.get_id()) &&
                all_of(lm_node.parents.begin(), lm_node.parents.end(),
                       [&](const pair<LandmarkNode *, EdgeType> parent) {
-                          return reached.test(parent.first->get_id());
+                          return past.test(parent.first->get_id());
                       });
     }
 }
@@ -56,8 +57,9 @@ void LandmarkHeuristic::initialize(const plugins::Options &opts) {
     }
 
     compute_landmark_graph(opts);
-    lm_status_manager =
-        utils::make_unique_ptr<LandmarkStatusManager>(*lm_graph);
+    lm_status_manager = utils::make_unique_ptr<LandmarkStatusManager>(
+        *lm_graph, opts.get<bool>("prog_goal"),
+        opts.get<bool>("prog_gn"), opts.get<bool>("prog_r"));
 
     if (use_preferred_operators) {
         /* Ideally, we should reuse the successor generator of the main
@@ -93,7 +95,7 @@ void LandmarkHeuristic::compute_landmark_graph(const plugins::Options &opts) {
 }
 
 void LandmarkHeuristic::generate_preferred_operators(
-    const State &state, const BitsetView &reached) {
+    const State &state, ConstBitsetView &past) {
     /*
       Find operators that achieve landmark leaves. If a simple landmark can be
       achieved, prefer only operators that achieve simple landmarks. Otherwise,
@@ -110,10 +112,10 @@ void LandmarkHeuristic::generate_preferred_operators(
     vector<OperatorID> preferred_operators_simple;
     vector<OperatorID> preferred_operators_disjunctive;
 
-    bool all_landmarks_reached = true;
-    for (int i = 0; i < reached.size(); ++i) {
-        if (!reached.test(i)) {
-            all_landmarks_reached = false;
+    bool all_landmarks_past = true;
+    for (int i = 0; i < past.size(); ++i) {
+        if (!past.test(i)) {
+            all_landmarks_past = false;
             break;
         }
     }
@@ -127,7 +129,7 @@ void LandmarkHeuristic::generate_preferred_operators(
             FactProxy fact_proxy = effect.get_fact();
             LandmarkNode *lm_node = lm_graph->get_node(fact_proxy.get_pair());
             if (lm_node && landmark_is_interesting(
-                    state, reached, *lm_node, all_landmarks_reached)) {
+                    state, past, *lm_node, all_landmarks_past)) {
                 if (lm_node->get_landmark().disjunctive) {
                     preferred_operators_disjunctive.push_back(op_id);
                 } else {
@@ -150,34 +152,42 @@ void LandmarkHeuristic::generate_preferred_operators(
 }
 
 int LandmarkHeuristic::compute_heuristic(const State &ancestor_state) {
-    State state = convert_ancestor_state(ancestor_state);
-    lm_status_manager->update_lm_status(ancestor_state);
-    int h = get_heuristic_value(state);
-
+    int h = get_heuristic_value(ancestor_state);
     if (use_preferred_operators) {
-        BitsetView reached_lms =
-            lm_status_manager->get_reached_landmarks(ancestor_state);
-        generate_preferred_operators(state, reached_lms);
+        ConstBitsetView past = lm_status_manager->get_past_landmarks(ancestor_state);
+        State state = convert_ancestor_state(ancestor_state);
+        generate_preferred_operators(state, past);
     }
-
     return h;
 }
 
 void LandmarkHeuristic::notify_initial_state(const State &initial_state) {
-    lm_status_manager->process_initial_state(initial_state, log);
+    lm_status_manager->progress_initial_state(initial_state);
 }
 
 void LandmarkHeuristic::notify_state_transition(
     const State &parent_state, OperatorID op_id, const State &state) {
-    lm_status_manager->process_state_transition(parent_state, op_id, state);
+    lm_status_manager->progress(parent_state, op_id, state);
     if (cache_evaluator_values) {
-        /* TODO:  It may be more efficient to check that the reached landmark
+        /* TODO:  It may be more efficient to check that the past landmark
             set has actually changed and only then mark the h value as dirty. */
         heuristic_cache[state].dirty = true;
     }
 }
 
 void LandmarkHeuristic::add_options_to_feature(plugins::Feature &feature) {
+    feature.document_synopsis(
+        "Landmark progression is implemented according to the following paper:"
+        + utils::format_conference_reference(
+            {"Clemens Büchner", "Thomas Keller", "Salomé Eriksson", "Malte Helmert"},
+            "Landmarks Progression in Heuristic Search",
+            "https://ai.dmi.unibas.ch/papers/buechner-et-al-icaps2023.pdf",
+            "Proceedings of the Thirty-Third International Conference on "
+            "Automated Planning and Scheduling (ICAPS 2023)",
+            "70-79",
+            "AAAI Press",
+            "2023"));
+
     feature.add_option<shared_ptr<LandmarkFactory>>(
         "lm_factory",
         "the set of landmarks to use for this heuristic. "
@@ -187,6 +197,14 @@ void LandmarkHeuristic::add_options_to_feature(plugins::Feature &feature) {
         "pref",
         "enable preferred operators (see note below)",
         "false");
+    /* TODO: Do we really want these options or should we just allways progress
+        everything we can? */
+    feature.add_option<bool>(
+        "prog_goal", "Use goal progression.", "true");
+    feature.add_option<bool>(
+        "prog_gn", "Use greedy-necessary ordering progression.", "true");
+    feature.add_option<bool>(
+        "prog_r", "Use reasonable ordering progression.", "true");
     Heuristic::add_options_to_feature(feature);
 
     feature.document_property("preferred operators",

src/search/landmarks/landmark_heuristic.h

@@ -3,7 +3,7 @@
 
 # include "../heuristic.h"
 
-class BitsetView;
+class ConstBitsetView;
 
 namespace successor_generator {
 class SuccessorGenerator;
@@ -26,22 +26,10 @@ class LandmarkHeuristic : public Heuristic {
     void initialize(const plugins::Options &opts);
     void compute_landmark_graph(const plugins::Options &opts);
 
-    /*
-      Unlike most landmark-related code, this function takes the
-      task-transformation of the state, not the original one (i.e., not
-      *ancestor_state*). This is because updating the landmark status manager
-      happens in *compute_heuristic(...)* before *get_heuristic_value(...)*
-      is called. Here, we only compute a heuristic value based on the
-      information in the landmark status manager, which does not require the
-      state at this point. The only reason we need this argument is to guarantee
-      goal-awareness of the LM-count heuristic which does not hold under the
-      current function used for progressing the landmark statuses. Checking
-      whether a state is a goal state requires the task-transformed state.
-    */
-    virtual int get_heuristic_value(const State &state) = 0;
+    virtual int get_heuristic_value(const State &ancestor_state) = 0;
 
     void generate_preferred_operators(
-        const State &state, const BitsetView &reached);
+        const State &state, ConstBitsetView &past);
     virtual int compute_heuristic(const State &ancestor_state) override;
 public:
     explicit LandmarkHeuristic(const plugins::Options &opts);