From 957a6d44280378f2b5047c0c3d0e34d76cdd7fc0 Mon Sep 17 00:00:00 2001 From: tanjaschindler Date: Fri, 16 Feb 2024 16:55:29 +0100 Subject: [PATCH] [issue1082] triage TODOs --- src/search/AAA_Mechanical_Changes.md | 16 ++++++++-------- .../additive_cartesian_heuristic.cc | 2 +- src/search/evaluator.cc | 5 ----- src/search/evaluator.h | 1 - .../landmark_cost_partitioning_heuristic.cc | 4 ++-- .../landmark_cost_partitioning_heuristic.h | 4 ++-- src/search/landmarks/landmark_sum_heuristic.cc | 6 +++--- src/search/landmarks/landmark_sum_heuristic.h | 2 +- src/search/search_algorithm.cc | 2 +- src/search/search_algorithm.h | 2 +- .../enforced_hill_climbing_search.cc | 2 +- src/search/search_algorithms/iterated_search.h | 2 +- .../search_algorithms/plugin_eager_greedy.cc | 12 ++++++------ .../search_algorithms/plugin_lazy_greedy.cc | 10 +++++----- src/search/search_algorithms/search_common.cc | 1 - 15 files changed, 32 insertions(+), 39 deletions(-) diff --git a/src/search/AAA_Mechanical_Changes.md b/src/search/AAA_Mechanical_Changes.md index 7ee39e3764..dca0be6b2d 100644 --- a/src/search/AAA_Mechanical_Changes.md +++ b/src/search/AAA_Mechanical_Changes.md @@ -66,11 +66,11 @@ search ├── ✅ evaluation_result.h ├── ✅ evaluator_cache.cc ├── ✅ evaluator_cache.h -├── ❓ evaluator.cc -├── ❓ evaluator.h +├── ✅ evaluator.cc +├── ✅ evaluator.h ├── ✅ evaluators -├── ❓ heuristic.cc -├── ❓ heuristic.h +├── ✅ heuristic.cc +├── ✅ heuristic.h ├── ✅ heuristics ├── ✅ landmarks ├── ✅ lp @@ -78,7 +78,7 @@ search ├── ✅ open_list_factory.cc ├── ✅ open_list_factory.h ├── ✅ open_list.h -├── ❌ open_lists +├── ✅ open_lists ├── ✅ operator_cost.cc ├── ✅ operator_cost.h ├── ✅ operator_counting @@ -95,7 +95,7 @@ search ├── ✅ plan_manager.h ├── ✅ planner.cc ├── ✅ plugins -├── ❌ potentials +├── ✅ potentials ├── ✅ pruning ├── ✅ pruning_method.cc ├── ✅ pruning_method.h @@ -117,8 +117,8 @@ search │ ├── ✅ plugin_lazy.cc │ ├── ✅ plugin_lazy_greedy.cc │ ├── ✅ plugin_lazy_wastar.cc -│ ├── ❓ search_common.cc -│ └── ❓ search_common.h +│ ├── ✅ search_common.cc +│ └── ✅ search_common.h ├── ✅ search_node_info.cc ├── ✅ search_node_info.h ├── ✅ search_progress.cc diff --git a/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc b/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc index 59436d67c2..88963428c3 100644 --- a/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc +++ b/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc @@ -146,7 +146,7 @@ class AdditiveCartesianHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts, const utils::Context &) const override { return plugins::make_shared_from_arg_tuples( - opts.get>>("subtasks"), // TODO issue1082 why not get_list? + opts.get_list>("subtasks"), opts.get("max_states"), opts.get("max_transitions"), opts.get("max_time"), diff --git a/src/search/evaluator.cc b/src/search/evaluator.cc index 530ce89b24..fb35f178c3 100644 --- a/src/search/evaluator.cc +++ b/src/search/evaluator.cc @@ -83,11 +83,6 @@ void add_evaluator_options_to_feature(plugins::Feature &feature, const string &d utils::add_log_options_to_feature(feature); } -// TODO 1082 remove this, just keep the one above -void add_evaluator_options_to_feature(plugins::Feature &feature) { - utils::add_log_options_to_feature(feature); -} - tuple get_evaluator_arguments_from_options( const plugins::Options &opts) { return tuple_cat( diff --git a/src/search/evaluator.h b/src/search/evaluator.h index 5ab55daf99..a3a5726686 100644 --- a/src/search/evaluator.h +++ b/src/search/evaluator.h @@ -102,7 +102,6 @@ class Evaluator { }; extern void add_evaluator_options_to_feature(plugins::Feature &feature, const std::string &description); -extern void add_evaluator_options_to_feature(plugins::Feature &feature); // TODO 1082 remove this, just keep the one above extern std::tuple get_evaluator_arguments_from_options(const plugins::Options &opts); extern std::tuple get_evaluator_default_arguments(); diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc index 9639473c94..36bce18a7e 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc @@ -39,9 +39,9 @@ LandmarkCostPartitioningHeuristic::LandmarkCostPartitioningHeuristic( } void LandmarkCostPartitioningHeuristic::check_unsupported_features( - const plugins::Options &opts) { + const plugins::Options &lm_factory_option) { shared_ptr lm_graph_factory = - opts.get>("lm_factory"); + lm_factory_option.get>("lm_factory"); if (task_properties::has_axioms(task_proxy)) { cerr << "Cost partitioning does not support axioms." << endl; diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.h b/src/search/landmarks/landmark_cost_partitioning_heuristic.h index 8d7de52067..bbcb4877c2 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.h +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.h @@ -15,7 +15,7 @@ enum class CostPartitioningMethod { class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic { std::unique_ptr cost_partitioning_algorithm; - void check_unsupported_features(const plugins::Options &opts); // TODO issue1082 this needs Options to construct the lm_factory later. + void check_unsupported_features(const plugins::Options &lm_factory_option); // TODO issue1082 this needs Options to construct the lm_factory later. void set_cost_partitioning_algorithm(CostPartitioningMethod cost_partitioning, lp::LPSolverType lpsolver, bool alm); @@ -32,7 +32,7 @@ class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic { This should be handled by issue559 eventually. */ LandmarkCostPartitioningHeuristic( - const plugins::Options &options, + const plugins::Options &lm_factory_option, bool use_preferred_operators, bool prog_goal, bool prog_gn, diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 5ba1be9e59..94b82a340a 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_options, + const plugins::Options &lm_factory_option, 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_options.get>("lm_factory"), + lm_factory_option.get>("lm_factory"), task_proxy)) { if (log.is_at_least_normal()) { log << "Initializing landmark sum heuristic..." << endl; } - initialize(lm_factory_options, prog_goal, prog_gn, prog_r); + initialize(lm_factory_option, prog_goal, prog_gn, prog_r); compute_landmark_costs(); } diff --git a/src/search/landmarks/landmark_sum_heuristic.h b/src/search/landmarks/landmark_sum_heuristic.h index 835c147056..6e89696d18 100644 --- a/src/search/landmarks/landmark_sum_heuristic.h +++ b/src/search/landmarks/landmark_sum_heuristic.h @@ -24,7 +24,7 @@ class LandmarkSumHeuristic : public LandmarkHeuristic { 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_options, + LandmarkSumHeuristic(const plugins::Options &lm_factory_option, bool use_preferred_operators, bool prog_goal, bool prog_gn, diff --git a/src/search/search_algorithm.cc b/src/search/search_algorithm.cc index a47d3d2db0..2519fc1936 100644 --- a/src/search/search_algorithm.cc +++ b/src/search/search_algorithm.cc @@ -67,7 +67,7 @@ SearchAlgorithm::SearchAlgorithm( task_properties::print_variable_statistics(task_proxy); } -SearchAlgorithm::SearchAlgorithm(const plugins::Options &opts) // TODO issue1082 needed for iterated search +SearchAlgorithm::SearchAlgorithm(const plugins::Options &opts) // TODO objections object is needed for iterated search, the prototype for issue559 resolves this : description(opts.get_unparsed_config()), status(IN_PROGRESS), solution_found(false), diff --git a/src/search/search_algorithm.h b/src/search/search_algorithm.h index 5b4365deab..5f0c464b90 100644 --- a/src/search/search_algorithm.h +++ b/src/search/search_algorithm.h @@ -66,7 +66,7 @@ class SearchAlgorithm { double max_time, const std::string &description, utils::Verbosity verbosity); - explicit SearchAlgorithm(const plugins::Options &opts); // TODO issue1082 needed for iterated search + explicit SearchAlgorithm(const plugins::Options &opts); // TODO objections object is needed for iterated search, the prototype for issue559 resolves this virtual ~SearchAlgorithm(); virtual void print_statistics() const = 0; virtual void save_plan_if_necessary(); diff --git a/src/search/search_algorithms/enforced_hill_climbing_search.cc b/src/search/search_algorithms/enforced_hill_climbing_search.cc index 7ab17686b7..087e04d67d 100644 --- a/src/search/search_algorithms/enforced_hill_climbing_search.cc +++ b/src/search/search_algorithms/enforced_hill_climbing_search.cc @@ -32,7 +32,7 @@ static shared_ptr create_ehc_open_list_factory( } else { /* TODO: Reduce code duplication with search_common.cc, - function create_astar_open_list_factory_and_f_eval. TODO: issue1082 update comment + function create_astar_open_list_factory_and_f_eval. It would probably make sense to add a factory function or constructor that encapsulates this work to the tie-breaking diff --git a/src/search/search_algorithms/iterated_search.h b/src/search/search_algorithms/iterated_search.h index f3a2dc875d..026ba53804 100644 --- a/src/search/search_algorithms/iterated_search.h +++ b/src/search/search_algorithms/iterated_search.h @@ -29,7 +29,7 @@ class IteratedSearch : public SearchAlgorithm { virtual SearchStatus step() override; public: - IteratedSearch(const plugins::Options &opts); // TODO issue1082 still needs the options objects, the prototype for issue559 resolves this + IteratedSearch(const plugins::Options &opts); // TODO this still needs the options objects, the prototype for issue559 resolves this virtual void save_plan_if_necessary() override; virtual void print_statistics() const override; diff --git a/src/search/search_algorithms/plugin_eager_greedy.cc b/src/search/search_algorithms/plugin_eager_greedy.cc index 1b5ae8d319..c57efd7261 100644 --- a/src/search/search_algorithms/plugin_eager_greedy.cc +++ b/src/search/search_algorithms/plugin_eager_greedy.cc @@ -34,28 +34,28 @@ class EagerGreedySearchFeature : public plugins::TypedFeature