From 69a875f76fbd2306577b263401047d9529cf8262 Mon Sep 17 00:00:00 2001 From: ClemensBuechner Date: Thu, 7 Sep 2023 17:13:02 +0200 Subject: [PATCH] Apply review comments. --- .../landmarks/landmark_cost_assignment.cc | 16 ++++++++-------- .../landmarks/landmark_cost_assignment.h | 18 +++++++++--------- .../landmark_cost_partitioning_heuristic.cc | 15 +++++++-------- .../landmark_cost_partitioning_heuristic.h | 7 +++---- 4 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/search/landmarks/landmark_cost_assignment.cc b/src/search/landmarks/landmark_cost_assignment.cc index 4be4b57b62..d1ba0a65c2 100644 --- a/src/search/landmarks/landmark_cost_assignment.cc +++ b/src/search/landmarks/landmark_cost_assignment.cc @@ -17,12 +17,12 @@ using namespace std; namespace landmarks { -LandmarkCostPartitioningAlgorithm::LandmarkCostPartitioningAlgorithm( +CostPartitioningAlgorithm::CostPartitioningAlgorithm( const vector &operator_costs, const LandmarkGraph &graph) : lm_graph(graph), operator_costs(operator_costs) { } -const set &LandmarkCostPartitioningAlgorithm::get_achievers( +const set &CostPartitioningAlgorithm::get_achievers( const Landmark &landmark, bool past) const { // Return relevant achievers of the landmark according to its status. if (past) { @@ -36,11 +36,11 @@ const set &LandmarkCostPartitioningAlgorithm::get_achievers( UniformCostPartitioningAlgorithm::UniformCostPartitioningAlgorithm( const vector &operator_costs, const LandmarkGraph &graph, bool use_action_landmarks) - : LandmarkCostPartitioningAlgorithm(operator_costs, graph), + : CostPartitioningAlgorithm(operator_costs, graph), use_action_landmarks(use_action_landmarks) { } -double UniformCostPartitioningAlgorithm::compute_cost_partitioning( +double UniformCostPartitioningAlgorithm::compute_cost_partitioned_h_value( const LandmarkStatusManager &lm_status_manager, const State &ancestor_state) { vector achieved_lms_by_op(operator_costs.size(), 0); @@ -128,9 +128,9 @@ double UniformCostPartitioningAlgorithm::compute_cost_partitioning( int num_achieved = achieved_lms_by_op[op_id]; assert(num_achieved >= 1); assert(utils::in_bounds(op_id, operator_costs)); - double shared_cost = + double partitioned_cost = static_cast(operator_costs[op_id]) / num_achieved; - min_cost = min(min_cost, shared_cost); + min_cost = min(min_cost, partitioned_cost); } h += min_cost; } @@ -142,7 +142,7 @@ double UniformCostPartitioningAlgorithm::compute_cost_partitioning( OptimalCostPartitioningAlgorithm::OptimalCostPartitioningAlgorithm( const vector &operator_costs, const LandmarkGraph &graph, lp::LPSolverType solver_type) - : LandmarkCostPartitioningAlgorithm(operator_costs, graph), + : CostPartitioningAlgorithm(operator_costs, graph), lp_solver(solver_type), lp(build_initial_lp()) { } @@ -175,7 +175,7 @@ lp::LinearProgram OptimalCostPartitioningAlgorithm::build_initial_lp() { {}, lp_solver.get_infinity()); } -double OptimalCostPartitioningAlgorithm::compute_cost_partitioning( +double OptimalCostPartitioningAlgorithm::compute_cost_partitioned_h_value( const LandmarkStatusManager &lm_status_manager, const State &ancestor_state) { /* TODO: We could also do the same thing with action landmarks we diff --git a/src/search/landmarks/landmark_cost_assignment.h b/src/search/landmarks/landmark_cost_assignment.h index 33a4cd2d40..28385b3c71 100644 --- a/src/search/landmarks/landmark_cost_assignment.h +++ b/src/search/landmarks/landmark_cost_assignment.h @@ -16,7 +16,7 @@ class LandmarkGraph; class LandmarkNode; class LandmarkStatusManager; -class LandmarkCostPartitioningAlgorithm { +class CostPartitioningAlgorithm { protected: const LandmarkGraph &lm_graph; const std::vector operator_costs; @@ -24,28 +24,28 @@ class LandmarkCostPartitioningAlgorithm { const std::set &get_achievers( const Landmark &landmark, bool past) const; public: - LandmarkCostPartitioningAlgorithm(const std::vector &operator_costs, - const LandmarkGraph &graph); - virtual ~LandmarkCostPartitioningAlgorithm() = default; + CostPartitioningAlgorithm(const std::vector &operator_costs, + const LandmarkGraph &graph); + virtual ~CostPartitioningAlgorithm() = default; - virtual double compute_cost_partitioning( + virtual double compute_cost_partitioned_h_value( const LandmarkStatusManager &lm_status_manager, const State &ancestor_state) = 0; }; -class UniformCostPartitioningAlgorithm : public LandmarkCostPartitioningAlgorithm { +class UniformCostPartitioningAlgorithm : public CostPartitioningAlgorithm { bool use_action_landmarks; public: UniformCostPartitioningAlgorithm(const std::vector &operator_costs, const LandmarkGraph &graph, bool use_action_landmarks); - virtual double compute_cost_partitioning( + virtual double compute_cost_partitioned_h_value( const LandmarkStatusManager &lm_status_manager, const State &ancestor_state) override; }; -class OptimalCostPartitioningAlgorithm : public LandmarkCostPartitioningAlgorithm { +class OptimalCostPartitioningAlgorithm : public CostPartitioningAlgorithm { lp::LPSolver lp_solver; /* We keep an additional copy of the constraints around to avoid some effort with recreating the vector (see issue443). */ @@ -64,7 +64,7 @@ class OptimalCostPartitioningAlgorithm : public LandmarkCostPartitioningAlgorith const LandmarkGraph &graph, lp::LPSolverType solver_type); - virtual double compute_cost_partitioning( + virtual double compute_cost_partitioned_h_value( 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 f923da58c7..e41d0e0787 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc @@ -17,9 +17,7 @@ using namespace std; namespace landmarks { LandmarkCostPartitioningHeuristic::LandmarkCostPartitioningHeuristic( const plugins::Options &opts) - : LandmarkHeuristic(opts), - cost_partitioning_strategy( - opts.get("cost_partitioning")) { + : LandmarkHeuristic(opts) { if (log.is_at_least_normal()) { log << "Initializing landmark cost partitioning heuristic..." << endl; } @@ -48,12 +46,13 @@ void LandmarkCostPartitioningHeuristic::check_unsupported_features( void LandmarkCostPartitioningHeuristic::set_cost_partitioning_algorithm( const plugins::Options &opts) { - if (cost_partitioning_strategy == CostPartitioningStrategy::OPTIMAL) { + auto method = opts.get("cost_partitioning"); + if (method == CostPartitioningMethod::OPTIMAL) { cost_partitioning_algorithm = utils::make_unique_ptr( task_properties::get_operator_costs(task_proxy), *lm_graph, opts.get("lpsolver")); - } else if (cost_partitioning_strategy == CostPartitioningStrategy::UNIFORM) { + } else if (method == CostPartitioningMethod::UNIFORM) { cost_partitioning_algorithm = utils::make_unique_ptr( task_properties::get_operator_costs(task_proxy), @@ -67,7 +66,7 @@ int LandmarkCostPartitioningHeuristic::get_heuristic_value( const State &ancestor_state) { double epsilon = 0.01; - double h_val = cost_partitioning_algorithm->compute_cost_partitioning( + double h_val = cost_partitioning_algorithm->compute_cost_partitioned_h_value( *lm_status_manager, ancestor_state); if (h_val == numeric_limits::max()) { return DEAD_END; @@ -108,7 +107,7 @@ class LandmarkCostPartitioningHeuristicFeature : public plugins::TypedFeature( + add_option( "cost_partitioning", "strategy for partitioning operator costs among landmarks", "uniform"); @@ -156,7 +155,7 @@ class LandmarkCostPartitioningHeuristicFeature : public plugins::TypedFeature _plugin; -static plugins::TypedEnumPlugin _enum_plugin({ +static plugins::TypedEnumPlugin _enum_plugin({ {"optimal", "use optimal (LP-based) cost partitioning"}, {"uniform", diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.h b/src/search/landmarks/landmark_cost_partitioning_heuristic.h index 2d3b77ea68..cd08edff54 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.h +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.h @@ -4,16 +4,15 @@ #include "landmark_heuristic.h" namespace landmarks { -class LandmarkCostPartitioningAlgorithm; +class CostPartitioningAlgorithm; -enum class CostPartitioningStrategy { +enum class CostPartitioningMethod { OPTIMAL, UNIFORM, }; class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic { - const CostPartitioningStrategy cost_partitioning_strategy; - std::unique_ptr cost_partitioning_algorithm; + std::unique_ptr cost_partitioning_algorithm; void check_unsupported_features(const plugins::Options &opts); void set_cost_partitioning_algorithm(const plugins::Options &opts);