[issue1082] minimize the use of Options in lm_heuristics.

src/search/AAA_Mechanical_Changes.md

@@ -50,7 +50,6 @@ Mark in the list below what you updated:
 
 ```
 search
-<<<<<<< HEAD
 ├── ✅ abstract_task.cc
 ├── ✅ abstract_task.h
 ├── ✅ algorithms
@@ -73,66 +72,7 @@ search
 ├── ❓ heuristic.cc
 ├── ❓ heuristic.h
 ├── ✅ heuristics
-│   ├── ✅ additive_heuristic.cc
-│   ├── ✅ additive_heuristic.h
-│   ├── ✅ array_pool.h
-│   ├── ✅ blind_search_heuristic.cc
-│   ├── ✅ blind_search_heuristic.h
-│   ├── ✅ cea_heuristic.cc
-│   ├── ✅ cea_heuristic.h
-│   ├── ✅ cg_cache.cc
-│   ├── ✅ cg_cache.h
-│   ├── ✅ cg_heuristic.cc
-│   ├── ✅ cg_heuristic.h
-│   ├── ✅ domain_transition_graph.cc
-│   ├── ✅ domain_transition_graph.h
-│   ├── ✅ ff_heuristic.cc
-│   ├── ✅ ff_heuristic.h
-│   ├── ✅ goal_count_heuristic.cc
-│   ├── ✅ goal_count_heuristic.h
-│   ├── ✅ hm_heuristic.cc
-│   ├── ✅ hm_heuristic.h
-│   ├── ✅ lm_cut_heuristic.cc
-│   ├── ✅ lm_cut_heuristic.h
-│   ├── ✅ lm_cut_landmarks.cc
-│   ├── ✅ lm_cut_landmarks.h
-│   ├── ✅ max_heuristic.cc
-│   ├── ✅ max_heuristic.h
-│   ├── ✅ relaxation_heuristic.cc
-│   └── ✅ relaxation_heuristic.h
 ├── ✅ landmarks
-│   ├── ✅ exploration.cc
-│   ├── ✅ exploration.h
-│   ├── ✅ landmark.cc
-│   ├── ✅ landmark.h
-│   ├── ✅ landmark_cost_partitioning_algorithms.cc
-│   ├── ✅ landmark_cost_partitioning_algorithms.h
-│   ├── ✅ landmark_cost_partitioning_heuristic.cc
-│   ├── ✅ landmark_cost_partitioning_heuristic.h
-│   ├── ✅ landmark_factory.cc
-│   ├── ✅ landmark_factory.h
-│   ├── ✅ landmark_factory_h_m.cc
-│   ├── ✅ landmark_factory_h_m.h
-│   ├── ✅ landmark_factory_merged.cc
-│   ├── ✅ landmark_factory_merged.h
-│   ├── ✅ landmark_factory_reasonable_orders_hps.cc
-│   ├── ✅ landmark_factory_reasonable_orders_hps.h
-│   ├── ✅ landmark_factory_relaxation.cc
-│   ├── ✅ landmark_factory_relaxation.h
-│   ├── ✅ landmark_factory_rpg_exhaust.cc
-│   ├── ✅ landmark_factory_rpg_exhaust.h
-│   ├── ✅ landmark_factory_rpg_sasp.cc
-│   ├── ✅ landmark_factory_rpg_sasp.h
-│   ├── ✅ landmark_factory_zhu_givan.cc
-│   ├── ✅ landmark_factory_zhu_givan.h
-│   ├── ✅ landmark_graph.cc
-│   ├── ✅ landmark_graph.h
-│   ├── ✅ landmark_heuristic.cc
-│   ├── ✅ landmark_heuristic.h
-│   ├── ✅ landmark_status_manager.cc
-│   ├── ✅ landmark_status_manager.h
-│   ├── ✅ landmark_sum_heuristic.cc
-│   └── ✅ landmark_sum_heuristic.h
 ├── ❌ lp
 ├── ✅ merge_and_shrink
 ├── ✅ open_list_factory.cc

src/search/landmarks/landmark_cost_partitioning_heuristic.cc

@@ -16,20 +16,26 @@ using namespace std;
 
 namespace landmarks {
 LandmarkCostPartitioningHeuristic::LandmarkCostPartitioningHeuristic(
-    const plugins::Options &options,
+    const plugins::Options &lm_factory_option,
     bool use_preferred_operators,
+    bool prog_goal,
+    bool prog_gn,
+    bool prog_r,
     const shared_ptr<AbstractTask> &transform,
     bool cache_estimates,
     const string &description,
-    utils::Verbosity verbosity)
+    utils::Verbosity verbosity,
+    CostPartitioningMethod cost_partitioning,
+    bool alm,
+    lp::LPSolverType lpsolver)
     : LandmarkHeuristic(use_preferred_operators, transform, cache_estimates,
                         description, verbosity) {
     if (log.is_at_least_normal()) {
         log << "Initializing landmark cost partitioning heuristic..." << endl;
     }
-    check_unsupported_features(options);
-    initialize(options);
-    set_cost_partitioning_algorithm(options);
+    check_unsupported_features(lm_factory_option);
+    initialize(lm_factory_option, prog_goal, prog_gn, prog_r);
+    set_cost_partitioning_algorithm(cost_partitioning, lpsolver, alm);
 }
 
 void LandmarkCostPartitioningHeuristic::check_unsupported_features(
@@ -51,18 +57,19 @@ void LandmarkCostPartitioningHeuristic::check_unsupported_features(
 }
 
 void LandmarkCostPartitioningHeuristic::set_cost_partitioning_algorithm(
-    const plugins::Options &opts) {
-    auto method = opts.get<CostPartitioningMethod>("cost_partitioning");
-    if (method == CostPartitioningMethod::OPTIMAL) {
+    CostPartitioningMethod cost_partitioning,
+    lp::LPSolverType lpsolver,
+    bool alm) {
+    if (cost_partitioning == CostPartitioningMethod::OPTIMAL) {
         cost_partitioning_algorithm =
             utils::make_unique_ptr<OptimalCostPartitioningAlgorithm>(
                 task_properties::get_operator_costs(task_proxy),
-                *lm_graph, opts.get<lp::LPSolverType>("lpsolver"));
-    } else if (method == CostPartitioningMethod::UNIFORM) {
+                *lm_graph, lpsolver);
+    } else if (cost_partitioning == CostPartitioningMethod::UNIFORM) {
         cost_partitioning_algorithm =
             utils::make_unique_ptr<UniformCostPartitioningAlgorithm>(
                 task_properties::get_operator_costs(task_proxy),
-                *lm_graph, opts.get<bool>("alm"));
+                *lm_graph, alm);
     } else {
         ABORT("Unknown cost partitioning method");
     }
@@ -169,18 +176,17 @@ class LandmarkCostPartitioningHeuristicFeature : public plugins::TypedFeature<Ev
     }
 
     virtual shared_ptr<LandmarkCostPartitioningHeuristic> create_component(
-        const plugins::Options &options, const utils::Context &) const override {
-        plugins::Options lmcp_options =
-            collect_landmark_heuristic_options(options);
-        lmcp_options.set<CostPartitioningMethod>(
-            "cost_partitioning",
-            options.get<CostPartitioningMethod>("cost_partitioning"));
-        lmcp_options.set<bool>("alm", options.get<bool>("alm"));
-        lmcp_options.set<lp::LPSolverType>(
-            "lpsolver", options.get<lp::LPSolverType>("lpsolver"));
+        const plugins::Options &opts, const utils::Context &) const override {
+        plugins::Options lm_factory_options;
+        lm_factory_options.set<shared_ptr<LandmarkFactory>>(
+            "lm_factory", opts.get<shared_ptr<LandmarkFactory>>("lm_factory"));
         return plugins::make_shared_from_arg_tuples<LandmarkCostPartitioningHeuristic>(
-            lmcp_options,
-            get_landmark_heuristic_arguments_from_options(options));
+            lm_factory_options,
+            get_landmark_heuristic_arguments_from_options(opts),
+            opts.get<CostPartitioningMethod>("cost_partitioning"),
+            opts.get<bool>("alm"),
+            lp::get_lp_solver_arguments_from_options(opts)
+            );
     }
 };
 

src/search/landmarks/landmark_cost_partitioning_heuristic.h

@@ -2,6 +2,7 @@
 #define LANDMARKS_LANDMARK_COST_PARTITIONING_HEURISTIC_H
 
 #include "landmark_heuristic.h"
+#include "../lp/lp_solver.h"
 
 namespace landmarks {
 class CostPartitioningAlgorithm;
@@ -14,8 +15,10 @@ enum class CostPartitioningMethod {
 class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic {
     std::unique_ptr<CostPartitioningAlgorithm> cost_partitioning_algorithm;
 
-    void check_unsupported_features(const plugins::Options &opts);
-    void set_cost_partitioning_algorithm(const plugins::Options &opts);
+    void check_unsupported_features(const plugins::Options &opts); // TODO issue1082 this needs Options to construct the lm_factory later.
+    void set_cost_partitioning_algorithm(CostPartitioningMethod cost_partitioning,
+                                         lp::LPSolverType lpsolver,
+                                         bool alm);
 
     int get_heuristic_value(const State &ancestor_state) override;
 public:
@@ -26,14 +29,21 @@ class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic {
        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 &options,
         bool use_preferred_operators,
+        bool prog_goal,
+        bool prog_gn,
+        bool prog_r,
         const std::shared_ptr<AbstractTask> &transform,
         bool cache_estimates,
         const std::string &description,
-        utils::Verbosity verbosity);
+        utils::Verbosity verbosity,
+        CostPartitioningMethod cost_partitioning,
+        bool alm,
+        lp::LPSolverType lpsolver);
 
     virtual bool dead_ends_are_reliable() const override;
 };

src/search/landmarks/landmark_heuristic.cc

@@ -24,7 +24,11 @@ LandmarkHeuristic::LandmarkHeuristic(
       successor_generator(nullptr) {
 }
 
-void LandmarkHeuristic::initialize(const plugins::Options &initialize_options) {
+void LandmarkHeuristic::initialize(
+    const plugins::Options &lm_factory_option,
+    bool prog_goal,
+    bool prog_gn,
+    bool prog_r) {
     /*
       Actually, we should test if this is the root task or a
       CostAdaptedTask *of the root task*, but there is currently no good
@@ -38,12 +42,12 @@ void LandmarkHeuristic::initialize(const plugins::Options &initialize_options) {
         utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED);
     }
 
-    compute_landmark_graph(initialize_options);
+    compute_landmark_graph(lm_factory_option);
     lm_status_manager = utils::make_unique_ptr<LandmarkStatusManager>(
         *lm_graph,
-        initialize_options.get<bool>("prog_goal"),
-        initialize_options.get<bool>("prog_gn"),
-        initialize_options.get<bool>("prog_r"));
+        prog_goal,
+        prog_gn,
+        prog_r);
 
     initial_landmark_graph_has_cycle_of_natural_orderings =
         landmark_graph_has_cycle_of_natural_orderings();
@@ -96,14 +100,14 @@ bool LandmarkHeuristic::depth_first_search_for_cycle_of_natural_orderings(
     return false;
 }
 
-void LandmarkHeuristic::compute_landmark_graph(const plugins::Options &opts) {
+void LandmarkHeuristic::compute_landmark_graph(const plugins::Options &lm_factory_option) {
     utils::Timer lm_graph_timer;
     if (log.is_at_least_normal()) {
         log << "Generating landmark graph..." << endl;
     }
 
     shared_ptr<LandmarkFactory> lm_graph_factory =
-        opts.get<shared_ptr<LandmarkFactory>>("lm_factory");
+        lm_factory_option.get<shared_ptr<LandmarkFactory>>("lm_factory");
     lm_graph = lm_graph_factory->compute_lm_graph(task);
     assert(lm_graph_factory->achievers_are_calculated());
 
@@ -233,29 +237,20 @@ void add_landmark_heuristic_options_to_feature(plugins::Feature &feature,
                               "yes (if enabled; see ``pref`` option)");
 }
 
-tuple<bool, shared_ptr<AbstractTask>, bool, string, utils::Verbosity>
+tuple<bool, bool, bool, bool, shared_ptr<AbstractTask>, bool, string, utils::Verbosity>
 get_landmark_heuristic_arguments_from_options(const plugins::Options &options) {
     return tuple_cat(
-        make_tuple(options.get<bool>("pref")),
+        make_tuple(
+            options.get<bool>("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<bool>("prog_goal"),
+            options.get<bool>("prog_gn"),
+            options.get<bool>("prog_r")
+            ),
         Heuristic::get_heuristic_arguments_from_options(options));
 }
-
-/*
-  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.
-*/
-plugins::Options collect_landmark_heuristic_options(
-    const plugins::Options &options) {
-    plugins::Options lm_options;
-    lm_options.set<shared_ptr<LandmarkFactory>>(
-        "lm_factory", options.get<shared_ptr<LandmarkFactory>>("lm_factory"));
-    lm_options.set<bool>("prog_goal", options.get<bool>("prog_goal"));
-    lm_options.set<bool>("prog_gn", options.get<bool>("prog_gn"));
-    lm_options.set<bool>("prog_r", options.get<bool>("prog_r"));
-    return lm_options;
-}
 }

src/search/landmarks/landmark_heuristic.h

@@ -31,8 +31,10 @@ class LandmarkHeuristic : public Heuristic {
     std::unique_ptr<LandmarkStatusManager> lm_status_manager;
     std::unique_ptr<successor_generator::SuccessorGenerator> successor_generator;
 
-    void initialize(const plugins::Options &initialize_options);
-    void compute_landmark_graph(const plugins::Options &opts);
+    // 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);
 
     virtual int get_heuristic_value(const State &ancestor_state) = 0;
 
@@ -59,9 +61,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::shared_ptr<AbstractTask>, bool, std::string, utils::Verbosity> get_landmark_heuristic_arguments_from_options(
-    const plugins::Options &options);
-extern plugins::Options collect_landmark_heuristic_options(
+extern std::tuple<bool, bool, bool, bool, std::shared_ptr<AbstractTask>, bool, std::string, utils::Verbosity> get_landmark_heuristic_arguments_from_options(
     const plugins::Options &options);
 }
 

src/search/landmarks/landmark_sum_heuristic.cc

@@ -31,8 +31,11 @@ static bool are_dead_ends_reliable(
 }
 
 LandmarkSumHeuristic::LandmarkSumHeuristic(
-    const plugins::Options &options,
+    const plugins::Options &lm_factory_options,
     bool use_preferred_operators,
+    bool prog_goal,
+    bool prog_gn,
+    bool prog_r,
     const shared_ptr<AbstractTask> &transform,
     bool cache_estimates,
     const string &description,
@@ -41,12 +44,12 @@ LandmarkSumHeuristic::LandmarkSumHeuristic(
                         description, verbosity),
       dead_ends_reliable(
           are_dead_ends_reliable(
-              options.get<shared_ptr<LandmarkFactory>>("lm_factory"),
+              lm_factory_options.get<shared_ptr<LandmarkFactory>>("lm_factory"),
               task_proxy)) {
     if (log.is_at_least_normal()) {
         log << "Initializing landmark sum heuristic..." << endl;
     }
-    initialize(options);
+    initialize(lm_factory_options, prog_goal, prog_gn, prog_r);
     compute_landmark_costs();
 }
 
@@ -198,12 +201,13 @@ class LandmarkSumHeuristicFeature : public plugins::TypedFeature<Evaluator, Land
     }
 
     virtual shared_ptr<LandmarkSumHeuristic> create_component(
-        const plugins::Options &options, const utils::Context &) const override {
-        plugins::Options lm_options =
-            collect_landmark_heuristic_options(options);
+        const plugins::Options &opts, const utils::Context &) const override {
+        plugins::Options lm_factory_options;
+        lm_factory_options.set<shared_ptr<LandmarkFactory>>(
+            "lm_factory", opts.get<shared_ptr<LandmarkFactory>>("lm_factory"));
         return plugins::make_shared_from_arg_tuples<LandmarkSumHeuristic>(
-            lm_options,
-            get_landmark_heuristic_arguments_from_options(options));
+            lm_factory_options,
+            get_landmark_heuristic_arguments_from_options(opts));
     }
 };
 

src/search/landmarks/landmark_sum_heuristic.h

@@ -24,8 +24,11 @@ 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 &options,
+    LandmarkSumHeuristic(const plugins::Options &lm_factory_options,
                          bool use_preferred_operators,
+                         bool prog_goal,
+                         bool prog_gn,
+                         bool prog_r,
                          const std::shared_ptr<AbstractTask> &transform,
                          bool cache_estimates,
                          const std::string &description,