From 503d5db126d132cc64ad12fd77e4775d5ee21ccd Mon Sep 17 00:00:00 2001 From: Simon Dold Date: Wed, 6 Mar 2024 12:26:19 +0100 Subject: [PATCH] remove options object from LandmarHeuristic. --- .../landmark_cost_partitioning_heuristic.cc | 17 +++------- .../landmark_cost_partitioning_heuristic.h | 13 ++------ src/search/landmarks/landmark_heuristic.cc | 32 ++++++++----------- src/search/landmarks/landmark_heuristic.h | 8 ++--- .../landmarks/landmark_sum_heuristic.cc | 10 ++---- src/search/landmarks/landmark_sum_heuristic.h | 10 +----- 6 files changed, 27 insertions(+), 63 deletions(-) diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc index 36bce18a7e..71cfbd94b9 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc @@ -16,7 +16,7 @@ using namespace std; namespace landmarks { LandmarkCostPartitioningHeuristic::LandmarkCostPartitioningHeuristic( - const plugins::Options &lm_factory_option, + const shared_ptr &lm_factory, bool use_preferred_operators, bool prog_goal, bool prog_gn, @@ -33,22 +33,19 @@ LandmarkCostPartitioningHeuristic::LandmarkCostPartitioningHeuristic( if (log.is_at_least_normal()) { log << "Initializing landmark cost partitioning heuristic..." << endl; } - check_unsupported_features(lm_factory_option); - initialize(lm_factory_option, prog_goal, prog_gn, prog_r); + check_unsupported_features(lm_factory); + initialize(lm_factory, prog_goal, prog_gn, prog_r); set_cost_partitioning_algorithm(cost_partitioning, lpsolver, alm); } void LandmarkCostPartitioningHeuristic::check_unsupported_features( - const plugins::Options &lm_factory_option) { - shared_ptr lm_graph_factory = - lm_factory_option.get>("lm_factory"); - + const shared_ptr &lm_factory) { if (task_properties::has_axioms(task_proxy)) { cerr << "Cost partitioning does not support axioms." << endl; utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); } - if (!lm_graph_factory->supports_conditional_effects() + if (!lm_factory->supports_conditional_effects() && task_properties::has_conditional_effects(task_proxy)) { cerr << "Conditional effects not supported by the landmark " << "generation method." << endl; @@ -177,11 +174,7 @@ class LandmarkCostPartitioningHeuristicFeature : public plugins::TypedFeature create_component( const plugins::Options &opts, const utils::Context &) const override { - plugins::Options lm_factory_options; - lm_factory_options.set>( - "lm_factory", opts.get>("lm_factory")); return plugins::make_shared_from_arg_tuples( - lm_factory_options, get_landmark_heuristic_arguments_from_options(opts), opts.get("cost_partitioning"), opts.get("alm"), diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.h b/src/search/landmarks/landmark_cost_partitioning_heuristic.h index bbcb4877c2..c1e0b30016 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.h +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.h @@ -15,24 +15,15 @@ enum class CostPartitioningMethod { class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic { std::unique_ptr cost_partitioning_algorithm; - void check_unsupported_features(const plugins::Options &lm_factory_option); // TODO issue1082 this needs Options to construct the lm_factory later. + void check_unsupported_features(const std::shared_ptr &lm_factory); void set_cost_partitioning_algorithm(CostPartitioningMethod cost_partitioning, lp::LPSolverType lpsolver, bool alm); int get_heuristic_value(const State &ancestor_state) override; public: - /* - TODO: issue1082 aimed to remove the options object from constructors. - This is not possible here because we need to wait with initializing the - landmark factory until the task is given (e.g., cost transformation). - Therefore, we can only extract the landmark factory from the options - after this happened, so we allow the landmark heuristics to keep a - (small) options object around for that purpose. - This should be handled by issue559 eventually. - */ LandmarkCostPartitioningHeuristic( - const plugins::Options &lm_factory_option, + const std::shared_ptr &lm_factory, bool use_preferred_operators, bool prog_goal, bool prog_gn, diff --git a/src/search/landmarks/landmark_heuristic.cc b/src/search/landmarks/landmark_heuristic.cc index 94fd381a3c..17a9da38e6 100644 --- a/src/search/landmarks/landmark_heuristic.cc +++ b/src/search/landmarks/landmark_heuristic.cc @@ -25,7 +25,7 @@ LandmarkHeuristic::LandmarkHeuristic( } void LandmarkHeuristic::initialize( - const plugins::Options &lm_factory_option, + const shared_ptr &lm_factory, bool prog_goal, bool prog_gn, bool prog_r) { @@ -42,7 +42,7 @@ void LandmarkHeuristic::initialize( utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); } - compute_landmark_graph(lm_factory_option); + compute_landmark_graph(lm_factory); lm_status_manager = utils::make_unique_ptr( *lm_graph, prog_goal, @@ -100,16 +100,14 @@ bool LandmarkHeuristic::depth_first_search_for_cycle_of_natural_orderings( return false; } -void LandmarkHeuristic::compute_landmark_graph(const plugins::Options &lm_factory_option) { +void LandmarkHeuristic::compute_landmark_graph(const shared_ptr &lm_factory) { utils::Timer lm_graph_timer; if (log.is_at_least_normal()) { log << "Generating landmark graph..." << endl; } - shared_ptr lm_graph_factory = - lm_factory_option.get>("lm_factory"); - lm_graph = lm_graph_factory->compute_lm_graph(task); - assert(lm_graph_factory->achievers_are_calculated()); + lm_graph = lm_factory->compute_lm_graph(task); + assert(lm_factory->achievers_are_calculated()); if (log.is_at_least_normal()) { log << "Landmark graph generation time: " << lm_graph_timer << endl; @@ -237,20 +235,16 @@ void add_landmark_heuristic_options_to_feature(plugins::Feature &feature, "yes (if enabled; see ``pref`` option)"); } -tuple, bool, string, utils::Verbosity> -get_landmark_heuristic_arguments_from_options(const plugins::Options &options) { +tuple, bool, bool, bool, bool, shared_ptr, bool, string, utils::Verbosity> +get_landmark_heuristic_arguments_from_options(const plugins::Options &opts) { return tuple_cat( make_tuple( - options.get("pref"), - /* - TODO: add_landmark_heuristic_options_to_feature also adds "lm_factory". - Here we do not extract it to put it in the argument tuple because we want the lm_factory to be - created later. - */ - options.get("prog_goal"), - options.get("prog_gn"), - options.get("prog_r") + opts.get>("lm_factory"), + opts.get("pref"), + opts.get("prog_goal"), + opts.get("prog_gn"), + opts.get("prog_r") ), - get_heuristic_arguments_from_options(options)); + get_heuristic_arguments_from_options(opts)); } } diff --git a/src/search/landmarks/landmark_heuristic.h b/src/search/landmarks/landmark_heuristic.h index 079f9ec89e..f7edcf3876 100644 --- a/src/search/landmarks/landmark_heuristic.h +++ b/src/search/landmarks/landmark_heuristic.h @@ -31,10 +31,8 @@ class LandmarkHeuristic : public Heuristic { std::unique_ptr lm_status_manager; std::unique_ptr successor_generator; - // TODO this needs lm_factory_options to construct the lm_factory later. - // This should be handled by issue559 eventually. - void initialize(const plugins::Options &lm_factory_option, bool prog_goal, bool prog_gn, bool prog_r); - void compute_landmark_graph(const plugins::Options &lm_factory_option); + void initialize(const std::shared_ptr &lm_factory, bool prog_goal, bool prog_gn, bool prog_r); + void compute_landmark_graph(const std::shared_ptr &lm_factory); virtual int get_heuristic_value(const State &ancestor_state) = 0; @@ -61,7 +59,7 @@ class LandmarkHeuristic : public Heuristic { extern void add_landmark_heuristic_options_to_feature( plugins::Feature &feature, const std::string &description); -extern std::tuple, bool, std::string, utils::Verbosity> get_landmark_heuristic_arguments_from_options( +extern std::tuple, bool, bool, bool, bool, std::shared_ptr, bool, std::string, utils::Verbosity> get_landmark_heuristic_arguments_from_options( const plugins::Options &options); } diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 94b82a340a..20c92cc664 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -31,7 +31,7 @@ static bool are_dead_ends_reliable( } LandmarkSumHeuristic::LandmarkSumHeuristic( - const plugins::Options &lm_factory_option, + const shared_ptr &lm_factory, bool use_preferred_operators, bool prog_goal, bool prog_gn, @@ -44,12 +44,12 @@ LandmarkSumHeuristic::LandmarkSumHeuristic( description, verbosity), dead_ends_reliable( are_dead_ends_reliable( - lm_factory_option.get>("lm_factory"), + lm_factory, task_proxy)) { if (log.is_at_least_normal()) { log << "Initializing landmark sum heuristic..." << endl; } - initialize(lm_factory_option, prog_goal, prog_gn, prog_r); + initialize(lm_factory, prog_goal, prog_gn, prog_r); compute_landmark_costs(); } @@ -202,11 +202,7 @@ class LandmarkSumHeuristicFeature : public plugins::TypedFeature create_component( const plugins::Options &opts, const utils::Context &) const override { - plugins::Options lm_factory_options; - lm_factory_options.set>( - "lm_factory", opts.get>("lm_factory")); return plugins::make_shared_from_arg_tuples( - lm_factory_options, get_landmark_heuristic_arguments_from_options(opts)); } }; diff --git a/src/search/landmarks/landmark_sum_heuristic.h b/src/search/landmarks/landmark_sum_heuristic.h index 6e89696d18..3ba38ed294 100644 --- a/src/search/landmarks/landmark_sum_heuristic.h +++ b/src/search/landmarks/landmark_sum_heuristic.h @@ -16,15 +16,7 @@ class LandmarkSumHeuristic : public LandmarkHeuristic { int get_heuristic_value(const State &ancestor_state) override; public: - /* - TODO: issue1082 aimed to remove the options object from constructors. - This is not possible here because we need to wait with initializing the - landmark factory until the task is given (e.g., cost transformation). - Therefore, we can only extract the landmark factory from the options - after this happened, so we allow the landmark heuristics to keep a - (small) options object around for that purpose. - */ - LandmarkSumHeuristic(const plugins::Options &lm_factory_option, + LandmarkSumHeuristic(const std::shared_ptr &lm_factory, bool use_preferred_operators, bool prog_goal, bool prog_gn,