From 1070582ec143c1eb36707213c10c6be1cf602372 Mon Sep 17 00:00:00 2001 From: ClemensBuechner Date: Wed, 2 Aug 2023 13:52:09 +0200 Subject: [PATCH] [issue1036] Refactor landmark progression to make it sound. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We implement valid landmark progression according to Büchner et al. (ICAPS 2023). With this, the previously incomplete LAMA becomes complete and also we can now use reasonable landmark ordrings for optimal planning. The implemented progression functions can be turned on or off by the following (boolean) command line options (default on) of the landmark_sum and landmark_cost_partitioning heuristic: prog_goal, prog_gn, and prog_r (for goal landmarks, greedy-necessary orderings, and reasonable orderings, respectively). The changes require more memory per state, but in general the performance of these heuristics is merely unaffected. --- .../landmarks/landmark_cost_assignment.cc | 59 ++-- .../landmarks/landmark_cost_assignment.h | 16 +- .../landmark_cost_partitioning_heuristic.cc | 10 +- .../landmark_cost_partitioning_heuristic.h | 2 +- src/search/landmarks/landmark_heuristic.cc | 62 ++-- src/search/landmarks/landmark_heuristic.h | 18 +- .../landmarks/landmark_status_manager.cc | 301 ++++++++++-------- .../landmarks/landmark_status_manager.h | 69 ++-- .../landmarks/landmark_sum_heuristic.cc | 36 +-- src/search/landmarks/landmark_sum_heuristic.h | 2 +- src/search/per_state_array.h | 56 +++- src/search/per_state_bitset.cc | 19 +- src/search/per_state_bitset.h | 25 +- 13 files changed, 394 insertions(+), 281 deletions(-) diff --git a/src/search/landmarks/landmark_cost_assignment.cc b/src/search/landmarks/landmark_cost_assignment.cc index 3889ad23e9..822374cfd8 100644 --- a/src/search/landmarks/landmark_cost_assignment.cc +++ b/src/search/landmarks/landmark_cost_assignment.cc @@ -19,18 +19,17 @@ using namespace std; namespace landmarks { LandmarkCostAssignment::LandmarkCostAssignment( const vector &operator_costs, const LandmarkGraph &graph) - : empty(), lm_graph(graph), operator_costs(operator_costs) { + : lm_graph(graph), operator_costs(operator_costs) { } const set &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,11 +43,16 @@ LandmarkUniformSharedCostAssignment::LandmarkUniformSharedCostAssignment( double LandmarkUniformSharedCostAssignment::cost_sharing_h_value( - const LandmarkStatusManager &lm_status_manager) { + const LandmarkStatusManager &lm_status_manager, + const State &ancestor_state) { vector achieved_lms_by_op(operator_costs.size(), 0); vector 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; @@ -56,11 +60,10 @@ double LandmarkUniformSharedCostAssignment::cost_sharing_h_value( 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 &achievers = - get_achievers(lmn_status, node->get_landmark()); + get_achievers(node->get_landmark(), past.test(id)); if (achievers.empty()) return numeric_limits::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 &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 &achievers = - get_achievers(lmn_status, node->get_landmark()); + get_achievers(node->get_landmark(), past.test(id)); double min_cost = numeric_limits::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 &achievers = - get_achievers(lm_status, landmark); + get_achievers(landmark, past.test(lm_id)); if (achievers.empty()) return numeric_limits::max(); for (int op_id : achievers) { diff --git a/src/search/landmarks/landmark_cost_assignment.h b/src/search/landmarks/landmark_cost_assignment.h index fd4477d9cb..bab5651b3b 100644 --- a/src/search/landmarks/landmark_cost_assignment.h +++ b/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 @@ -15,20 +17,20 @@ class LandmarkNode; class LandmarkStatusManager; class LandmarkCostAssignment { - const std::set empty; protected: const LandmarkGraph &lm_graph; const std::vector operator_costs; - const std::set &get_achievers(int lmn_status, - const Landmark &landmark) const; + const std::set &get_achievers( + const Landmark &landmark, bool past) const; public: LandmarkCostAssignment(const std::vector &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; }; } diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc index fe571c254d..f11f0d6767 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc @@ -30,11 +30,6 @@ void LandmarkCostPartitioningHeuristic::check_unsupported_features( const plugins::Options &opts) { shared_ptr lm_graph_factory = opts.get>("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::max()) { return DEAD_END; } else { diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.h b/src/search/landmarks/landmark_cost_partitioning_heuristic.h index 9674254ee5..33ce536c58 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.h +++ b/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); diff --git a/src/search/landmarks/landmark_heuristic.cc b/src/search/landmarks/landmark_heuristic.cc index 3bac31bb69..7cb82db0ca 100644 --- a/src/search/landmarks/landmark_heuristic.cc +++ b/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 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(*lm_graph); + lm_status_manager = utils::make_unique_ptr( + *lm_graph, opts.get("prog_goal"), + opts.get("prog_gn"), opts.get("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 preferred_operators_simple; vector 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>( "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( + "prog_goal", "Use goal progression.", "true"); + feature.add_option( + "prog_gn", "Use greedy-necessary ordering progression.", "true"); + feature.add_option( + "prog_r", "Use reasonable ordering progression.", "true"); Heuristic::add_options_to_feature(feature); feature.document_property("preferred operators", diff --git a/src/search/landmarks/landmark_heuristic.h b/src/search/landmarks/landmark_heuristic.h index 4a644849fd..5387d464e7 100644 --- a/src/search/landmarks/landmark_heuristic.h +++ b/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); diff --git a/src/search/landmarks/landmark_status_manager.cc b/src/search/landmarks/landmark_status_manager.cc index 70c5da1fa7..c76af33a41 100644 --- a/src/search/landmarks/landmark_status_manager.cc +++ b/src/search/landmarks/landmark_status_manager.cc @@ -1,171 +1,222 @@ #include "landmark_status_manager.h" +#include "util.h" #include "landmark.h" -#include "../utils/logging.h" - using namespace std; namespace landmarks { -/* - By default we mark all landmarks as reached, since we do an intersection when - computing new landmark information. -*/ -LandmarkStatusManager::LandmarkStatusManager(LandmarkGraph &graph) +static vector get_goal_landmarks(const LandmarkGraph &graph) { + vector goals; + for (auto &node : graph.get_nodes()) { + if (node->get_landmark().is_true_in_goal) { + goals.push_back(node.get()); + } + } + return goals; +} + +static vector>> get_greedy_necessary_children( + const LandmarkGraph &graph) { + vector>> orderings; + for (auto &node : graph.get_nodes()) { + vector greedy_necessary_children; + for (auto &child : node->children) { + if (child.second == EdgeType::GREEDY_NECESSARY) { + greedy_necessary_children.push_back(child.first); + } + } + if (!greedy_necessary_children.empty()) { + orderings.emplace_back(node.get(), move(greedy_necessary_children)); + } + } + return orderings; +} + +static vector>> get_reasonable_parents( + const LandmarkGraph &graph) { + vector>> orderings; + for (auto &node : graph.get_nodes()) { + vector reasonable_parents; + for (auto &parent : node->parents) { + if (parent.second == EdgeType::REASONABLE) { + reasonable_parents.push_back(parent.first); + } + } + if (!reasonable_parents.empty()) { + orderings.emplace_back(node.get(), move(reasonable_parents)); + } + } + return orderings; +} + +LandmarkStatusManager::LandmarkStatusManager( + LandmarkGraph &graph, + bool progress_goals, + bool progress_greedy_necessary_orderings, + bool progress_reasonable_orderings) : lm_graph(graph), - reached_lms(vector(graph.get_num_landmarks(), true)), - lm_status(graph.get_num_landmarks(), lm_not_reached) { + goal_landmarks(progress_goals ? get_goal_landmarks(graph) + : vector{}), + greedy_necessary_children( + progress_greedy_necessary_orderings + ? get_greedy_necessary_children(graph) + : vector>>{}), + reasonable_parents( + progress_reasonable_orderings + ? get_reasonable_parents(graph) + : vector>>{}), + /* We initialize to true in *past_landmarks* because true is the + neutral element of conjunction/set intersection. */ + past_landmarks(vector(graph.get_num_landmarks(), true)), + /* We initialize to false in *future_landmarks* because false is + the neutral element for disjunction/set union. */ + future_landmarks(vector(graph.get_num_landmarks(), false)) { } -BitsetView LandmarkStatusManager::get_reached_landmarks(const State &state) { - return reached_lms[state]; +BitsetView LandmarkStatusManager::get_past_landmarks(const State &state) { + return past_landmarks[state]; } -void LandmarkStatusManager::process_initial_state( - const State &initial_state, utils::LogProxy &log) { - set_reached_landmarks_for_initial_state(initial_state, log); - update_lm_status(initial_state); +BitsetView LandmarkStatusManager::get_future_landmarks(const State &state) { + return future_landmarks[state]; } -void LandmarkStatusManager::set_reached_landmarks_for_initial_state( - const State &initial_state, utils::LogProxy &log) { - BitsetView reached = get_reached_landmarks(initial_state); - // This is necessary since the default is "true for all" (see comment above). - reached.reset(); - - int inserted = 0; - int num_goal_lms = 0; - for (auto &lm_node : lm_graph.get_nodes()) { - const Landmark &landmark = lm_node->get_landmark(); - if (landmark.is_true_in_goal) { - ++num_goal_lms; - } +ConstBitsetView LandmarkStatusManager::get_past_landmarks(const State &state) const { + return past_landmarks[state]; +} - if (!lm_node->parents.empty()) { - continue; - } - if (landmark.conjunctive) { - bool lm_true = true; - for (const FactPair &fact : landmark.facts) { - if (initial_state[fact.var].get_value() != fact.value) { - lm_true = false; - break; - } - } - if (lm_true) { - reached.set(lm_node->get_id()); - ++inserted; +ConstBitsetView LandmarkStatusManager::get_future_landmarks(const State &state) const { + return future_landmarks[state]; +} + +void LandmarkStatusManager::progress_initial_state(const State &initial_state) { + BitsetView past = get_past_landmarks(initial_state); + BitsetView future = get_future_landmarks(initial_state); + + for (auto &node : lm_graph.get_nodes()) { + int id = node->get_id(); + const Landmark &lm = node->get_landmark(); + if (lm.is_true_in_state(initial_state)) { + assert(past.test(id)); + /* + A landmark B that holds initially is always past. If there is a + reasonable ordering A->B such that A does not hold initially, we + know B must be added again after A is added (or at the same time). + Therefore, we can set B future in these cases. + + In solvable tasks and for natural (or stronger) orderings A->B, it + is not valid for B to hold initially. Hence, if such an ordering + exists, the problem is unsolvable. Consequently, it is fine to + mark B future also in these cases, because for unsolvable + problems anything is a landmark. + */ + if (any_of(node->parents.begin(), node->parents.end(), + [initial_state](auto &parent) { + Landmark &landmark = parent.first->get_landmark(); + return !landmark.is_true_in_state(initial_state); + })) { + future.set(id); } } else { - for (const FactPair &fact : landmark.facts) { - if (initial_state[fact.var].get_value() == fact.value) { - reached.set(lm_node->get_id()); - ++inserted; - break; - } - } + past.reset(id); + future.set(id); } } - if (log.is_at_least_normal()) { - log << inserted << " initial landmarks, " - << num_goal_lms << " goal landmarks" << endl; - } } -bool LandmarkStatusManager::process_state_transition( +void LandmarkStatusManager::progress( const State &parent_ancestor_state, OperatorID, const State &ancestor_state) { if (ancestor_state == parent_ancestor_state) { // This can happen, e.g., in Satellite-01. - return false; + return; } - const BitsetView parent_reached = get_reached_landmarks(parent_ancestor_state); - BitsetView reached = get_reached_landmarks(ancestor_state); - - int num_landmarks = lm_graph.get_num_landmarks(); - assert(reached.size() == num_landmarks); - assert(parent_reached.size() == num_landmarks); - - /* - Set all landmarks not reached by this parent as "not reached". - Over multiple paths, this has the effect of computing the intersection - of "reached" for the parents. It is important here that upon first visit, - all elements in "reached" are true because true is the neutral element - of intersection. - - In the case where the landmark we are setting to false here is actually - achieved right now, it is set to "true" again below. - */ - reached.intersect(parent_reached); - - - // Mark landmarks reached right now as "reached" (if they are "leaves"). - for (int id = 0; id < num_landmarks; ++id) { - if (!reached.test(id)) { - LandmarkNode *node = lm_graph.get_node(id); - if (node->get_landmark().is_true_in_state(ancestor_state)) { - if (landmark_is_leaf(*node, reached)) { - reached.set(id); + ConstBitsetView parent_past = get_past_landmarks(parent_ancestor_state); + BitsetView past = get_past_landmarks(ancestor_state); + + ConstBitsetView parent_future = get_future_landmarks(parent_ancestor_state); + BitsetView future = get_future_landmarks(ancestor_state); + + assert(past.size() == lm_graph.get_num_landmarks()); + assert(parent_past.size() == lm_graph.get_num_landmarks()); + assert(future.size() == lm_graph.get_num_landmarks()); + assert(parent_future.size() == lm_graph.get_num_landmarks()); + + progress_landmarks( + parent_past, parent_future, parent_ancestor_state, + past, future, ancestor_state); + progress_goals(ancestor_state, future); + progress_greedy_necessary_orderings(ancestor_state, past, future); + progress_reasonable_orderings(past, future); +} + +void LandmarkStatusManager::progress_landmarks( + ConstBitsetView &parent_past, ConstBitsetView &parent_future, + const State &parent_ancestor_state, BitsetView &past, + BitsetView &future, const State &ancestor_state) { + for (auto &node : lm_graph.get_nodes()) { + int id = node->get_id(); + const Landmark &lm = node->get_landmark(); + if (parent_future.test(id)) { + if (!lm.is_true_in_state(ancestor_state)) { + /* + A landmark that is future in the parent remains future + if it does not hold in the current state. If it also + wasn't past in the parent, it remains not past. + */ + future.set(id); + if (!parent_past.test(id)) { + past.reset(id); } + } else if (lm.is_true_in_state(parent_ancestor_state)) { + /* + If the landmark held in the parent already, then it + was not added by this transition and should remain + future. + */ + assert(parent_past.test(id)); + future.set(id); } } } - - return true; } -void LandmarkStatusManager::update_lm_status(const State &ancestor_state) { - const BitsetView reached = get_reached_landmarks(ancestor_state); - - const int num_landmarks = lm_graph.get_num_landmarks(); - /* This first loop is necessary as setup for the needed-again - check in the second loop. */ - for (int id = 0; id < num_landmarks; ++id) { - lm_status[id] = reached.test(id) ? lm_reached : lm_not_reached; - } - for (int id = 0; id < num_landmarks; ++id) { - if (lm_status[id] == lm_reached - && landmark_needed_again(id, ancestor_state)) { - lm_status[id] = lm_needed_again; +void LandmarkStatusManager::progress_goals(const State &ancestor_state, + BitsetView &future) { + for (auto &node : goal_landmarks) { + if (!node->get_landmark().is_true_in_state(ancestor_state)) { + future.set(node->get_id()); } } } -bool LandmarkStatusManager::landmark_needed_again( - int id, const State &state) { - LandmarkNode *node = lm_graph.get_node(id); - const Landmark &landmark = node->get_landmark(); - if (landmark.is_true_in_state(state)) { - return false; - } else if (landmark.is_true_in_goal) { - return true; - } else { - /* - For all A ->_gn B, if B is not reached and A currently not - true, since A is a necessary precondition for actions - achieving B for the first time, it must become true again. - */ - for (const auto &child : node->children) { - if (child.second >= EdgeType::GREEDY_NECESSARY - && lm_status[child.first->get_id()] == lm_not_reached) { - return true; +void LandmarkStatusManager::progress_greedy_necessary_orderings( + const State &ancestor_state, const BitsetView &past, BitsetView &future) { + for (auto &[tail, children] : greedy_necessary_children) { + const Landmark &lm = tail->get_landmark(); + assert(!children.empty()); + for (auto &child : children) { + if (!past.test(child->get_id()) + && !lm.is_true_in_state(ancestor_state)) { + future.set(tail->get_id()); + break; } } - return false; } } -bool LandmarkStatusManager::landmark_is_leaf(const LandmarkNode &node, - const BitsetView &reached) const { - //Note: this is the same as !check_node_orders_disobeyed - for (const auto &parent : node.parents) { - LandmarkNode *parent_node = parent.first; - // Note: no condition on edge type here - if (!reached.test(parent_node->get_id())) { - return false; +void LandmarkStatusManager::progress_reasonable_orderings( + const BitsetView &past, BitsetView &future) { + for (auto &[head, parents] : reasonable_parents) { + assert(!parents.empty()); + for (auto &parent : parents) { + if (!past.test(parent->get_id())) { + future.set(head->get_id()); + break; + } } } - return true; } } diff --git a/src/search/landmarks/landmark_status_manager.h b/src/search/landmarks/landmark_status_manager.h index 0791e01efb..2458e827cd 100644 --- a/src/search/landmarks/landmark_status_manager.h +++ b/src/search/landmarks/landmark_status_manager.h @@ -9,50 +9,41 @@ namespace landmarks { class LandmarkGraph; class LandmarkNode; -enum landmark_status {lm_reached = 0, lm_not_reached = 1, lm_needed_again = 2}; - class LandmarkStatusManager { LandmarkGraph &lm_graph; - - PerStateBitset reached_lms; - std::vector lm_status; - - bool landmark_is_leaf(const LandmarkNode &node, const BitsetView &reached) const; - bool landmark_needed_again(int id, const State &state); - - void set_reached_landmarks_for_initial_state( - const State &initial_state, utils::LogProxy &log); + const std::vector goal_landmarks; + const std::vector>> greedy_necessary_children; + const std::vector>> reasonable_parents; + + PerStateBitset past_landmarks; + PerStateBitset future_landmarks; + + void progress_landmarks( + ConstBitsetView &parent_past, ConstBitsetView &parent_future, + const State &parent_ancestor_state, BitsetView &past, + BitsetView &future, const State &ancestor_state); + void progress_goals(const State &ancestor_state, BitsetView &future); + void progress_greedy_necessary_orderings( + const State &ancestor_state, const BitsetView &past, + BitsetView &future); + void progress_reasonable_orderings( + const BitsetView &past, BitsetView &future); public: - explicit LandmarkStatusManager(LandmarkGraph &graph); - - BitsetView get_reached_landmarks(const State &state); - - void update_lm_status(const State &ancestor_state); - - void process_initial_state( - const State &initial_state, utils::LogProxy &log); - bool process_state_transition( + LandmarkStatusManager( + LandmarkGraph &graph, + bool progress_goals, + bool progress_greedy_necessary_orderings, + bool progress_reasonable_orderings); + + BitsetView get_past_landmarks(const State &state); + BitsetView get_future_landmarks(const State &state); + ConstBitsetView get_past_landmarks(const State &state) const; + ConstBitsetView get_future_landmarks(const State &state) const; + + void progress_initial_state(const State &initial_state); + void progress( const State &parent_ancestor_state, OperatorID op_id, const State &ancestor_state); - - /* - TODO: - The status of a landmark is actually dependent on the state. This - is not represented in the function below. Furthermore, the status - manager only stores the status for one particular state at a time. - - At the day of writing this comment, this works as - *update_reached_lms()* is always called before the status - information is used (by calling *get_landmark_status()*). - - It would be a good idea to ensure that the status for the - desired state is returned at all times, or an error is thrown - if the desired information does not exist. - */ - landmark_status get_landmark_status(size_t id) const { - assert(static_cast(id) < lm_graph.get_num_landmarks()); - return lm_status[id]; - } }; } diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 6d777dfe2f..07cfc0068e 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -85,33 +85,21 @@ void LandmarkSumHeuristic::compute_landmark_costs() { } } -int LandmarkSumHeuristic::get_heuristic_value(const State &state) { - /* - Need explicit test to see if state is a goal state. The landmark - heuristic may compute h != 0 for a goal state if landmarks are - achieved before their parents in the landmarks graph (because - they do not get counted as reached in that case). However, we - must return 0 for a goal state. - - TODO: This check could be done before updating the *lm_status_manager*, - but if we want to do that in the base class, we need to delay this check - because it is only relevant for the inadmissible case. Moreover, it - should be redundant once we update the landmark progression. - */ - if (task_properties::is_goal_state(task_proxy, state)) - return 0; - +int LandmarkSumHeuristic::get_heuristic_value(const State &ancestor_state) { int h = 0; + ConstBitsetView past = + lm_status_manager->get_past_landmarks(ancestor_state); + ConstBitsetView future = + lm_status_manager->get_future_landmarks(ancestor_state); for (int id = 0; id < lm_graph->get_num_landmarks(); ++id) { - landmark_status status = lm_status_manager->get_landmark_status(id); - if (status == lm_not_reached) { - if (min_first_achiever_costs[id] == numeric_limits::max()) - return DEAD_END; - h += min_first_achiever_costs[id]; - } else if (status == lm_needed_again) { - if (min_possible_achiever_costs[id] == numeric_limits::max()) + if (future.test(id)) { + int min_achiever_cost = past.test(id) ? min_possible_achiever_costs[id] + : min_first_achiever_costs[id]; + if (min_achiever_cost < numeric_limits::max()) { + h += min_achiever_cost; + } else { return DEAD_END; - h += min_possible_achiever_costs[id]; + } } } return h; diff --git a/src/search/landmarks/landmark_sum_heuristic.h b/src/search/landmarks/landmark_sum_heuristic.h index db04b925a6..6a9a21ab08 100644 --- a/src/search/landmarks/landmark_sum_heuristic.h +++ b/src/search/landmarks/landmark_sum_heuristic.h @@ -13,7 +13,7 @@ class LandmarkSumHeuristic : public LandmarkHeuristic { int get_min_cost_of_achievers(const std::set &achievers) const; void compute_landmark_costs(); - int get_heuristic_value(const State &state) override; + int get_heuristic_value(const State &ancestor_state) override; public: explicit LandmarkSumHeuristic(const plugins::Options &opts); diff --git a/src/search/per_state_array.h b/src/search/per_state_array.h index 534effd862..1fad15ddc9 100644 --- a/src/search/per_state_array.h +++ b/src/search/per_state_array.h @@ -7,6 +7,26 @@ #include +template +class ConstArrayView { + const T *p; + int size_; +public: + ConstArrayView(const T *p, int size) : p(p), size_(size) {} + ConstArrayView(const ConstArrayView &other) = default; + + ConstArrayView &operator=(const ConstArrayView &other) = default; + + const T &operator[](int index) const { + assert(index >= 0 && index < size_); + return p[index]; + } + + int size() const { + return size_; + } +}; + template class ArrayView { T *p; @@ -17,6 +37,10 @@ class ArrayView { ArrayView &operator=(const ArrayView &other) = default; + operator ConstArrayView() const { + return ConstArrayView(p, size_); + } + T &operator[](int index) { assert(index >= 0 && index < size_); return p[index]; @@ -120,16 +144,28 @@ class PerStateArray : public subscriber::Subscriber { return ArrayView((*entries)[state_id], default_array.size()); } - ArrayView operator[](const State &) const { - ABORT("PerStateArray::operator[] const not implemented. " - "See source code for more information."); - /* - This method is not implemented because it is currently not used and - would require quite a bit of boilerplate, introducing a ConstArrayView - class similar to ArrayView. If you need it, it should be easy to - implement based on PerStateInformation:operator[] const. This method - should return a ConstArrayView. - */ + ConstArrayView operator[](const State &state) const { + const StateRegistry *registry = state.get_registry(); + if (!registry) { + std::cerr << "Tried to access per-state array with an unregistered " + << "state." << std::endl; + utils::exit_with(utils::ExitCode::SEARCH_CRITICAL_ERROR); + } + const segmented_vector::SegmentedArrayVector *entries = + get_entries(registry); + if (!entries) { + ABORT("PerStateArray::operator[] const tried to access " + "non-existing entry."); + } + int state_id = state.get_id().value; + assert(state.get_id() != StateID::no_state); + assert(utils::in_bounds(state_id, *registry)); + int num_entries = entries->size(); + if (state_id >= num_entries) { + ABORT("PerStateArray::operator[] const tried to access " + "non-existing entry."); + } + return ConstArrayView((*entries)[state_id], default_array.size()); } virtual void notify_service_destroyed(const StateRegistry *registry) override { diff --git a/src/search/per_state_bitset.cc b/src/search/per_state_bitset.cc index 0675d5bbe8..374b5213fb 100644 --- a/src/search/per_state_bitset.cc +++ b/src/search/per_state_bitset.cc @@ -20,10 +20,6 @@ BitsetMath::Block BitsetMath::bit_mask(size_t pos) { } -BitsetView::BitsetView(ArrayView data, int num_bits) : - data(data), num_bits(num_bits) {} - - void BitsetView::set(int index) { assert(index >= 0 && index < num_bits); int block_index = BitsetMath::block_index(index); @@ -82,3 +78,18 @@ PerStateBitset::PerStateBitset(const vector &default_bits) BitsetView PerStateBitset::operator[](const State &state) { return BitsetView(data[state], num_bits_per_entry); } + + +bool ConstBitsetView::test(int index) const { + assert(index >= 0 && index < num_bits); + int block_index = BitsetMath::block_index(index); + return (data[block_index] & BitsetMath::bit_mask(index)) != 0; +} + +int ConstBitsetView::size() const { + return num_bits; +} + +ConstBitsetView PerStateBitset::operator[](const State &state) const { + return ConstBitsetView(data[state], num_bits_per_entry); +} diff --git a/src/search/per_state_bitset.h b/src/search/per_state_bitset.h index 0fbc922f0a..21c12d8eb4 100644 --- a/src/search/per_state_bitset.h +++ b/src/search/per_state_bitset.h @@ -25,15 +25,37 @@ class BitsetMath { }; +class ConstBitsetView { + ConstArrayView data; + int num_bits; +public: + ConstBitsetView(ConstArrayView data, int num_bits) : + data(data), num_bits(num_bits) {} + + + ConstBitsetView(const ConstBitsetView &other) = default; + ConstBitsetView &operator=(const ConstBitsetView &other) = default; + + bool test(int index) const; + int size() const; +}; + + class BitsetView { ArrayView data; int num_bits; public: - BitsetView(ArrayView data, int num_bits); + BitsetView(ArrayView data, int num_bits) : + data(data), num_bits(num_bits) {} + BitsetView(const BitsetView &other) = default; BitsetView &operator=(const BitsetView &other) = default; + operator ConstBitsetView() const { + return ConstBitsetView(data, num_bits); + } + void set(int index); void reset(int index); void reset(); @@ -53,6 +75,7 @@ class PerStateBitset { PerStateBitset &operator=(const PerStateBitset &) = delete; BitsetView operator[](const State &state); + ConstBitsetView operator[](const State &state) const; }; #endif