From 10068a089728f58675a1c160f68cb0cf8e588282 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Tue, 21 Jul 2026 19:58:51 +0200 Subject: [PATCH 01/29] evaluator to handle default value axioms --- src/search/CMakeLists.txt | 10 ++ .../cartesian_abstractions/split_selector.cc | 10 +- .../subtask_generators.cc | 10 +- .../default_value_axioms_evaluator.cc | 115 ++++++++++++++++++ .../default_value_axioms_evaluator.h | 60 +++++++++ src/search/heuristics/additive_heuristic.cc | 18 +-- src/search/heuristics/additive_heuristic.h | 3 +- src/search/heuristics/cea_heuristic.cc | 20 +-- src/search/heuristics/cea_heuristic.h | 3 +- src/search/heuristics/cg_heuristic.cc | 21 ++-- src/search/heuristics/cg_heuristic.h | 5 +- src/search/heuristics/ff_heuristic.cc | 17 +-- src/search/heuristics/ff_heuristic.h | 3 +- src/search/heuristics/max_heuristic.cc | 18 +-- src/search/heuristics/max_heuristic.h | 3 +- src/search/heuristics/relaxation_heuristic.cc | 9 +- src/search/heuristics/relaxation_heuristic.h | 3 +- .../landmarks/landmark_sum_heuristic.cc | 18 +-- src/search/landmarks/landmark_sum_heuristic.h | 3 +- 19 files changed, 271 insertions(+), 78 deletions(-) create mode 100644 src/search/evaluators/default_value_axioms_evaluator.cc create mode 100644 src/search/evaluators/default_value_axioms_evaluator.h diff --git a/src/search/CMakeLists.txt b/src/search/CMakeLists.txt index 701b6ab9ca..efe989c957 100644 --- a/src/search/CMakeLists.txt +++ b/src/search/CMakeLists.txt @@ -348,6 +348,16 @@ create_fast_downward_library( DEPENDENCY_ONLY ) +create_fast_downward_library( + NAME default_value_axioms_evaluator + HELP "Evaluator modifying axioms to handle negative values" + SOURCES + evaluators/default_value_axioms_evaluator + DEPENDS + core_tasks + task_properties +) + create_fast_downward_library( NAME modify_costs_evaluator HELP "Evaluator modifying the costs" diff --git a/src/search/cartesian_abstractions/split_selector.cc b/src/search/cartesian_abstractions/split_selector.cc index 75cc326b77..7a484b3451 100644 --- a/src/search/cartesian_abstractions/split_selector.cc +++ b/src/search/cartesian_abstractions/split_selector.cc @@ -18,9 +18,15 @@ SplitSelector::SplitSelector( const shared_ptr &task, PickSplit pick) : task(task), task_proxy(*task), pick(pick) { if (pick == PickSplit::MIN_HADD || pick == PickSplit::MAX_HADD) { + /* + issue1208: normally hadd would be wrapped inside a task transformation + for axioms. This is not necessary here, as the heuristic using this + class does not support axioms anyway. If that ever changes, this part + would have to be adapted as well. + */ additive_heuristic = make_unique( - task, tasks::AxiomHandlingType::APPROXIMATE_NEGATIVE, false, - "h^add within CEGAR abstractions", utils::Verbosity::SILENT); + task, false, "h^add within CEGAR abstractions", + utils::Verbosity::SILENT); additive_heuristic->compute_heuristic_for_cegar( task_proxy.get_initial_state()); } diff --git a/src/search/cartesian_abstractions/subtask_generators.cc b/src/search/cartesian_abstractions/subtask_generators.cc index 6344315a01..5b06c0f53b 100644 --- a/src/search/cartesian_abstractions/subtask_generators.cc +++ b/src/search/cartesian_abstractions/subtask_generators.cc @@ -34,9 +34,15 @@ class SortFactsByIncreasingHaddValues { public: explicit SortFactsByIncreasingHaddValues( const shared_ptr &task) + /* + issue1208: normally hadd would be wrapped inside a task transformation + for axioms. This is not necessary here, as the heuristic using this + class does not support axioms anyway. If that ever changes, this part + would have to be adapted as well. + */ : hadd(make_unique( - task, tasks::AxiomHandlingType::APPROXIMATE_NEGATIVE, false, - "h^add within CEGAR abstractions", utils::Verbosity::SILENT)) { + task, false, "h^add within CEGAR abstractions", + utils::Verbosity::SILENT)) { TaskProxy task_proxy(*task); hadd->compute_heuristic_for_cegar(task_proxy.get_initial_state()); } diff --git a/src/search/evaluators/default_value_axioms_evaluator.cc b/src/search/evaluators/default_value_axioms_evaluator.cc new file mode 100644 index 0000000000..20e12963fd --- /dev/null +++ b/src/search/evaluators/default_value_axioms_evaluator.cc @@ -0,0 +1,115 @@ +#include "default_value_axioms_evaluator.h" + +#include "../evaluation_context.h" + +#include "../task_utils/task_properties.h" + +#include + +using namespace std; + +namespace default_value_axioms_evaluator { +DefaultValueAxiomsEvaluator::DefaultValueAxiomsEvaluator( + const shared_ptr &task, const shared_ptr &nested, + bool use_for_reporting_minima, bool use_for_boosting, + bool use_for_counting_evaluations, const string &description, + utils::Verbosity verbosity) + : Evaluator( + task, use_for_reporting_minima, use_for_boosting, + use_for_counting_evaluations, description, verbosity), + nested(nested) { +} + +bool DefaultValueAxiomsEvaluator::dead_ends_are_reliable() const { + return nested->dead_ends_are_reliable(); +} + +void DefaultValueAxiomsEvaluator::get_path_dependent_evaluators( + set &evals) { + nested->get_path_dependent_evaluators(evals); +} + +void DefaultValueAxiomsEvaluator::notify_initial_state( + const State &initial_state) { + /* + TODO issue1208: Once we remove the task transformation code from + heuristics, we might have to transform the task here. While the state data + doesn't change, the State class currently references the task it belongs + to, and `nested` uses a different task. + */ + nested->notify_initial_state(initial_state); +} + +void DefaultValueAxiomsEvaluator::notify_state_transition( + const State &parent_state, OperatorID op_id, const State &state) { + // TODO issue1208: see above + nested->notify_state_transition(parent_state, op_id, state); +} + +EvaluationResult DefaultValueAxiomsEvaluator::compute_result( + EvaluationContext &eval_context) { + // TODO issue1208: see above (in particular, eval_context is specific to a + // state) + return nested->compute_result(eval_context); +} + +bool DefaultValueAxiomsEvaluator::does_cache_estimates() const { + return nested->does_cache_estimates(); +} + +bool DefaultValueAxiomsEvaluator::is_estimate_cached(const State &state) const { + // TODO issue1208: see above + return nested->is_estimate_cached(state); +} + +int DefaultValueAxiomsEvaluator::get_cached_estimate(const State &state) const { + // TODO issue1208: see above + return nested->get_cached_estimate(state); +} + +shared_ptr +TaskIndependentDefaultValueAxiomsEvaluator::create_task_specific_component( + const shared_ptr &task) const { + TaskProxy proxy(*task); + if (task_properties::has_axioms(proxy)) { + shared_ptr axioms_task = + make_shared(task, axioms); + shared_ptr eval = nested->bind_task(axioms_task); + return make_shared( + task, eval, use_for_reporting_minima, use_for_boosting, + use_for_counting_evaluations, description, verbosity); + } else { + /* + If the task has no axioms, there is no need to wrap the evaluator + and we can bind the unmodified task to the nested evaluator directly. + */ + return nested->bind_task(task); + } +} + +TaskIndependentDefaultValueAxiomsEvaluator:: + TaskIndependentDefaultValueAxiomsEvaluator( + shared_ptr nested, + tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, + bool use_for_boosting, bool use_for_counting_evaluations, + const std::string &description, utils::Verbosity verbosity) + : nested(move(nested)), + axioms(axioms), + use_for_reporting_minima(use_for_reporting_minima), + use_for_boosting(use_for_boosting), + use_for_counting_evaluations(use_for_counting_evaluations), + description(description), + verbosity(verbosity) { +} + +shared_ptr wrap_in_default_axiom_evaluator( + const shared_ptr &eval, + const plugins::Options &opts) { + return components::make_shared_from_arg_tuples< + default_value_axioms_evaluator:: + TaskIndependentDefaultValueAxiomsEvaluator>( + eval, tasks::get_axioms_arguments_from_options(opts), true, true, true, + get_evaluator_arguments_from_options(opts)); +} + +} diff --git a/src/search/evaluators/default_value_axioms_evaluator.h b/src/search/evaluators/default_value_axioms_evaluator.h new file mode 100644 index 0000000000..7c8eb17a76 --- /dev/null +++ b/src/search/evaluators/default_value_axioms_evaluator.h @@ -0,0 +1,60 @@ +#ifndef EVALUATORS_DEFAULT_VALUE_AXIOMS_EVALUATOR_H +#define EVALUATORS_DEFAULT_VALUE_AXIOMS_EVALUATOR_H + +#include "../component.h" +#include "../evaluator.h" + +#include "../tasks/default_value_axioms_task.h" + +#include + +namespace default_value_axioms_evaluator { +class DefaultValueAxiomsEvaluator : public Evaluator { + std::shared_ptr nested; +public: + DefaultValueAxiomsEvaluator( + const std::shared_ptr &task, + const std::shared_ptr &nested, bool use_for_reporting_minima, + bool use_for_boosting, bool use_for_counting_evaluations, + const std::string &description, utils::Verbosity verbosity); + + virtual bool dead_ends_are_reliable() const override; + virtual void get_path_dependent_evaluators( + std::set &evals) override; + virtual void notify_initial_state(const State &initial_state) override; + virtual void notify_state_transition( + const State &parent_state, OperatorID op_id, + const State &state) override; + virtual EvaluationResult compute_result( + EvaluationContext &eval_context) override; + virtual bool does_cache_estimates() const override; + virtual bool is_estimate_cached(const State &state) const override; + virtual int get_cached_estimate(const State &state) const override; +}; + +class TaskIndependentDefaultValueAxiomsEvaluator + : public TaskIndependentEvaluator { + std::shared_ptr nested; + tasks::AxiomHandlingType axioms; + bool use_for_reporting_minima; + bool use_for_boosting; + bool use_for_counting_evaluations; + const std::string description; + utils::Verbosity verbosity; + + virtual std::shared_ptr create_task_specific_component( + const std::shared_ptr &task) const override; +public: + TaskIndependentDefaultValueAxiomsEvaluator( + std::shared_ptr nested, + tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, + bool use_for_boosting, bool use_for_counting_evaluations, + const std::string &description, utils::Verbosity verbosity); +}; + +std::shared_ptr wrap_in_default_axiom_evaluator( + const std::shared_ptr &eval, + const plugins::Options &opts); +} + +#endif diff --git a/src/search/heuristics/additive_heuristic.cc b/src/search/heuristics/additive_heuristic.cc index 48c0d411e0..23bc836004 100644 --- a/src/search/heuristics/additive_heuristic.cc +++ b/src/search/heuristics/additive_heuristic.cc @@ -1,5 +1,6 @@ #include "additive_heuristic.h" +#include "../evaluators/default_value_axioms_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" @@ -13,10 +14,9 @@ namespace additive_heuristic { const int AdditiveHeuristic::MAX_COST_VALUE; AdditiveHeuristic::AdditiveHeuristic( - const shared_ptr &task, tasks::AxiomHandlingType axioms, - bool cache_estimates, const string &description, utils::Verbosity verbosity) - : RelaxationHeuristic( - task, axioms, cache_estimates, description, verbosity), + const shared_ptr &task, bool cache_estimates, + const string &description, utils::Verbosity verbosity) + : RelaxationHeuristic(task, cache_estimates, description, verbosity), did_write_overflow_warning(false) { if (log.is_at_least_normal()) { log << "Initializing additive heuristic..." << endl; @@ -166,10 +166,12 @@ class AdditiveHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return components::make_auto_task_independent_component< - AdditiveHeuristic, Evaluator>( - relaxation_heuristic:: - get_relaxation_heuristic_arguments_from_options(opts)); + shared_ptr eval = + components::make_auto_task_independent_component< + AdditiveHeuristic, Evaluator>( + get_heuristic_arguments_from_options(opts)); + return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + eval, opts); } }; diff --git a/src/search/heuristics/additive_heuristic.h b/src/search/heuristics/additive_heuristic.h index 372275f0b7..88fe2ff8b4 100644 --- a/src/search/heuristics/additive_heuristic.h +++ b/src/search/heuristics/additive_heuristic.h @@ -66,8 +66,7 @@ class AdditiveHeuristic : public relaxation_heuristic::RelaxationHeuristic { int compute_add_and_ff(const State &state); public: AdditiveHeuristic( - const std::shared_ptr &task, - tasks::AxiomHandlingType axioms, bool cache_estimates, + const std::shared_ptr &task, bool cache_estimates, const std::string &description, utils::Verbosity verbosity); /* diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index 6db48f065b..f04f5b07a6 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -2,6 +2,7 @@ #include "domain_transition_graph.h" +#include "../evaluators/default_value_axioms_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" @@ -411,12 +412,9 @@ int ContextEnhancedAdditiveHeuristic::compute_heuristic( } ContextEnhancedAdditiveHeuristic::ContextEnhancedAdditiveHeuristic( - const shared_ptr &task, tasks::AxiomHandlingType axioms, - bool cache_estimates, const string &description, utils::Verbosity verbosity) - : Heuristic( - // issue1208 move this transformation to task-independent level? - tasks::get_default_value_axioms_task_if_needed(task, axioms), - cache_estimates, description, verbosity), + const shared_ptr &task, bool cache_estimates, + const string &description, utils::Verbosity verbosity) + : Heuristic(task, cache_estimates, description, verbosity), min_action_cost(task_properties::get_min_operator_cost(task_proxy)) { if (log.is_at_least_normal()) { log << "Initializing context-enhanced additive heuristic..." << endl; @@ -471,10 +469,12 @@ class ContextEnhancedAdditiveHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return components::make_auto_task_independent_component< - ContextEnhancedAdditiveHeuristic, Evaluator>( - tasks::get_axioms_arguments_from_options(opts), - get_heuristic_arguments_from_options(opts)); + shared_ptr eval = + components::make_auto_task_independent_component< + ContextEnhancedAdditiveHeuristic, Evaluator>( + get_heuristic_arguments_from_options(opts)); + return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + eval, opts); } }; diff --git a/src/search/heuristics/cea_heuristic.h b/src/search/heuristics/cea_heuristic.h index cee22659ba..f7c37bd2b8 100644 --- a/src/search/heuristics/cea_heuristic.h +++ b/src/search/heuristics/cea_heuristic.h @@ -54,8 +54,7 @@ class ContextEnhancedAdditiveHeuristic : public Heuristic { virtual int compute_heuristic(const State &ancestor_state) override; public: ContextEnhancedAdditiveHeuristic( - const std::shared_ptr &task, - tasks::AxiomHandlingType axioms, bool cache_estimates, + const std::shared_ptr &task, bool cache_estimates, const std::string &description, utils::Verbosity verbosity); virtual ~ContextEnhancedAdditiveHeuristic() override; diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index 59af18192c..42f05c1047 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -3,6 +3,7 @@ #include "cg_cache.h" #include "domain_transition_graph.h" +#include "../evaluators/default_value_axioms_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" @@ -18,12 +19,8 @@ using namespace domain_transition_graph; namespace cg_heuristic { CGHeuristic::CGHeuristic( const shared_ptr &task, int max_cache_size, - tasks::AxiomHandlingType axioms, bool cache_estimates, - const string &description, utils::Verbosity verbosity) - : Heuristic( - // issue1208 move this transformation to task-independent level? - tasks::get_default_value_axioms_task_if_needed(task, axioms), - cache_estimates, description, verbosity), + bool cache_estimates, const string &description, utils::Verbosity verbosity) + : Heuristic(task, cache_estimates, description, verbosity), helpful_transition_extraction_counter(0), min_action_cost(task_properties::get_min_operator_cost(task_proxy)) { if (log.is_at_least_normal()) { @@ -311,11 +308,13 @@ class CGHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return components::make_auto_task_independent_component< - CGHeuristic, Evaluator>( - opts.get("max_cache_size"), - tasks::get_axioms_arguments_from_options(opts), - get_heuristic_arguments_from_options(opts)); + shared_ptr eval = + components::make_auto_task_independent_component< + CGHeuristic, Evaluator>( + opts.get("max_cache_size"), + get_heuristic_arguments_from_options(opts)); + return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + eval, opts); } }; diff --git a/src/search/heuristics/cg_heuristic.h b/src/search/heuristics/cg_heuristic.h index d0d6eef972..e7a1cf653b 100644 --- a/src/search/heuristics/cg_heuristic.h +++ b/src/search/heuristics/cg_heuristic.h @@ -4,7 +4,6 @@ #include "../heuristic.h" #include "../algorithms/priority_queues.h" -#include "../tasks/default_value_axioms_task.h" #include #include @@ -43,8 +42,8 @@ class CGHeuristic : public Heuristic { public: CGHeuristic( const std::shared_ptr &task, int max_cache_size, - tasks::AxiomHandlingType axiom_hanlding, bool cache_estimates, - const std::string &description, utils::Verbosity verbosity); + bool cache_estimates, const std::string &description, + utils::Verbosity verbosity); virtual bool dead_ends_are_reliable() const override; }; } diff --git a/src/search/heuristics/ff_heuristic.cc b/src/search/heuristics/ff_heuristic.cc index 591ae6035b..bfc3da04ee 100644 --- a/src/search/heuristics/ff_heuristic.cc +++ b/src/search/heuristics/ff_heuristic.cc @@ -1,5 +1,6 @@ #include "ff_heuristic.h" +#include "../evaluators/default_value_axioms_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" @@ -11,9 +12,9 @@ using namespace std; namespace ff_heuristic { // construction and destruction FFHeuristic::FFHeuristic( - const shared_ptr &task, tasks::AxiomHandlingType axioms, - bool cache_estimates, const string &description, utils::Verbosity verbosity) - : AdditiveHeuristic(task, axioms, cache_estimates, description, verbosity), + const shared_ptr &task, bool cache_estimates, + const string &description, utils::Verbosity verbosity) + : AdditiveHeuristic(task, cache_estimates, description, verbosity), relaxed_plan(task_proxy.get_operators().size(), false) { if (log.is_at_least_normal()) { log << "Initializing FF heuristic..." << endl; @@ -90,10 +91,12 @@ class FFHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return components::make_auto_task_independent_component< - FFHeuristic, Evaluator>( - relaxation_heuristic:: - get_relaxation_heuristic_arguments_from_options(opts)); + shared_ptr eval = + components::make_auto_task_independent_component< + FFHeuristic, Evaluator>( + get_heuristic_arguments_from_options(opts)); + return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + eval, opts); } }; diff --git a/src/search/heuristics/ff_heuristic.h b/src/search/heuristics/ff_heuristic.h index 7f0ced2e64..544a2809e8 100644 --- a/src/search/heuristics/ff_heuristic.h +++ b/src/search/heuristics/ff_heuristic.h @@ -33,8 +33,7 @@ class FFHeuristic : public additive_heuristic::AdditiveHeuristic { virtual int compute_heuristic(const State &ancestor_state) override; public: FFHeuristic( - const std::shared_ptr &task, - tasks::AxiomHandlingType axioms, bool cache_estimates, + const std::shared_ptr &task, bool cache_estimates, const std::string &description, utils::Verbosity verbosity); }; } diff --git a/src/search/heuristics/max_heuristic.cc b/src/search/heuristics/max_heuristic.cc index 25668945ad..4b4405c680 100644 --- a/src/search/heuristics/max_heuristic.cc +++ b/src/search/heuristics/max_heuristic.cc @@ -1,5 +1,6 @@ #include "max_heuristic.h" +#include "../evaluators/default_value_axioms_evaluator.h" #include "../plugins/plugin.h" #include "../utils/logging.h" @@ -23,10 +24,9 @@ namespace max_heuristic { // construction and destruction HSPMaxHeuristic::HSPMaxHeuristic( - const shared_ptr &task, tasks::AxiomHandlingType axioms, - bool cache_estimates, const string &description, utils::Verbosity verbosity) - : RelaxationHeuristic( - task, axioms, cache_estimates, description, verbosity) { + const shared_ptr &task, bool cache_estimates, + const string &description, utils::Verbosity verbosity) + : RelaxationHeuristic(task, cache_estimates, description, verbosity) { if (log.is_at_least_normal()) { log << "Initializing HSP max heuristic..." << endl; } @@ -122,10 +122,12 @@ class HSPMaxHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return components::make_auto_task_independent_component< - HSPMaxHeuristic, Evaluator>( - relaxation_heuristic:: - get_relaxation_heuristic_arguments_from_options(opts)); + shared_ptr eval = + components::make_auto_task_independent_component< + HSPMaxHeuristic, Evaluator>( + get_heuristic_arguments_from_options(opts)); + return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + eval, opts); } }; diff --git a/src/search/heuristics/max_heuristic.h b/src/search/heuristics/max_heuristic.h index b0c2f6698e..44bbccd74c 100644 --- a/src/search/heuristics/max_heuristic.h +++ b/src/search/heuristics/max_heuristic.h @@ -34,8 +34,7 @@ class HSPMaxHeuristic : public relaxation_heuristic::RelaxationHeuristic { virtual int compute_heuristic(const State &ancestor_state) override; public: HSPMaxHeuristic( - const std::shared_ptr &task, - tasks::AxiomHandlingType axioms, bool cache_estimates, + const std::shared_ptr &task, bool cache_estimates, const std::string &description, utils::Verbosity verbosity); }; } diff --git a/src/search/heuristics/relaxation_heuristic.cc b/src/search/heuristics/relaxation_heuristic.cc index bfd1ead14b..799e67d57d 100644 --- a/src/search/heuristics/relaxation_heuristic.cc +++ b/src/search/heuristics/relaxation_heuristic.cc @@ -48,12 +48,9 @@ get_relaxation_heuristic_arguments_from_options(const plugins::Options &opts) { // construction and destruction RelaxationHeuristic::RelaxationHeuristic( - const shared_ptr &task, tasks::AxiomHandlingType axioms, - bool cache_estimates, const string &description, utils::Verbosity verbosity) - : Heuristic( - // issue1208 move this transformation to task-independent level? - tasks::get_default_value_axioms_task_if_needed(task, axioms), - cache_estimates, description, verbosity) { + const shared_ptr &task, bool cache_estimates, + const string &description, utils::Verbosity verbosity) + : Heuristic(task, cache_estimates, description, verbosity) { // Build propositions. propositions.resize(task_properties::get_num_facts(task_proxy)); diff --git a/src/search/heuristics/relaxation_heuristic.h b/src/search/heuristics/relaxation_heuristic.h index 5fea7d1a06..885d9219b3 100644 --- a/src/search/heuristics/relaxation_heuristic.h +++ b/src/search/heuristics/relaxation_heuristic.h @@ -112,8 +112,7 @@ class RelaxationHeuristic : public Heuristic { Proposition *get_proposition(const FactProxy &fact); public: RelaxationHeuristic( - const std::shared_ptr &task, - tasks::AxiomHandlingType axioms, bool cache_estimates, + const std::shared_ptr &task, bool cache_estimates, const std::string &description, utils::Verbosity verbosity); virtual bool dead_ends_are_reliable() const override; diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 0eb73ddda7..a724e3c3ea 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -5,6 +5,7 @@ #include "landmark_status_manager.h" #include "util.h" +#include "../evaluators/default_value_axioms_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/successor_generator.h" #include "../task_utils/task_properties.h" @@ -34,11 +35,8 @@ LandmarkSumHeuristic::LandmarkSumHeuristic( const shared_ptr &task, const shared_ptr &lm_factory, bool pref, bool prog_goal, bool prog_gn, bool prog_r, bool cache_estimates, const string &description, - utils::Verbosity verbosity, tasks::AxiomHandlingType axioms) - : LandmarkHeuristic( - // issue1208 move this transformation to task-independent level? - tasks::get_default_value_axioms_task_if_needed(task, axioms), pref, - cache_estimates, description, verbosity), + utils::Verbosity verbosity) + : LandmarkHeuristic(task, pref, cache_estimates, description, verbosity), dead_ends_reliable(are_dead_ends_reliable(lm_factory, task_proxy)) { if (log.is_at_least_normal()) { log << "Initializing landmark sum heuristic..." << endl; @@ -194,10 +192,12 @@ class LandmarkSumHeuristicFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return components::make_auto_task_independent_component< - LandmarkSumHeuristic, Evaluator>( - get_landmark_heuristic_arguments_from_options(opts), - tasks::get_axioms_arguments_from_options(opts)); + shared_ptr eval = + components::make_auto_task_independent_component< + LandmarkSumHeuristic, Evaluator>( + get_landmark_heuristic_arguments_from_options(opts)); + return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + eval, opts); } }; diff --git a/src/search/landmarks/landmark_sum_heuristic.h b/src/search/landmarks/landmark_sum_heuristic.h index 123a1408cf..615acc80e9 100644 --- a/src/search/landmarks/landmark_sum_heuristic.h +++ b/src/search/landmarks/landmark_sum_heuristic.h @@ -26,8 +26,7 @@ class LandmarkSumHeuristic : public LandmarkHeuristic { const std::shared_ptr &task, const std::shared_ptr &lm_factory, bool pref, bool prog_goal, bool prog_gn, bool prog_r, bool cache_estimates, - const std::string &description, utils::Verbosity verbosity, - tasks::AxiomHandlingType axioms); + const std::string &description, utils::Verbosity verbosity); virtual bool dead_ends_are_reliable() const override; }; From 42b2c1cbd20fb2b294e25e441b3e55e97ecdf731 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 22 Jul 2026 10:05:56 +0200 Subject: [PATCH 02/29] remove std --- src/search/evaluators/default_value_axioms_evaluator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search/evaluators/default_value_axioms_evaluator.cc b/src/search/evaluators/default_value_axioms_evaluator.cc index 20e12963fd..cade6068a5 100644 --- a/src/search/evaluators/default_value_axioms_evaluator.cc +++ b/src/search/evaluators/default_value_axioms_evaluator.cc @@ -92,7 +92,7 @@ TaskIndependentDefaultValueAxiomsEvaluator:: shared_ptr nested, tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, bool use_for_boosting, bool use_for_counting_evaluations, - const std::string &description, utils::Verbosity verbosity) + const string &description, utils::Verbosity verbosity) : nested(move(nested)), axioms(axioms), use_for_reporting_minima(use_for_reporting_minima), From cd07a8e8b23913a3603914f68942abe9e3d42671 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 22 Jul 2026 15:37:22 +0200 Subject: [PATCH 03/29] remove dead code --- src/search/tasks/default_value_axioms_task.cc | 10 ---------- src/search/tasks/default_value_axioms_task.h | 2 -- 2 files changed, 12 deletions(-) diff --git a/src/search/tasks/default_value_axioms_task.cc b/src/search/tasks/default_value_axioms_task.cc index c94b8b704e..e71e301c0c 100644 --- a/src/search/tasks/default_value_axioms_task.cc +++ b/src/search/tasks/default_value_axioms_task.cc @@ -390,16 +390,6 @@ int DefaultValueAxiomsTask::get_num_axioms() const { return parent->get_num_axioms() + default_value_axioms.size(); } -shared_ptr get_default_value_axioms_task_if_needed( - const shared_ptr &task, AxiomHandlingType axioms) { - TaskProxy proxy(*task); - if (task_properties::has_axioms(proxy)) { - return make_shared( - DefaultValueAxiomsTask(task, axioms)); - } - return task; -} - void add_axioms_option_to_feature(plugins::Feature &feature) { feature.add_option( "axioms", diff --git a/src/search/tasks/default_value_axioms_task.h b/src/search/tasks/default_value_axioms_task.h index 1b24f17f43..b1d0b7ca87 100644 --- a/src/search/tasks/default_value_axioms_task.h +++ b/src/search/tasks/default_value_axioms_task.h @@ -88,8 +88,6 @@ class DefaultValueAxiomsTask : public DelegatingTask { virtual int get_num_axioms() const override; }; -extern std::shared_ptr get_default_value_axioms_task_if_needed( - const std::shared_ptr &task, AxiomHandlingType axioms); extern void add_axioms_option_to_feature(plugins::Feature &feature); extern std::tuple get_axioms_arguments_from_options( const plugins::Options &opts); From 031299edadd68f955af11f5fdcac11c030aa2257 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 22 Jul 2026 15:54:19 +0200 Subject: [PATCH 04/29] add warnings for unsupported axioms in heuristics previously using DefaultValueAxiomsTask --- src/search/heuristics/cea_heuristic.cc | 9 +++++++++ src/search/heuristics/cg_heuristic.cc | 8 ++++++++ src/search/heuristics/relaxation_heuristic.cc | 8 ++++++++ src/search/landmarks/landmark_sum_heuristic.cc | 8 ++++++++ 4 files changed, 33 insertions(+) diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index f04f5b07a6..faf5598e45 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -419,6 +419,15 @@ ContextEnhancedAdditiveHeuristic::ContextEnhancedAdditiveHeuristic( if (log.is_at_least_normal()) { log << "Initializing context-enhanced additive heuristic..." << endl; } + if (task_properties::has_axioms(task_proxy) && + !dynamic_cast(task.get())) { + cerr << "The context-enhanced additive heuristic currently only " + "supports axioms by wrapping the task in a " + "DefaultValueAxiomsTask. This is done automatically when using " + "the class from the command line." + << endl; + utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); + } DTGFactory factory(task_proxy, true, [](int, int) { return false; }); transition_graphs = factory.build_dtgs(); diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index 42f05c1047..92213adc90 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -26,6 +26,14 @@ CGHeuristic::CGHeuristic( if (log.is_at_least_normal()) { log << "Initializing causal graph heuristic..." << endl; } + if (task_properties::has_axioms(task_proxy) && + !dynamic_cast(task.get())) { + cerr << "The causal graph heuristic currently only supports axioms by " + "wrapping the task in a DefaultValueAxiomsTask. This is done " + "automatically when using the class from the command line." + << endl; + utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); + } if (max_cache_size > 0) cache = make_unique(task_proxy, max_cache_size, log); diff --git a/src/search/heuristics/relaxation_heuristic.cc b/src/search/heuristics/relaxation_heuristic.cc index 799e67d57d..71e2254a81 100644 --- a/src/search/heuristics/relaxation_heuristic.cc +++ b/src/search/heuristics/relaxation_heuristic.cc @@ -51,6 +51,14 @@ RelaxationHeuristic::RelaxationHeuristic( const shared_ptr &task, bool cache_estimates, const string &description, utils::Verbosity verbosity) : Heuristic(task, cache_estimates, description, verbosity) { + if (task_properties::has_axioms(task_proxy) && + !dynamic_cast(task.get())) { + cerr << "Relaxation heuristics currently only supports axioms by " + "wrapping the task in a DefaultValueAxiomsTask. This is done " + "automatically when using the class from the command line." + << endl; + utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); + } // Build propositions. propositions.resize(task_properties::get_num_facts(task_proxy)); diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index a724e3c3ea..26759d1b8b 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -41,6 +41,14 @@ LandmarkSumHeuristic::LandmarkSumHeuristic( if (log.is_at_least_normal()) { log << "Initializing landmark sum heuristic..." << endl; } + if (task_properties::has_axioms(task_proxy) && + !dynamic_cast(task.get())) { + cerr << "The landmark sum heuristic currently only supports axioms by " + "wrapping the task in a DefaultValueAxiomsTask. This is done " + "automatically when using the class from the command line." + << endl; + utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); + } initialize(lm_factory, prog_goal, prog_gn, prog_r); compute_landmark_costs(); } From e21fb0da225dd5c165667b101ab58ba122add77f Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 22 Jul 2026 17:42:09 +0200 Subject: [PATCH 05/29] delegating implementation for the state registry --- src/search/search_algorithm.h | 2 +- src/search/state_id.h | 1 + src/search/state_registry.cc | 25 ++-- src/search/state_registry.h | 210 ++++++++++++++++++++++------------ 4 files changed, 153 insertions(+), 85 deletions(-) diff --git a/src/search/search_algorithm.h b/src/search/search_algorithm.h index b404906746..e5b5683411 100644 --- a/src/search/search_algorithm.h +++ b/src/search/search_algorithm.h @@ -45,7 +45,7 @@ class SearchAlgorithm : public components::TaskSpecificComponent { protected: mutable utils::LogProxy log; PlanManager plan_manager; - StateRegistry state_registry; + ExplicitStateRegistry state_registry; const successor_generator::SuccessorGenerator &successor_generator; SearchSpace search_space; SearchProgress search_progress; diff --git a/src/search/state_id.h b/src/search/state_id.h index a630b040f8..970f6f5240 100644 --- a/src/search/state_id.h +++ b/src/search/state_id.h @@ -8,6 +8,7 @@ class StateID { friend class StateRegistry; + friend class ExplicitStateRegistry; friend std::ostream &operator<<(std::ostream &os, StateID id); template friend class PerStateInformation; diff --git a/src/search/state_registry.cc b/src/search/state_registry.cc index c16af8df48..3f14e6399c 100644 --- a/src/search/state_registry.cc +++ b/src/search/state_registry.cc @@ -10,16 +10,21 @@ using namespace std; StateRegistry::StateRegistry(const TaskProxy &task_proxy) : task_proxy(task_proxy), + num_variables(task_proxy.get_variables().size()) { +} + + +ExplicitStateRegistry::ExplicitStateRegistry(const TaskProxy &task_proxy) + : StateRegistry(task_proxy), state_packer(task_properties::g_state_packers[task_proxy]), axiom_evaluator(g_axiom_evaluators[task_proxy]), - num_variables(task_proxy.get_variables().size()), state_data_pool(get_bins_per_state()), registered_states( StateIDSemanticHash(state_data_pool, get_bins_per_state()), StateIDSemanticEqual(state_data_pool, get_bins_per_state())) { } -StateID StateRegistry::insert_id_or_pop_state() { +StateID ExplicitStateRegistry::insert_id_or_pop_state() { /* Attempt to insert a StateID for the last state of state_data_pool if none is present yet. If this fails (another entry for this state @@ -37,18 +42,18 @@ StateID StateRegistry::insert_id_or_pop_state() { return StateID(result.first); } -State StateRegistry::lookup_state(StateID id) const { +State ExplicitStateRegistry::lookup_state(StateID id) const { const PackedStateBin *buffer = state_data_pool[id.value]; return task_proxy.create_state(*this, id, buffer); } -State StateRegistry::lookup_state( +State ExplicitStateRegistry::lookup_state( StateID id, vector &&state_values) const { const PackedStateBin *buffer = state_data_pool[id.value]; return task_proxy.create_state(*this, id, buffer, move(state_values)); } -const State &StateRegistry::get_initial_state() { +const State &ExplicitStateRegistry::get_initial_state() { if (!cached_initial_state) { int num_bins = get_bins_per_state(); unique_ptr buffer(new PackedStateBin[num_bins]); @@ -70,7 +75,7 @@ const State &StateRegistry::get_initial_state() { // application) // out of the StateRegistry. This could for example be done by global // functions operating on state buffers (PackedStateBin *). -State StateRegistry::get_successor_state( +State ExplicitStateRegistry::get_successor_state( const State &predecessor, const OperatorProxy &op) { assert(!op.is_axiom()); /* @@ -118,15 +123,11 @@ State StateRegistry::get_successor_state( } } -int StateRegistry::get_bins_per_state() const { +int ExplicitStateRegistry::get_bins_per_state() const { return state_packer.get_num_bins(); } -int StateRegistry::get_state_size_in_bytes() const { - return get_bins_per_state() * sizeof(PackedStateBin); -} - -void StateRegistry::print_statistics(utils::LogProxy &log) const { +void ExplicitStateRegistry::print_statistics(utils::LogProxy &log) const { log << "Number of registered states: " << size() << endl; registered_states.print_statistics(log); } diff --git a/src/search/state_registry.h b/src/search/state_registry.h index ff87119ed9..5f74324990 100644 --- a/src/search/state_registry.h +++ b/src/search/state_registry.h @@ -110,67 +110,10 @@ class IntPacker; } using PackedStateBin = int_packer::IntPacker::Bin; - class StateRegistry : public subscriber::SubscriberService { - struct StateIDSemanticHash { - const segmented_vector::SegmentedArrayVector - &state_data_pool; - int state_size; - StateIDSemanticHash( - const segmented_vector::SegmentedArrayVector - &state_data_pool, - int state_size) - : state_data_pool(state_data_pool), state_size(state_size) { - } - - int_hash_set::HashType operator()(int id) const { - const PackedStateBin *data = state_data_pool[id]; - utils::HashState hash_state; - for (int i = 0; i < state_size; ++i) { - hash_state.feed(data[i]); - } - return hash_state.get_hash32(); - } - }; - - struct StateIDSemanticEqual { - const segmented_vector::SegmentedArrayVector - &state_data_pool; - int state_size; - StateIDSemanticEqual( - const segmented_vector::SegmentedArrayVector - &state_data_pool, - int state_size) - : state_data_pool(state_data_pool), state_size(state_size) { - } - - bool operator()(int lhs, int rhs) const { - const PackedStateBin *lhs_data = state_data_pool[lhs]; - const PackedStateBin *rhs_data = state_data_pool[rhs]; - return std::equal(lhs_data, lhs_data + state_size, rhs_data); - } - }; - - /* - Hash set of StateIDs used to detect states that are already registered in - this registry and find their IDs. States are compared/hashed semantically, - i.e. the actual state data is compared, not the memory location. - */ - using StateIDSet = - int_hash_set::IntHashSet; - +protected: TaskProxy task_proxy; - const int_packer::IntPacker &state_packer; - AxiomEvaluator &axiom_evaluator; const int num_variables; - - segmented_vector::SegmentedArrayVector state_data_pool; - StateIDSet registered_states; - - std::unique_ptr cached_initial_state; - - StateID insert_id_or_pop_state(); - int get_bins_per_state() const; public: explicit StateRegistry(const TaskProxy &task_proxy); @@ -182,49 +125,44 @@ class StateRegistry : public subscriber::SubscriberService { return num_variables; } - const int_packer::IntPacker &get_state_packer() const { - return state_packer; - } + virtual const int_packer::IntPacker &get_state_packer() const = 0; /* Returns the state that was registered at the given ID. The ID must refer to a state in this registry. Do not mix IDs from from different registries. */ - State lookup_state(StateID id) const; + virtual State lookup_state(StateID id) const = 0; /* Like lookup_state above, but creates a state with unpacked data, moved in via state_values. It is the caller's responsibility that the unpacked data matches the state's data. */ - State lookup_state(StateID id, std::vector &&state_values) const; + virtual State lookup_state( + StateID id, std::vector &&state_values) const = 0; /* Returns a reference to the initial state and registers it if this was not done before. The result is cached internally so subsequent calls are cheap. */ - const State &get_initial_state(); + virtual const State &get_initial_state() = 0; /* Returns the state that results from applying op to predecessor and registers it if this was not done before. This is an expensive operation as it includes duplicate checking. */ - State get_successor_state( - const State &predecessor, const OperatorProxy &op); + virtual State get_successor_state( + const State &predecessor, const OperatorProxy &op) = 0; /* Returns the number of states registered so far. */ - size_t size() const { - return registered_states.size(); - } - - int get_state_size_in_bytes() const; + virtual size_t size() const = 0; - void print_statistics(utils::LogProxy &log) const; + virtual void print_statistics(utils::LogProxy &log) const = 0; class const_iterator { using iterator_category = std::forward_iterator_tag; @@ -281,4 +219,132 @@ class StateRegistry : public subscriber::SubscriberService { } }; +class ExplicitStateRegistry : public StateRegistry { + struct StateIDSemanticHash { + const segmented_vector::SegmentedArrayVector + &state_data_pool; + int state_size; + StateIDSemanticHash( + const segmented_vector::SegmentedArrayVector + &state_data_pool, + int state_size) + : state_data_pool(state_data_pool), state_size(state_size) { + } + + int_hash_set::HashType operator()(int id) const { + const PackedStateBin *data = state_data_pool[id]; + utils::HashState hash_state; + for (int i = 0; i < state_size; ++i) { + hash_state.feed(data[i]); + } + return hash_state.get_hash32(); + } + }; + + struct StateIDSemanticEqual { + const segmented_vector::SegmentedArrayVector + &state_data_pool; + int state_size; + StateIDSemanticEqual( + const segmented_vector::SegmentedArrayVector + &state_data_pool, + int state_size) + : state_data_pool(state_data_pool), state_size(state_size) { + } + + bool operator()(int lhs, int rhs) const { + const PackedStateBin *lhs_data = state_data_pool[lhs]; + const PackedStateBin *rhs_data = state_data_pool[rhs]; + return std::equal(lhs_data, lhs_data + state_size, rhs_data); + } + }; + + /* + Hash set of StateIDs used to detect states that are already registered in + this registry and find their IDs. States are compared/hashed semantically, + i.e. the actual state data is compared, not the memory location. + */ + using StateIDSet = + int_hash_set::IntHashSet; + + const int_packer::IntPacker &state_packer; + AxiomEvaluator &axiom_evaluator; + + segmented_vector::SegmentedArrayVector state_data_pool; + StateIDSet registered_states; + + std::unique_ptr cached_initial_state; + + StateID insert_id_or_pop_state(); + int get_bins_per_state() const; +public: + explicit ExplicitStateRegistry(const TaskProxy &task_proxy); + + const int_packer::IntPacker &get_state_packer() const override { + return state_packer; + } + State lookup_state(StateID id) const override; + State lookup_state( + StateID id, std::vector &&state_values) const override; + const State &get_initial_state() override; + State get_successor_state( + const State &predecessor, const OperatorProxy &op) override; + size_t size() const override { + return registered_states.size(); + } + void print_statistics(utils::LogProxy &log) const override; +}; + +class DelegatingStateRegistry : public StateRegistry { + /* + Task-transforming components that transform the task in a way that + states remain valid, can use a DelegatingStateRegistry to register + and lookup states without storing the state data themselves. + */ + const std::shared_ptr &task; + StateRegistry &nested; + std::unique_ptr cached_initial_state; + State repackage_state(const State &state) const { + return State(*task, *this, state.get_id(), state.get_buffer()); + } +public: + /* + The caller must ensure that the states stored in `nested` are compatible + with the states of `task`. + */ + DelegatingStateRegistry( + const std::shared_ptr &task, StateRegistry &nested) + : StateRegistry(TaskProxy(*task)), task(task), nested(nested) { + } + + const int_packer::IntPacker &get_state_packer() const override { + return nested.get_state_packer(); + } + State lookup_state(StateID id) const override { + return repackage_state(nested.lookup_state(id)); + } + State lookup_state( + StateID id, std::vector &&state_values) const override { + return repackage_state(nested.lookup_state(id, move(state_values))); + } + const State &get_initial_state() override { + if (!cached_initial_state) { + State init = nested.get_initial_state(); + cached_initial_state = std::make_unique( + *task, *this, init.get_id(), init.get_buffer()); + } + return *cached_initial_state; + } + State get_successor_state( + const State &predecessor, const OperatorProxy &op) override { + return repackage_state(nested.get_successor_state(predecessor, op)); + } + size_t size() const override { + return nested.size(); + } + void print_statistics(utils::LogProxy &log) const override { + nested.print_statistics(log); + } +}; + #endif From 3f42d5ec5d3ab52b354018468c9dd6830b8d86ea Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 23 Jul 2026 13:47:30 +0200 Subject: [PATCH 06/29] hacked partial implementation for discussion --- src/search/evaluation_context.cc | 7 ++++ src/search/evaluation_context.h | 9 +++- .../default_value_axioms_evaluator.cc | 41 +++++++++++-------- .../default_value_axioms_evaluator.h | 3 ++ src/search/state_registry.h | 27 +++++------- 5 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/search/evaluation_context.cc b/src/search/evaluation_context.cc index 9103ea2e05..13a4f32da2 100644 --- a/src/search/evaluation_context.cc +++ b/src/search/evaluation_context.cc @@ -27,6 +27,13 @@ EvaluationContext::EvaluationContext( calculate_preferred) { } +EvaluationContext::EvaluationContext( + const EvaluationContext &other, const State &state) + : EvaluationContext( + other.cache, state, other.g_value, other.preferred, other.statistics, + other.calculate_preferred) { +} + EvaluationContext::EvaluationContext( const State &state, int g_value, bool is_preferred, SearchStatistics *statistics, bool calculate_preferred) diff --git a/src/search/evaluation_context.h b/src/search/evaluation_context.h index acb8777cca..550ebdb553 100644 --- a/src/search/evaluation_context.h +++ b/src/search/evaluation_context.h @@ -59,11 +59,16 @@ class EvaluationContext { Copy existing heuristic cache and use it to look up heuristic values. Used for example by lazy search. - TODO: Can we reuse caches? Can we move them instead of copying them? - */ + TODO: Can we reuse caches? Can we move them instead of copying them? + */ EvaluationContext( const EvaluationContext &other, int g_value, bool is_preferred, SearchStatistics *statistics, bool calculate_preferred = false); + /* + Copy existing heuristic cache and use it to look up heuristic values. + Used by transforming evaluators. + */ + EvaluationContext(const EvaluationContext &other, const State &state); /* Create new heuristic cache for caching heuristic values. Used for example by eager search. diff --git a/src/search/evaluators/default_value_axioms_evaluator.cc b/src/search/evaluators/default_value_axioms_evaluator.cc index cade6068a5..a3e614c6a1 100644 --- a/src/search/evaluators/default_value_axioms_evaluator.cc +++ b/src/search/evaluators/default_value_axioms_evaluator.cc @@ -20,6 +20,20 @@ DefaultValueAxiomsEvaluator::DefaultValueAxiomsEvaluator( nested(nested) { } +State DefaultValueAxiomsEvaluator::repackage_state(const State &state) const { + const StateRegistry *existing_state_registry = state.get_registry(); + if (!state_registry) { + // issue1208: avoid const cast? + state_registry = make_unique( + task, const_cast(*existing_state_registry)); + } else { + // issue1208: verify that state_registry.nested == + // existing_state_registry? or use a map from registries to delegating + // registries? + } + return state_registry->repackage_state(state); +} + bool DefaultValueAxiomsEvaluator::dead_ends_are_reliable() const { return nested->dead_ends_are_reliable(); } @@ -31,26 +45,23 @@ void DefaultValueAxiomsEvaluator::get_path_dependent_evaluators( void DefaultValueAxiomsEvaluator::notify_initial_state( const State &initial_state) { - /* - TODO issue1208: Once we remove the task transformation code from - heuristics, we might have to transform the task here. While the state data - doesn't change, the State class currently references the task it belongs - to, and `nested` uses a different task. - */ - nested->notify_initial_state(initial_state); + nested->notify_initial_state(repackage_state(initial_state)); } void DefaultValueAxiomsEvaluator::notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) { - // TODO issue1208: see above - nested->notify_state_transition(parent_state, op_id, state); + nested->notify_state_transition( + repackage_state(parent_state), op_id, repackage_state(state)); } EvaluationResult DefaultValueAxiomsEvaluator::compute_result( EvaluationContext &eval_context) { - // TODO issue1208: see above (in particular, eval_context is specific to a - // state) - return nested->compute_result(eval_context); + State translated_state = repackage_state(eval_context.get_state()); + EvaluationContext translated_context(eval_context, translated_state); + EvaluationResult result = nested->compute_result(translated_context); + return result; + // TODO do we need something like this? + //return eval_context.add_to_cache(nested.get(), move(result)); } bool DefaultValueAxiomsEvaluator::does_cache_estimates() const { @@ -58,13 +69,11 @@ bool DefaultValueAxiomsEvaluator::does_cache_estimates() const { } bool DefaultValueAxiomsEvaluator::is_estimate_cached(const State &state) const { - // TODO issue1208: see above - return nested->is_estimate_cached(state); + return nested->is_estimate_cached(repackage_state(state)); } int DefaultValueAxiomsEvaluator::get_cached_estimate(const State &state) const { - // TODO issue1208: see above - return nested->get_cached_estimate(state); + return nested->get_cached_estimate(repackage_state(state)); } shared_ptr diff --git a/src/search/evaluators/default_value_axioms_evaluator.h b/src/search/evaluators/default_value_axioms_evaluator.h index 7c8eb17a76..56f4aeedec 100644 --- a/src/search/evaluators/default_value_axioms_evaluator.h +++ b/src/search/evaluators/default_value_axioms_evaluator.h @@ -3,6 +3,7 @@ #include "../component.h" #include "../evaluator.h" +#include "../state_registry.h" #include "../tasks/default_value_axioms_task.h" @@ -11,6 +12,8 @@ namespace default_value_axioms_evaluator { class DefaultValueAxiomsEvaluator : public Evaluator { std::shared_ptr nested; + mutable std::unique_ptr state_registry; + State repackage_state(const State &state) const; public: DefaultValueAxiomsEvaluator( const std::shared_ptr &task, diff --git a/src/search/state_registry.h b/src/search/state_registry.h index 5f74324990..e303938d4e 100644 --- a/src/search/state_registry.h +++ b/src/search/state_registry.h @@ -134,14 +134,6 @@ class StateRegistry : public subscriber::SubscriberService { */ virtual State lookup_state(StateID id) const = 0; - /* - Like lookup_state above, but creates a state with unpacked data, - moved in via state_values. It is the caller's responsibility that - the unpacked data matches the state's data. - */ - virtual State lookup_state( - StateID id, std::vector &&state_values) const = 0; - /* Returns a reference to the initial state and registers it if this was not done before. The result is cached internally so subsequent calls are @@ -277,6 +269,13 @@ class ExplicitStateRegistry : public StateRegistry { StateID insert_id_or_pop_state(); int get_bins_per_state() const; + + /* + Like lookup_state(id), but creates a state with unpacked data, + moved in via state_values. It is the caller's responsibility that + the unpacked data matches the state's data. + */ + State lookup_state(StateID id, std::vector &&state_values) const; public: explicit ExplicitStateRegistry(const TaskProxy &task_proxy); @@ -284,8 +283,6 @@ class ExplicitStateRegistry : public StateRegistry { return state_packer; } State lookup_state(StateID id) const override; - State lookup_state( - StateID id, std::vector &&state_values) const override; const State &get_initial_state() override; State get_successor_state( const State &predecessor, const OperatorProxy &op) override; @@ -304,9 +301,6 @@ class DelegatingStateRegistry : public StateRegistry { const std::shared_ptr &task; StateRegistry &nested; std::unique_ptr cached_initial_state; - State repackage_state(const State &state) const { - return State(*task, *this, state.get_id(), state.get_buffer()); - } public: /* The caller must ensure that the states stored in `nested` are compatible @@ -317,16 +311,15 @@ class DelegatingStateRegistry : public StateRegistry { : StateRegistry(TaskProxy(*task)), task(task), nested(nested) { } + State repackage_state(const State &state) const { + return State(*task, *this, state.get_id(), state.get_buffer()); + } const int_packer::IntPacker &get_state_packer() const override { return nested.get_state_packer(); } State lookup_state(StateID id) const override { return repackage_state(nested.lookup_state(id)); } - State lookup_state( - StateID id, std::vector &&state_values) const override { - return repackage_state(nested.lookup_state(id, move(state_values))); - } const State &get_initial_state() override { if (!cached_initial_state) { State init = nested.get_initial_state(); From 3c933eab505c92e4fd7c8e21c0d1c264cf5015de Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 10:03:18 +0200 Subject: [PATCH 07/29] code review --- .../cartesian_abstractions/split_selector.cc | 6 -- .../subtask_generators.cc | 6 -- src/search/evaluation_context.cc | 4 +- src/search/evaluation_context.h | 16 +++- .../default_value_axioms_evaluator.cc | 87 ++++++++++--------- .../default_value_axioms_evaluator.h | 18 ++-- src/search/heuristics/additive_heuristic.cc | 2 +- src/search/heuristics/cea_heuristic.cc | 2 +- src/search/heuristics/cg_heuristic.cc | 2 +- src/search/heuristics/ff_heuristic.cc | 2 +- src/search/heuristics/max_heuristic.cc | 2 +- .../landmarks/landmark_sum_heuristic.cc | 10 +-- src/search/plugins/plugin.h | 9 +- src/search/state_registry.cc | 48 +++++++++- src/search/state_registry.h | 75 +++++++--------- 15 files changed, 163 insertions(+), 126 deletions(-) diff --git a/src/search/cartesian_abstractions/split_selector.cc b/src/search/cartesian_abstractions/split_selector.cc index 7a484b3451..921e3f98ee 100644 --- a/src/search/cartesian_abstractions/split_selector.cc +++ b/src/search/cartesian_abstractions/split_selector.cc @@ -18,12 +18,6 @@ SplitSelector::SplitSelector( const shared_ptr &task, PickSplit pick) : task(task), task_proxy(*task), pick(pick) { if (pick == PickSplit::MIN_HADD || pick == PickSplit::MAX_HADD) { - /* - issue1208: normally hadd would be wrapped inside a task transformation - for axioms. This is not necessary here, as the heuristic using this - class does not support axioms anyway. If that ever changes, this part - would have to be adapted as well. - */ additive_heuristic = make_unique( task, false, "h^add within CEGAR abstractions", utils::Verbosity::SILENT); diff --git a/src/search/cartesian_abstractions/subtask_generators.cc b/src/search/cartesian_abstractions/subtask_generators.cc index 5b06c0f53b..753d5cec49 100644 --- a/src/search/cartesian_abstractions/subtask_generators.cc +++ b/src/search/cartesian_abstractions/subtask_generators.cc @@ -34,12 +34,6 @@ class SortFactsByIncreasingHaddValues { public: explicit SortFactsByIncreasingHaddValues( const shared_ptr &task) - /* - issue1208: normally hadd would be wrapped inside a task transformation - for axioms. This is not necessary here, as the heuristic using this - class does not support axioms anyway. If that ever changes, this part - would have to be adapted as well. - */ : hadd(make_unique( task, false, "h^add within CEGAR abstractions", utils::Verbosity::SILENT)) { diff --git a/src/search/evaluation_context.cc b/src/search/evaluation_context.cc index 13a4f32da2..e0b5065683 100644 --- a/src/search/evaluation_context.cc +++ b/src/search/evaluation_context.cc @@ -30,8 +30,8 @@ EvaluationContext::EvaluationContext( EvaluationContext::EvaluationContext( const EvaluationContext &other, const State &state) : EvaluationContext( - other.cache, state, other.g_value, other.preferred, other.statistics, - other.calculate_preferred) { + EvaluatorCache(), state, other.g_value, other.preferred, + other.statistics, other.calculate_preferred) { } EvaluationContext::EvaluationContext( diff --git a/src/search/evaluation_context.h b/src/search/evaluation_context.h index 550ebdb553..a2d9b0649e 100644 --- a/src/search/evaluation_context.h +++ b/src/search/evaluation_context.h @@ -59,14 +59,22 @@ class EvaluationContext { Copy existing heuristic cache and use it to look up heuristic values. Used for example by lazy search. - TODO: Can we reuse caches? Can we move them instead of copying them? - */ + TODO: Can we reuse caches? Can we move them instead of copying them? + */ EvaluationContext( const EvaluationContext &other, int g_value, bool is_preferred, SearchStatistics *statistics, bool calculate_preferred = false); /* - Copy existing heuristic cache and use it to look up heuristic values. - Used by transforming evaluators. + Create new heuristic cache for caching heuristic values but copy other + information from the other evaluation context. This is only supposed to + be used by task-transforming evaluators that want to call nested + evaluators with a converted state, while still using the original + statistics. Starting from an empty cache is correct in this case: + all nested evaluators will use the transformed task or further + transformations of it while all other evaluators used outside of the + transforming evaluator do not have access to the transformed task. This + implies that cached values used in the scope of the transforming evaluator + are disjoint from any cached values used outside of this scope. */ EvaluationContext(const EvaluationContext &other, const State &state); /* diff --git a/src/search/evaluators/default_value_axioms_evaluator.cc b/src/search/evaluators/default_value_axioms_evaluator.cc index a3e614c6a1..f6702ab1c1 100644 --- a/src/search/evaluators/default_value_axioms_evaluator.cc +++ b/src/search/evaluators/default_value_axioms_evaluator.cc @@ -4,12 +4,10 @@ #include "../task_utils/task_properties.h" -#include - using namespace std; -namespace default_value_axioms_evaluator { -DefaultValueAxiomsEvaluator::DefaultValueAxiomsEvaluator( +namespace axiom_handling_evaluator { +AxiomHandlingEvaluator::AxiomHandlingEvaluator( const shared_ptr &task, const shared_ptr &nested, bool use_for_reporting_minima, bool use_for_boosting, bool use_for_counting_evaluations, const string &description, @@ -20,71 +18,84 @@ DefaultValueAxiomsEvaluator::DefaultValueAxiomsEvaluator( nested(nested) { } -State DefaultValueAxiomsEvaluator::repackage_state(const State &state) const { +State AxiomHandlingEvaluator::convert_state(const State &state) const { const StateRegistry *existing_state_registry = state.get_registry(); + assert(existing_state_registry); if (!state_registry) { - // issue1208: avoid const cast? + /* + issue1227: + * avoid const cast? + -> currently needed because we only get the registry from states + passed to this evaluator instead of knowing the registry in the + constructor already. We only get a const registry pointer from + the state but need it to be mutable in the delegating registry. + * avoid mutable state_registry? + -> currently this is used for the delayed initialization which can + happen inside const methods. + * avoid delayed initialization alltogether + Both of the above problems would go away if we could create the + registry in the constructor. + */ state_registry = make_unique( task, const_cast(*existing_state_registry)); } else { - // issue1208: verify that state_registry.nested == - // existing_state_registry? or use a map from registries to delegating - // registries? + state_registry->assert_nested_is(*existing_state_registry); } - return state_registry->repackage_state(state); + return state_registry->convert_state(state); } -bool DefaultValueAxiomsEvaluator::dead_ends_are_reliable() const { +bool AxiomHandlingEvaluator::dead_ends_are_reliable() const { return nested->dead_ends_are_reliable(); } -void DefaultValueAxiomsEvaluator::get_path_dependent_evaluators( +void AxiomHandlingEvaluator::get_path_dependent_evaluators( set &evals) { nested->get_path_dependent_evaluators(evals); } -void DefaultValueAxiomsEvaluator::notify_initial_state( - const State &initial_state) { - nested->notify_initial_state(repackage_state(initial_state)); +void AxiomHandlingEvaluator::notify_initial_state(const State &initial_state) { + nested->notify_initial_state(convert_state(initial_state)); } -void DefaultValueAxiomsEvaluator::notify_state_transition( +void AxiomHandlingEvaluator::notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) { nested->notify_state_transition( - repackage_state(parent_state), op_id, repackage_state(state)); + convert_state(parent_state), op_id, convert_state(state)); } -EvaluationResult DefaultValueAxiomsEvaluator::compute_result( +EvaluationResult AxiomHandlingEvaluator::compute_result( EvaluationContext &eval_context) { - State translated_state = repackage_state(eval_context.get_state()); + State translated_state = convert_state(eval_context.get_state()); EvaluationContext translated_context(eval_context, translated_state); - EvaluationResult result = nested->compute_result(translated_context); - return result; - // TODO do we need something like this? - //return eval_context.add_to_cache(nested.get(), move(result)); + /* + Note that we do not need to update the cache inside eval_context. + the call below can only set and access evaluators that use the transformed + task and users of eval_context cannot know such evaluators. + */ + return nested->compute_result(translated_context); } -bool DefaultValueAxiomsEvaluator::does_cache_estimates() const { +bool AxiomHandlingEvaluator::does_cache_estimates() const { return nested->does_cache_estimates(); } -bool DefaultValueAxiomsEvaluator::is_estimate_cached(const State &state) const { - return nested->is_estimate_cached(repackage_state(state)); +bool AxiomHandlingEvaluator::is_estimate_cached(const State &state) const { + return nested->is_estimate_cached(convert_state(state)); } -int DefaultValueAxiomsEvaluator::get_cached_estimate(const State &state) const { - return nested->get_cached_estimate(repackage_state(state)); +int AxiomHandlingEvaluator::get_cached_estimate(const State &state) const { + return nested->get_cached_estimate(convert_state(state)); } shared_ptr -TaskIndependentDefaultValueAxiomsEvaluator::create_task_specific_component( +TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( const shared_ptr &task) const { TaskProxy proxy(*task); if (task_properties::has_axioms(proxy)) { shared_ptr axioms_task = make_shared(task, axioms); shared_ptr eval = nested->bind_task(axioms_task); - return make_shared( + return make_shared( task, eval, use_for_reporting_minima, use_for_boosting, use_for_counting_evaluations, description, verbosity); } else { @@ -96,12 +107,11 @@ TaskIndependentDefaultValueAxiomsEvaluator::create_task_specific_component( } } -TaskIndependentDefaultValueAxiomsEvaluator:: - TaskIndependentDefaultValueAxiomsEvaluator( - shared_ptr nested, - tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, - bool use_for_boosting, bool use_for_counting_evaluations, - const string &description, utils::Verbosity verbosity) +TaskIndependentAxiomHandlingEvaluator::TaskIndependentAxiomHandlingEvaluator( + shared_ptr nested, + tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, + bool use_for_boosting, bool use_for_counting_evaluations, + const string &description, utils::Verbosity verbosity) : nested(move(nested)), axioms(axioms), use_for_reporting_minima(use_for_reporting_minima), @@ -111,12 +121,11 @@ TaskIndependentDefaultValueAxiomsEvaluator:: verbosity(verbosity) { } -shared_ptr wrap_in_default_axiom_evaluator( +shared_ptr wrap_in_axiom_handling_evaluator( const shared_ptr &eval, const plugins::Options &opts) { return components::make_shared_from_arg_tuples< - default_value_axioms_evaluator:: - TaskIndependentDefaultValueAxiomsEvaluator>( + axiom_handling_evaluator::TaskIndependentAxiomHandlingEvaluator>( eval, tasks::get_axioms_arguments_from_options(opts), true, true, true, get_evaluator_arguments_from_options(opts)); } diff --git a/src/search/evaluators/default_value_axioms_evaluator.h b/src/search/evaluators/default_value_axioms_evaluator.h index 56f4aeedec..9787f26579 100644 --- a/src/search/evaluators/default_value_axioms_evaluator.h +++ b/src/search/evaluators/default_value_axioms_evaluator.h @@ -1,5 +1,5 @@ -#ifndef EVALUATORS_DEFAULT_VALUE_AXIOMS_EVALUATOR_H -#define EVALUATORS_DEFAULT_VALUE_AXIOMS_EVALUATOR_H +#ifndef EVALUATORS_AXIOM_HANDLING_EVALUATOR_H +#define EVALUATORS_AXIOM_HANDLING_EVALUATOR_H #include "../component.h" #include "../evaluator.h" @@ -9,13 +9,13 @@ #include -namespace default_value_axioms_evaluator { -class DefaultValueAxiomsEvaluator : public Evaluator { +namespace axiom_handling_evaluator { +class AxiomHandlingEvaluator : public Evaluator { std::shared_ptr nested; mutable std::unique_ptr state_registry; - State repackage_state(const State &state) const; + State convert_state(const State &state) const; public: - DefaultValueAxiomsEvaluator( + AxiomHandlingEvaluator( const std::shared_ptr &task, const std::shared_ptr &nested, bool use_for_reporting_minima, bool use_for_boosting, bool use_for_counting_evaluations, @@ -35,7 +35,7 @@ class DefaultValueAxiomsEvaluator : public Evaluator { virtual int get_cached_estimate(const State &state) const override; }; -class TaskIndependentDefaultValueAxiomsEvaluator +class TaskIndependentAxiomHandlingEvaluator : public TaskIndependentEvaluator { std::shared_ptr nested; tasks::AxiomHandlingType axioms; @@ -48,14 +48,14 @@ class TaskIndependentDefaultValueAxiomsEvaluator virtual std::shared_ptr create_task_specific_component( const std::shared_ptr &task) const override; public: - TaskIndependentDefaultValueAxiomsEvaluator( + TaskIndependentAxiomHandlingEvaluator( std::shared_ptr nested, tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, bool use_for_boosting, bool use_for_counting_evaluations, const std::string &description, utils::Verbosity verbosity); }; -std::shared_ptr wrap_in_default_axiom_evaluator( +std::shared_ptr wrap_in_axiom_handling_evaluator( const std::shared_ptr &eval, const plugins::Options &opts); } diff --git a/src/search/heuristics/additive_heuristic.cc b/src/search/heuristics/additive_heuristic.cc index 23bc836004..e5c69379a6 100644 --- a/src/search/heuristics/additive_heuristic.cc +++ b/src/search/heuristics/additive_heuristic.cc @@ -170,7 +170,7 @@ class AdditiveHeuristicFeature components::make_auto_task_independent_component< AdditiveHeuristic, Evaluator>( get_heuristic_arguments_from_options(opts)); - return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + return axiom_handling_evaluator::wrap_in_axiom_handling_evaluator( eval, opts); } }; diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index faf5598e45..23384c156d 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -482,7 +482,7 @@ class ContextEnhancedAdditiveHeuristicFeature components::make_auto_task_independent_component< ContextEnhancedAdditiveHeuristic, Evaluator>( get_heuristic_arguments_from_options(opts)); - return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + return axiom_handling_evaluator::wrap_in_axiom_handling_evaluator( eval, opts); } }; diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index 92213adc90..ad4706520f 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -321,7 +321,7 @@ class CGHeuristicFeature CGHeuristic, Evaluator>( opts.get("max_cache_size"), get_heuristic_arguments_from_options(opts)); - return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + return axiom_handling_evaluator::wrap_in_axiom_handling_evaluator( eval, opts); } }; diff --git a/src/search/heuristics/ff_heuristic.cc b/src/search/heuristics/ff_heuristic.cc index bfc3da04ee..709978d1b6 100644 --- a/src/search/heuristics/ff_heuristic.cc +++ b/src/search/heuristics/ff_heuristic.cc @@ -95,7 +95,7 @@ class FFHeuristicFeature components::make_auto_task_independent_component< FFHeuristic, Evaluator>( get_heuristic_arguments_from_options(opts)); - return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + return axiom_handling_evaluator::wrap_in_axiom_handling_evaluator( eval, opts); } }; diff --git a/src/search/heuristics/max_heuristic.cc b/src/search/heuristics/max_heuristic.cc index 4b4405c680..b028324a46 100644 --- a/src/search/heuristics/max_heuristic.cc +++ b/src/search/heuristics/max_heuristic.cc @@ -126,7 +126,7 @@ class HSPMaxHeuristicFeature components::make_auto_task_independent_component< HSPMaxHeuristic, Evaluator>( get_heuristic_arguments_from_options(opts)); - return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + return axiom_handling_evaluator::wrap_in_axiom_handling_evaluator( eval, opts); } }; diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 26759d1b8b..01fed7d1b2 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -43,11 +43,9 @@ LandmarkSumHeuristic::LandmarkSumHeuristic( } if (task_properties::has_axioms(task_proxy) && !dynamic_cast(task.get())) { - cerr << "The landmark sum heuristic currently only supports axioms by " - "wrapping the task in a DefaultValueAxiomsTask. This is done " - "automatically when using the class from the command line." - << endl; - utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); + ABORT("The landmark sum heuristic currently only supports axioms by " + "wrapping the task in a DefaultValueAxiomsTask. This is done " + "automatically when using the class from the command line."); } initialize(lm_factory, prog_goal, prog_gn, prog_r); compute_landmark_costs(); @@ -204,7 +202,7 @@ class LandmarkSumHeuristicFeature components::make_auto_task_independent_component< LandmarkSumHeuristic, Evaluator>( get_landmark_heuristic_arguments_from_options(opts)); - return default_value_axioms_evaluator::wrap_in_default_axiom_evaluator( + return axiom_handling_evaluator::wrap_in_axiom_handling_evaluator( eval, opts); } }; diff --git a/src/search/plugins/plugin.h b/src/search/plugins/plugin.h index 3c1fac5b7c..598da643bc 100644 --- a/src/search/plugins/plugin.h +++ b/src/search/plugins/plugin.h @@ -137,10 +137,11 @@ class CategoryPlugin { /* TODO: Currently, we do not support variable binding of all categories, so variables can only be used for categories explicitly marked. This might - change once we remove the old task transformation code (issue1208). If all - feature types can be bound to variables, we can probably get rid of this - flag and related code in CategoryPlugin, TypedCategoryPlugin, RawRegistry, - Registry, Parser, ... + change once we remove the old task transformation code (see point about + making search algorithms reusable in issue559). If all feature types can + be bound to variables, we can probably get rid of this flag and related + code in CategoryPlugin, TypedCategoryPlugin, RawRegistry, Registry, + Parser, ... */ bool can_be_bound_to_variable; public: diff --git a/src/search/state_registry.cc b/src/search/state_registry.cc index 3f14e6399c..a92c0efda8 100644 --- a/src/search/state_registry.cc +++ b/src/search/state_registry.cc @@ -42,6 +42,10 @@ StateID ExplicitStateRegistry::insert_id_or_pop_state() { return StateID(result.first); } +const int_packer::IntPacker &ExplicitStateRegistry::get_state_packer() const { + return state_packer; +} + State ExplicitStateRegistry::lookup_state(StateID id) const { const PackedStateBin *buffer = state_data_pool[id.value]; return task_proxy.create_state(*this, id, buffer); @@ -53,7 +57,7 @@ State ExplicitStateRegistry::lookup_state( return task_proxy.create_state(*this, id, buffer, move(state_values)); } -const State &ExplicitStateRegistry::get_initial_state() { +State ExplicitStateRegistry::get_initial_state() { if (!cached_initial_state) { int num_bins = get_bins_per_state(); unique_ptr buffer(new PackedStateBin[num_bins]); @@ -66,7 +70,7 @@ const State &ExplicitStateRegistry::get_initial_state() { } state_data_pool.push_back(buffer.get()); StateID id = insert_id_or_pop_state(); - cached_initial_state = make_unique(lookup_state(id)); + cached_initial_state.emplace(lookup_state(id)); } return *cached_initial_state; } @@ -127,7 +131,47 @@ int ExplicitStateRegistry::get_bins_per_state() const { return state_packer.get_num_bins(); } +size_t ExplicitStateRegistry::size() const { + return registered_states.size(); +} + void ExplicitStateRegistry::print_statistics(utils::LogProxy &log) const { log << "Number of registered states: " << size() << endl; registered_states.print_statistics(log); } + + +DelegatingStateRegistry::DelegatingStateRegistry( + const std::shared_ptr &task, StateRegistry &nested) + : StateRegistry(TaskProxy(*task)), task(task), nested(nested) { +} + +void DelegatingStateRegistry::assert_nested_is(const StateRegistry &nested) { + assert(&nested == &this->nested); + utils::unused_variable(nested); +} + +const int_packer::IntPacker &DelegatingStateRegistry::get_state_packer() const { + return nested.get_state_packer(); +} + +State DelegatingStateRegistry::lookup_state(StateID id) const { + return repackage_state(nested.lookup_state(id)); +} + +State DelegatingStateRegistry::get_initial_state() { + return repackage_state(nested.get_initial_state()); +} + +State DelegatingStateRegistry::get_successor_state( + const State &predecessor, const OperatorProxy &op) { + return repackage_state(nested.get_successor_state(predecessor, op)); +} + +size_t DelegatingStateRegistry::size() const { + return nested.size(); +} + +void DelegatingStateRegistry::print_statistics(utils::LogProxy &log) const { + nested.print_statistics(log); +} diff --git a/src/search/state_registry.h b/src/search/state_registry.h index e303938d4e..a50b91a36a 100644 --- a/src/search/state_registry.h +++ b/src/search/state_registry.h @@ -11,7 +11,7 @@ #include "algorithms/subscriber.h" #include "utils/hash.h" -#include +#include /* Overview of classes relevant to storing and working with registered states. @@ -110,6 +110,7 @@ class IntPacker; } using PackedStateBin = int_packer::IntPacker::Bin; + class StateRegistry : public subscriber::SubscriberService { protected: TaskProxy task_proxy; @@ -125,6 +126,13 @@ class StateRegistry : public subscriber::SubscriberService { return num_variables; } + /* + issue1227: The following method should not exist in the base class. + The reason it currently exists is that State::unpack() has to know how to + interpret packed state data. Maybe this interpretation could be moved into + the registry as a callback but this might be expensive, especially with a + virtual method call. + */ virtual const int_packer::IntPacker &get_state_packer() const = 0; /* @@ -139,7 +147,7 @@ class StateRegistry : public subscriber::SubscriberService { done before. The result is cached internally so subsequent calls are cheap. */ - virtual const State &get_initial_state() = 0; + virtual State get_initial_state() = 0; /* Returns the state that results from applying op to predecessor and @@ -265,7 +273,7 @@ class ExplicitStateRegistry : public StateRegistry { segmented_vector::SegmentedArrayVector state_data_pool; StateIDSet registered_states; - std::unique_ptr cached_initial_state; + std::optional cached_initial_state; StateID insert_id_or_pop_state(); int get_bins_per_state() const; @@ -279,17 +287,13 @@ class ExplicitStateRegistry : public StateRegistry { public: explicit ExplicitStateRegistry(const TaskProxy &task_proxy); - const int_packer::IntPacker &get_state_packer() const override { - return state_packer; - } - State lookup_state(StateID id) const override; - const State &get_initial_state() override; - State get_successor_state( + virtual const int_packer::IntPacker &get_state_packer() const override; + virtual State lookup_state(StateID id) const override; + virtual State get_initial_state() override; + virtual State get_successor_state( const State &predecessor, const OperatorProxy &op) override; - size_t size() const override { - return registered_states.size(); - } - void print_statistics(utils::LogProxy &log) const override; + virtual size_t size() const override; + virtual void print_statistics(utils::LogProxy &log) const override; }; class DelegatingStateRegistry : public StateRegistry { @@ -300,44 +304,29 @@ class DelegatingStateRegistry : public StateRegistry { */ const std::shared_ptr &task; StateRegistry &nested; - std::unique_ptr cached_initial_state; + State repackage_state(const State &state) const { + return State(*task, *this, state.get_id(), state.get_buffer()); + } public: /* The caller must ensure that the states stored in `nested` are compatible with the states of `task`. */ DelegatingStateRegistry( - const std::shared_ptr &task, StateRegistry &nested) - : StateRegistry(TaskProxy(*task)), task(task), nested(nested) { - } + const std::shared_ptr &task, StateRegistry &nested); - State repackage_state(const State &state) const { - return State(*task, *this, state.get_id(), state.get_buffer()); - } - const int_packer::IntPacker &get_state_packer() const override { - return nested.get_state_packer(); - } - State lookup_state(StateID id) const override { - return repackage_state(nested.lookup_state(id)); - } - const State &get_initial_state() override { - if (!cached_initial_state) { - State init = nested.get_initial_state(); - cached_initial_state = std::make_unique( - *task, *this, init.get_id(), init.get_buffer()); - } - return *cached_initial_state; - } - State get_successor_state( - const State &predecessor, const OperatorProxy &op) override { - return repackage_state(nested.get_successor_state(predecessor, op)); - } - size_t size() const override { - return nested.size(); - } - void print_statistics(utils::LogProxy &log) const override { - nested.print_statistics(log); + State convert_state(const State &state) const { + return repackage_state(state); } + + void assert_nested_is(const StateRegistry &nested); + virtual const int_packer::IntPacker &get_state_packer() const override; + virtual State lookup_state(StateID id) const override; + virtual State get_initial_state() override; + virtual State get_successor_state( + const State &predecessor, const OperatorProxy &op) override; + virtual size_t size() const override; + virtual void print_statistics(utils::LogProxy &log) const override; }; #endif From fbc7b3b5e388c264b17694a522e7c52afd5b3405 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 10:12:37 +0200 Subject: [PATCH 08/29] rename files default_value_axioms_evaluator.* to axiom_handling_evaluator.* --- src/search/CMakeLists.txt | 5 +++-- ...aluator.cc => axiom_handling_evaluator.cc} | 2 +- ...evaluator.h => axiom_handling_evaluator.h} | 0 src/search/heuristics/additive_heuristic.cc | 2 +- src/search/heuristics/cea_heuristic.cc | 2 +- src/search/heuristics/cg_heuristic.cc | 2 +- src/search/heuristics/ff_heuristic.cc | 2 +- src/search/heuristics/max_heuristic.cc | 2 +- src/search/landmarks/landmark_heuristic.cc | 19 ------------------- .../landmarks/landmark_sum_heuristic.cc | 2 +- 10 files changed, 10 insertions(+), 28 deletions(-) rename src/search/evaluators/{default_value_axioms_evaluator.cc => axiom_handling_evaluator.cc} (99%) rename src/search/evaluators/{default_value_axioms_evaluator.h => axiom_handling_evaluator.h} (100%) diff --git a/src/search/CMakeLists.txt b/src/search/CMakeLists.txt index efe989c957..8b7b7dc7cc 100644 --- a/src/search/CMakeLists.txt +++ b/src/search/CMakeLists.txt @@ -349,13 +349,14 @@ create_fast_downward_library( ) create_fast_downward_library( - NAME default_value_axioms_evaluator + NAME axiom_handling_evaluator HELP "Evaluator modifying axioms to handle negative values" SOURCES - evaluators/default_value_axioms_evaluator + evaluators/axiom_handling_evaluator DEPENDS core_tasks task_properties + default_value_axioms_task ) create_fast_downward_library( diff --git a/src/search/evaluators/default_value_axioms_evaluator.cc b/src/search/evaluators/axiom_handling_evaluator.cc similarity index 99% rename from src/search/evaluators/default_value_axioms_evaluator.cc rename to src/search/evaluators/axiom_handling_evaluator.cc index f6702ab1c1..6dc2729da7 100644 --- a/src/search/evaluators/default_value_axioms_evaluator.cc +++ b/src/search/evaluators/axiom_handling_evaluator.cc @@ -1,4 +1,4 @@ -#include "default_value_axioms_evaluator.h" +#include "axiom_handling_evaluator.h" #include "../evaluation_context.h" diff --git a/src/search/evaluators/default_value_axioms_evaluator.h b/src/search/evaluators/axiom_handling_evaluator.h similarity index 100% rename from src/search/evaluators/default_value_axioms_evaluator.h rename to src/search/evaluators/axiom_handling_evaluator.h diff --git a/src/search/heuristics/additive_heuristic.cc b/src/search/heuristics/additive_heuristic.cc index e5c69379a6..2723ac3c8f 100644 --- a/src/search/heuristics/additive_heuristic.cc +++ b/src/search/heuristics/additive_heuristic.cc @@ -1,6 +1,6 @@ #include "additive_heuristic.h" -#include "../evaluators/default_value_axioms_evaluator.h" +#include "../evaluators/axiom_handling_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index 23384c156d..eef1ce2ec4 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -2,7 +2,7 @@ #include "domain_transition_graph.h" -#include "../evaluators/default_value_axioms_evaluator.h" +#include "../evaluators/axiom_handling_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index ad4706520f..9247b5f480 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -3,7 +3,7 @@ #include "cg_cache.h" #include "domain_transition_graph.h" -#include "../evaluators/default_value_axioms_evaluator.h" +#include "../evaluators/axiom_handling_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" diff --git a/src/search/heuristics/ff_heuristic.cc b/src/search/heuristics/ff_heuristic.cc index 709978d1b6..ffb201ccde 100644 --- a/src/search/heuristics/ff_heuristic.cc +++ b/src/search/heuristics/ff_heuristic.cc @@ -1,6 +1,6 @@ #include "ff_heuristic.h" -#include "../evaluators/default_value_axioms_evaluator.h" +#include "../evaluators/axiom_handling_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/logging.h" diff --git a/src/search/heuristics/max_heuristic.cc b/src/search/heuristics/max_heuristic.cc index b028324a46..7ddcf911ee 100644 --- a/src/search/heuristics/max_heuristic.cc +++ b/src/search/heuristics/max_heuristic.cc @@ -1,6 +1,6 @@ #include "max_heuristic.h" -#include "../evaluators/default_value_axioms_evaluator.h" +#include "../evaluators/axiom_handling_evaluator.h" #include "../plugins/plugin.h" #include "../utils/logging.h" diff --git a/src/search/landmarks/landmark_heuristic.cc b/src/search/landmarks/landmark_heuristic.cc index ac0ea99a7e..f0c30a6d39 100644 --- a/src/search/landmarks/landmark_heuristic.cc +++ b/src/search/landmarks/landmark_heuristic.cc @@ -63,25 +63,6 @@ static bool landmark_graph_has_cycle_of_natural_orderings( void LandmarkHeuristic::initialize( const shared_ptr &landmark_factory, bool prog_goal, bool prog_gn, bool prog_r) { - /* - Actually, we should test if this is the root task or a task that *only* - transforms costs and/or adds negated axioms. However, there is currently - no good way to do this, so we use this incomplete, slightly less safe - test. - - issue1208 update the comment after removing the task transformation code. - The check can hopefully just be removed. - */ - if (task != tasks::g_root_task && - dynamic_cast(task.get()) == nullptr && - dynamic_cast(task.get()) == nullptr) { - cerr << "The landmark heuristics currently only support " - << "task transformations that modify the operator costs " - << "or add negated axioms. See issues 845, 686 and 454 " - << "for details." << endl; - utils::exit_with(utils::ExitCode::SEARCH_UNSUPPORTED); - } - compute_landmark_graph(landmark_factory); landmark_status_manager = make_unique( *landmark_graph, prog_goal, prog_gn, prog_r); diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 01fed7d1b2..503cf454ea 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -5,7 +5,7 @@ #include "landmark_status_manager.h" #include "util.h" -#include "../evaluators/default_value_axioms_evaluator.h" +#include "../evaluators/axiom_handling_evaluator.h" #include "../plugins/plugin.h" #include "../task_utils/successor_generator.h" #include "../task_utils/task_properties.h" From 27363afb06281e8cbc3f5130ebe4c1383e271653 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 10:47:32 +0200 Subject: [PATCH 09/29] replace checks that are now outdated with a check that states belong to the correct task. update isse numbers in comments --- src/search/heuristic.cc | 1 + src/search/landmarks/landmark_factory.cc | 2 +- src/search/task_proxy.h | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/search/heuristic.cc b/src/search/heuristic.cc index 1bbeb1a4d4..166e9115af 100644 --- a/src/search/heuristic.cc +++ b/src/search/heuristic.cc @@ -51,6 +51,7 @@ EvaluationResult Heuristic::compute_result(EvaluationContext &eval_context) { assert(preferred_operators.empty()); const State &state = eval_context.get_state(); + assert(state.get_task() == TaskProxy(*task)); bool calculate_preferred = eval_context.get_calculate_preferred(); int heuristic = NO_VALUE; diff --git a/src/search/landmarks/landmark_factory.cc b/src/search/landmarks/landmark_factory.cc index bfe56d9779..050373086b 100644 --- a/src/search/landmarks/landmark_factory.cc +++ b/src/search/landmarks/landmark_factory.cc @@ -160,7 +160,7 @@ void LandmarkFactory::log_landmark_graph_info( TaskProxy used by the Exploration object is the same as the TaskProxy object passed to this function. - issue1208: update comment above after removing the task transformation code. + issue1209: update comment above after removing the factory-factory pattern. */ shared_ptr LandmarkFactory::compute_landmark_graph( const shared_ptr &task) { diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index f1134bedb8..7f8088e406 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -662,6 +662,9 @@ class TaskProxy { } ~TaskProxy() = default; + // Compare tasks based on identity (not structural equivalence). + bool operator==(const TaskProxy &) const = default; + TaskID get_id() const { return TaskID(task); } From a1b6b4ce0533465f9fb25e240e60cab51861d3b8 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 11:17:18 +0200 Subject: [PATCH 10/29] move flags for evaluator inside AxiomHandlingEvaluator and change them to false like other wrapping evaluators --- .../evaluators/axiom_handling_evaluator.cc | 21 ++++++------------- .../evaluators/axiom_handling_evaluator.h | 14 ++++--------- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/search/evaluators/axiom_handling_evaluator.cc b/src/search/evaluators/axiom_handling_evaluator.cc index 6dc2729da7..ac573ffd09 100644 --- a/src/search/evaluators/axiom_handling_evaluator.cc +++ b/src/search/evaluators/axiom_handling_evaluator.cc @@ -9,12 +9,8 @@ using namespace std; namespace axiom_handling_evaluator { AxiomHandlingEvaluator::AxiomHandlingEvaluator( const shared_ptr &task, const shared_ptr &nested, - bool use_for_reporting_minima, bool use_for_boosting, - bool use_for_counting_evaluations, const string &description, - utils::Verbosity verbosity) - : Evaluator( - task, use_for_reporting_minima, use_for_boosting, - use_for_counting_evaluations, description, verbosity), + const string &description, utils::Verbosity verbosity) + : Evaluator(task, false, false, false, description, verbosity), nested(nested) { } @@ -96,8 +92,7 @@ TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( make_shared(task, axioms); shared_ptr eval = nested->bind_task(axioms_task); return make_shared( - task, eval, use_for_reporting_minima, use_for_boosting, - use_for_counting_evaluations, description, verbosity); + task, eval, description, verbosity); } else { /* If the task has no axioms, there is no need to wrap the evaluator @@ -109,14 +104,10 @@ TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( TaskIndependentAxiomHandlingEvaluator::TaskIndependentAxiomHandlingEvaluator( shared_ptr nested, - tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, - bool use_for_boosting, bool use_for_counting_evaluations, - const string &description, utils::Verbosity verbosity) + tasks::AxiomHandlingType axioms, const string &description, + utils::Verbosity verbosity) : nested(move(nested)), axioms(axioms), - use_for_reporting_minima(use_for_reporting_minima), - use_for_boosting(use_for_boosting), - use_for_counting_evaluations(use_for_counting_evaluations), description(description), verbosity(verbosity) { } @@ -126,7 +117,7 @@ shared_ptr wrap_in_axiom_handling_evaluator( const plugins::Options &opts) { return components::make_shared_from_arg_tuples< axiom_handling_evaluator::TaskIndependentAxiomHandlingEvaluator>( - eval, tasks::get_axioms_arguments_from_options(opts), true, true, true, + eval, tasks::get_axioms_arguments_from_options(opts), get_evaluator_arguments_from_options(opts)); } diff --git a/src/search/evaluators/axiom_handling_evaluator.h b/src/search/evaluators/axiom_handling_evaluator.h index 9787f26579..9fc0d69f3b 100644 --- a/src/search/evaluators/axiom_handling_evaluator.h +++ b/src/search/evaluators/axiom_handling_evaluator.h @@ -17,8 +17,7 @@ class AxiomHandlingEvaluator : public Evaluator { public: AxiomHandlingEvaluator( const std::shared_ptr &task, - const std::shared_ptr &nested, bool use_for_reporting_minima, - bool use_for_boosting, bool use_for_counting_evaluations, + const std::shared_ptr &nested, const std::string &description, utils::Verbosity verbosity); virtual bool dead_ends_are_reliable() const override; @@ -35,13 +34,9 @@ class AxiomHandlingEvaluator : public Evaluator { virtual int get_cached_estimate(const State &state) const override; }; -class TaskIndependentAxiomHandlingEvaluator - : public TaskIndependentEvaluator { +class TaskIndependentAxiomHandlingEvaluator : public TaskIndependentEvaluator { std::shared_ptr nested; tasks::AxiomHandlingType axioms; - bool use_for_reporting_minima; - bool use_for_boosting; - bool use_for_counting_evaluations; const std::string description; utils::Verbosity verbosity; @@ -50,9 +45,8 @@ class TaskIndependentAxiomHandlingEvaluator public: TaskIndependentAxiomHandlingEvaluator( std::shared_ptr nested, - tasks::AxiomHandlingType axioms, bool use_for_reporting_minima, - bool use_for_boosting, bool use_for_counting_evaluations, - const std::string &description, utils::Verbosity verbosity); + tasks::AxiomHandlingType axioms, const std::string &description, + utils::Verbosity verbosity); }; std::shared_ptr wrap_in_axiom_handling_evaluator( From 232d539e678b2c6f89771f1f9edf7368432f65b5 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 11:17:53 +0200 Subject: [PATCH 11/29] unify AxiomHandlingEvaluator and ModifyCostsEvaluator --- .../evaluators/modify_costs_evaluator.cc | 89 ++++++++++++++----- .../evaluators/modify_costs_evaluator.h | 8 +- 2 files changed, 73 insertions(+), 24 deletions(-) diff --git a/src/search/evaluators/modify_costs_evaluator.cc b/src/search/evaluators/modify_costs_evaluator.cc index 80ff9a26d2..dc52150e95 100644 --- a/src/search/evaluators/modify_costs_evaluator.cc +++ b/src/search/evaluators/modify_costs_evaluator.cc @@ -10,6 +10,39 @@ using namespace std; namespace cost_adapted_evaluator { +ModifyCostsEvaluator::ModifyCostsEvaluator( + const shared_ptr &task, const shared_ptr &nested, + const string &description, utils::Verbosity verbosity) + : Evaluator(task, false, false, false, description, verbosity), + nested(nested) { +} + +State ModifyCostsEvaluator::convert_state(const State &state) const { + const StateRegistry *existing_state_registry = state.get_registry(); + assert(existing_state_registry); + if (!state_registry) { + /* + issue1227: + * avoid const cast? + -> currently needed because we only get the registry from states + passed to this evaluator instead of knowing the registry in the + constructor already. We only get a const registry pointer from + the state but need it to be mutable in the delegating registry. + * avoid mutable state_registry? + -> currently this is used for the delayed initialization which can + happen inside const methods. + * avoid delayed initialization alltogether + Both of the above problems would go away if we could create the + registry in the constructor. + */ + state_registry = make_unique( + task, const_cast(*existing_state_registry)); + } else { + state_registry->assert_nested_is(*existing_state_registry); + } + return state_registry->convert_state(state); +} + bool ModifyCostsEvaluator::dead_ends_are_reliable() const { return nested->dead_ends_are_reliable(); } @@ -20,26 +53,25 @@ void ModifyCostsEvaluator::get_path_dependent_evaluators( } void ModifyCostsEvaluator::notify_initial_state(const State &initial_state) { - /* - TODO issue1208: Once we remove the task transformation code from - heuristics, we might have to transform the task here. While the state data - doesn't change, the State class currently references the task it belongs - to, and `nested` uses a different task. - */ - nested->notify_initial_state(initial_state); + nested->notify_initial_state(convert_state(initial_state)); } void ModifyCostsEvaluator::notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) { - // TODO issue1208: see above - nested->notify_state_transition(parent_state, op_id, state); + nested->notify_state_transition( + convert_state(parent_state), op_id, convert_state(state)); } EvaluationResult ModifyCostsEvaluator::compute_result( EvaluationContext &eval_context) { - // TODO issue1208: see above (in particular, eval_context is specific to a - // state) - return nested->compute_result(eval_context); + State translated_state = convert_state(eval_context.get_state()); + EvaluationContext translated_context(eval_context, translated_state); + /* + Note that we do not need to update the cache inside eval_context. + the call below can only set and access evaluators that use the transformed + task and users of eval_context cannot know such evaluators. + */ + return nested->compute_result(translated_context); } bool ModifyCostsEvaluator::does_cache_estimates() const { @@ -47,26 +79,34 @@ bool ModifyCostsEvaluator::does_cache_estimates() const { } bool ModifyCostsEvaluator::is_estimate_cached(const State &state) const { - // TODO issue1208: see above - return nested->is_estimate_cached(state); + return nested->is_estimate_cached(convert_state(state)); } int ModifyCostsEvaluator::get_cached_estimate(const State &state) const { - // TODO issue1208: see above - return nested->get_cached_estimate(state); + return nested->get_cached_estimate(convert_state(state)); } shared_ptr TaskIndependentModifyCostsEvaluator::create_task_specific_component( const shared_ptr &task) const { - shared_ptr cost_adapted_task = - make_shared(task, cost_type); - return nested->bind_task(cost_adapted_task); + if (cost_type == OperatorCost::NORMAL) { + return nested->bind_task(task); + } else { + shared_ptr cost_adapted_task = + make_shared(task, cost_type); + shared_ptr eval = nested->bind_task(cost_adapted_task); + return make_shared( + task, eval, description, verbosity); + } } TaskIndependentModifyCostsEvaluator::TaskIndependentModifyCostsEvaluator( - shared_ptr nested, OperatorCost cost_type) - : nested(move(nested)), cost_type(cost_type) { + shared_ptr nested, OperatorCost cost_type, + const string &description, utils::Verbosity verbosity) + : nested(move(nested)), + cost_type(cost_type), + description(description), + verbosity(verbosity) { } class ModifyCostEvaluatorFeature @@ -88,6 +128,7 @@ class ModifyCostEvaluatorFeature "axioms will always be considered as actions of cost 0 by the " "evaluators that treat axioms as actions.", "normal"); + add_evaluator_options_to_feature(*this, "eval_modify_costs"); document_language_support( "action costs", "supported if the nested evaluator supports them; " @@ -117,9 +158,11 @@ class ModifyCostEvaluatorFeature virtual shared_ptr create_component( const plugins::Options &opts) const override { - return make_shared( + return components::make_shared_from_arg_tuples< + TaskIndependentModifyCostsEvaluator>( opts.get>("nested"), - opts.get("cost_type")); + opts.get("cost_type"), + get_evaluator_arguments_from_options(opts)); } }; diff --git a/src/search/evaluators/modify_costs_evaluator.h b/src/search/evaluators/modify_costs_evaluator.h index 74a429db6c..f1e94da0f8 100644 --- a/src/search/evaluators/modify_costs_evaluator.h +++ b/src/search/evaluators/modify_costs_evaluator.h @@ -4,12 +4,15 @@ #include "../component.h" #include "../evaluator.h" #include "../operator_cost.h" +#include "../state_registry.h" #include namespace cost_adapted_evaluator { class ModifyCostsEvaluator : public Evaluator { std::shared_ptr nested; + mutable std::unique_ptr state_registry; + State convert_state(const State &state) const; public: ModifyCostsEvaluator( const std::shared_ptr &task, @@ -33,13 +36,16 @@ class ModifyCostsEvaluator : public Evaluator { class TaskIndependentModifyCostsEvaluator : public TaskIndependentEvaluator { std::shared_ptr nested; OperatorCost cost_type; + const std::string description; + utils::Verbosity verbosity; virtual std::shared_ptr create_task_specific_component( const std::shared_ptr &task) const override; public: TaskIndependentModifyCostsEvaluator( std::shared_ptr nested, - OperatorCost cost_type); + OperatorCost cost_type, const std::string &description, + utils::Verbosity verbosity); }; } From 75d7c6ff6c907cd38225dee14a9a82e13ee62154 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 12:37:17 +0200 Subject: [PATCH 12/29] extract simple task transforming evaluator --- src/search/CMakeLists.txt | 12 ++- .../evaluators/axiom_handling_evaluator.cc | 81 +---------------- .../evaluators/axiom_handling_evaluator.h | 26 ------ .../evaluators/modify_costs_evaluator.cc | 81 +---------------- .../evaluators/modify_costs_evaluator.h | 26 ------ .../simple_task_transforming_evaluator.cc | 86 +++++++++++++++++++ .../simple_task_transforming_evaluator.h | 45 ++++++++++ 7 files changed, 149 insertions(+), 208 deletions(-) create mode 100644 src/search/evaluators/simple_task_transforming_evaluator.cc create mode 100644 src/search/evaluators/simple_task_transforming_evaluator.h diff --git a/src/search/CMakeLists.txt b/src/search/CMakeLists.txt index 8b7b7dc7cc..09a8a6d937 100644 --- a/src/search/CMakeLists.txt +++ b/src/search/CMakeLists.txt @@ -348,15 +348,22 @@ create_fast_downward_library( DEPENDENCY_ONLY ) +create_fast_downward_library( + NAME simple_task_transforming_evaluator + HELP "Evaluator that uses a transformed task with the same set of states" + SOURCES + evaluators/simple_task_transforming_evaluator +) + create_fast_downward_library( NAME axiom_handling_evaluator HELP "Evaluator modifying axioms to handle negative values" SOURCES evaluators/axiom_handling_evaluator DEPENDS - core_tasks - task_properties default_value_axioms_task + simple_task_transforming_evaluator + task_properties ) create_fast_downward_library( @@ -366,6 +373,7 @@ create_fast_downward_library( evaluators/modify_costs_evaluator DEPENDS core_tasks + simple_task_transforming_evaluator task_properties ) diff --git a/src/search/evaluators/axiom_handling_evaluator.cc b/src/search/evaluators/axiom_handling_evaluator.cc index ac573ffd09..a72140a440 100644 --- a/src/search/evaluators/axiom_handling_evaluator.cc +++ b/src/search/evaluators/axiom_handling_evaluator.cc @@ -1,5 +1,7 @@ #include "axiom_handling_evaluator.h" +#include "simple_task_transforming_evaluator.h" + #include "../evaluation_context.h" #include "../task_utils/task_properties.h" @@ -7,82 +9,6 @@ using namespace std; namespace axiom_handling_evaluator { -AxiomHandlingEvaluator::AxiomHandlingEvaluator( - const shared_ptr &task, const shared_ptr &nested, - const string &description, utils::Verbosity verbosity) - : Evaluator(task, false, false, false, description, verbosity), - nested(nested) { -} - -State AxiomHandlingEvaluator::convert_state(const State &state) const { - const StateRegistry *existing_state_registry = state.get_registry(); - assert(existing_state_registry); - if (!state_registry) { - /* - issue1227: - * avoid const cast? - -> currently needed because we only get the registry from states - passed to this evaluator instead of knowing the registry in the - constructor already. We only get a const registry pointer from - the state but need it to be mutable in the delegating registry. - * avoid mutable state_registry? - -> currently this is used for the delayed initialization which can - happen inside const methods. - * avoid delayed initialization alltogether - Both of the above problems would go away if we could create the - registry in the constructor. - */ - state_registry = make_unique( - task, const_cast(*existing_state_registry)); - } else { - state_registry->assert_nested_is(*existing_state_registry); - } - return state_registry->convert_state(state); -} - -bool AxiomHandlingEvaluator::dead_ends_are_reliable() const { - return nested->dead_ends_are_reliable(); -} - -void AxiomHandlingEvaluator::get_path_dependent_evaluators( - set &evals) { - nested->get_path_dependent_evaluators(evals); -} - -void AxiomHandlingEvaluator::notify_initial_state(const State &initial_state) { - nested->notify_initial_state(convert_state(initial_state)); -} - -void AxiomHandlingEvaluator::notify_state_transition( - const State &parent_state, OperatorID op_id, const State &state) { - nested->notify_state_transition( - convert_state(parent_state), op_id, convert_state(state)); -} - -EvaluationResult AxiomHandlingEvaluator::compute_result( - EvaluationContext &eval_context) { - State translated_state = convert_state(eval_context.get_state()); - EvaluationContext translated_context(eval_context, translated_state); - /* - Note that we do not need to update the cache inside eval_context. - the call below can only set and access evaluators that use the transformed - task and users of eval_context cannot know such evaluators. - */ - return nested->compute_result(translated_context); -} - -bool AxiomHandlingEvaluator::does_cache_estimates() const { - return nested->does_cache_estimates(); -} - -bool AxiomHandlingEvaluator::is_estimate_cached(const State &state) const { - return nested->is_estimate_cached(convert_state(state)); -} - -int AxiomHandlingEvaluator::get_cached_estimate(const State &state) const { - return nested->get_cached_estimate(convert_state(state)); -} - shared_ptr TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( const shared_ptr &task) const { @@ -91,7 +17,8 @@ TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( shared_ptr axioms_task = make_shared(task, axioms); shared_ptr eval = nested->bind_task(axioms_task); - return make_shared( + return make_shared( task, eval, description, verbosity); } else { /* diff --git a/src/search/evaluators/axiom_handling_evaluator.h b/src/search/evaluators/axiom_handling_evaluator.h index 9fc0d69f3b..6a78bfca28 100644 --- a/src/search/evaluators/axiom_handling_evaluator.h +++ b/src/search/evaluators/axiom_handling_evaluator.h @@ -1,39 +1,13 @@ #ifndef EVALUATORS_AXIOM_HANDLING_EVALUATOR_H #define EVALUATORS_AXIOM_HANDLING_EVALUATOR_H -#include "../component.h" #include "../evaluator.h" -#include "../state_registry.h" #include "../tasks/default_value_axioms_task.h" #include namespace axiom_handling_evaluator { -class AxiomHandlingEvaluator : public Evaluator { - std::shared_ptr nested; - mutable std::unique_ptr state_registry; - State convert_state(const State &state) const; -public: - AxiomHandlingEvaluator( - const std::shared_ptr &task, - const std::shared_ptr &nested, - const std::string &description, utils::Verbosity verbosity); - - virtual bool dead_ends_are_reliable() const override; - virtual void get_path_dependent_evaluators( - std::set &evals) override; - virtual void notify_initial_state(const State &initial_state) override; - virtual void notify_state_transition( - const State &parent_state, OperatorID op_id, - const State &state) override; - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; - virtual bool does_cache_estimates() const override; - virtual bool is_estimate_cached(const State &state) const override; - virtual int get_cached_estimate(const State &state) const override; -}; - class TaskIndependentAxiomHandlingEvaluator : public TaskIndependentEvaluator { std::shared_ptr nested; tasks::AxiomHandlingType axioms; diff --git a/src/search/evaluators/modify_costs_evaluator.cc b/src/search/evaluators/modify_costs_evaluator.cc index dc52150e95..4162e567c3 100644 --- a/src/search/evaluators/modify_costs_evaluator.cc +++ b/src/search/evaluators/modify_costs_evaluator.cc @@ -1,5 +1,7 @@ #include "modify_costs_evaluator.h" +#include "simple_task_transforming_evaluator.h" + #include "../evaluation_context.h" #include "../plugins/plugin.h" @@ -10,82 +12,6 @@ using namespace std; namespace cost_adapted_evaluator { -ModifyCostsEvaluator::ModifyCostsEvaluator( - const shared_ptr &task, const shared_ptr &nested, - const string &description, utils::Verbosity verbosity) - : Evaluator(task, false, false, false, description, verbosity), - nested(nested) { -} - -State ModifyCostsEvaluator::convert_state(const State &state) const { - const StateRegistry *existing_state_registry = state.get_registry(); - assert(existing_state_registry); - if (!state_registry) { - /* - issue1227: - * avoid const cast? - -> currently needed because we only get the registry from states - passed to this evaluator instead of knowing the registry in the - constructor already. We only get a const registry pointer from - the state but need it to be mutable in the delegating registry. - * avoid mutable state_registry? - -> currently this is used for the delayed initialization which can - happen inside const methods. - * avoid delayed initialization alltogether - Both of the above problems would go away if we could create the - registry in the constructor. - */ - state_registry = make_unique( - task, const_cast(*existing_state_registry)); - } else { - state_registry->assert_nested_is(*existing_state_registry); - } - return state_registry->convert_state(state); -} - -bool ModifyCostsEvaluator::dead_ends_are_reliable() const { - return nested->dead_ends_are_reliable(); -} - -void ModifyCostsEvaluator::get_path_dependent_evaluators( - set &evals) { - nested->get_path_dependent_evaluators(evals); -} - -void ModifyCostsEvaluator::notify_initial_state(const State &initial_state) { - nested->notify_initial_state(convert_state(initial_state)); -} - -void ModifyCostsEvaluator::notify_state_transition( - const State &parent_state, OperatorID op_id, const State &state) { - nested->notify_state_transition( - convert_state(parent_state), op_id, convert_state(state)); -} - -EvaluationResult ModifyCostsEvaluator::compute_result( - EvaluationContext &eval_context) { - State translated_state = convert_state(eval_context.get_state()); - EvaluationContext translated_context(eval_context, translated_state); - /* - Note that we do not need to update the cache inside eval_context. - the call below can only set and access evaluators that use the transformed - task and users of eval_context cannot know such evaluators. - */ - return nested->compute_result(translated_context); -} - -bool ModifyCostsEvaluator::does_cache_estimates() const { - return nested->does_cache_estimates(); -} - -bool ModifyCostsEvaluator::is_estimate_cached(const State &state) const { - return nested->is_estimate_cached(convert_state(state)); -} - -int ModifyCostsEvaluator::get_cached_estimate(const State &state) const { - return nested->get_cached_estimate(convert_state(state)); -} - shared_ptr TaskIndependentModifyCostsEvaluator::create_task_specific_component( const shared_ptr &task) const { @@ -95,7 +21,8 @@ TaskIndependentModifyCostsEvaluator::create_task_specific_component( shared_ptr cost_adapted_task = make_shared(task, cost_type); shared_ptr eval = nested->bind_task(cost_adapted_task); - return make_shared( + return make_shared( task, eval, description, verbosity); } } diff --git a/src/search/evaluators/modify_costs_evaluator.h b/src/search/evaluators/modify_costs_evaluator.h index f1e94da0f8..97a847d785 100644 --- a/src/search/evaluators/modify_costs_evaluator.h +++ b/src/search/evaluators/modify_costs_evaluator.h @@ -1,38 +1,12 @@ #ifndef EVALUATORS_MODIFY_COSTS_EVALUATOR_H #define EVALUATORS_MODIFY_COSTS_EVALUATOR_H -#include "../component.h" #include "../evaluator.h" #include "../operator_cost.h" -#include "../state_registry.h" #include namespace cost_adapted_evaluator { -class ModifyCostsEvaluator : public Evaluator { - std::shared_ptr nested; - mutable std::unique_ptr state_registry; - State convert_state(const State &state) const; -public: - ModifyCostsEvaluator( - const std::shared_ptr &task, - const std::shared_ptr &nested, - const std::string &description, utils::Verbosity verbosity); - - virtual bool dead_ends_are_reliable() const override; - virtual void get_path_dependent_evaluators( - std::set &evals) override; - virtual void notify_initial_state(const State &initial_state) override; - virtual void notify_state_transition( - const State &parent_state, OperatorID op_id, - const State &state) override; - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; - virtual bool does_cache_estimates() const override; - virtual bool is_estimate_cached(const State &state) const override; - virtual int get_cached_estimate(const State &state) const override; -}; - class TaskIndependentModifyCostsEvaluator : public TaskIndependentEvaluator { std::shared_ptr nested; OperatorCost cost_type; diff --git a/src/search/evaluators/simple_task_transforming_evaluator.cc b/src/search/evaluators/simple_task_transforming_evaluator.cc new file mode 100644 index 0000000000..778c6f289e --- /dev/null +++ b/src/search/evaluators/simple_task_transforming_evaluator.cc @@ -0,0 +1,86 @@ +#include "simple_task_transforming_evaluator.h" + +#include "../evaluation_context.h" + +using namespace std; + +namespace simple_task_transforming_evaluator { +SimpleTaskTransformingEvaluator::SimpleTaskTransformingEvaluator( + const shared_ptr &task, const shared_ptr &nested, + const string &description, utils::Verbosity verbosity) + : Evaluator(task, false, false, false, description, verbosity), + nested(nested) { +} + +State SimpleTaskTransformingEvaluator::convert_state(const State &state) const { + const StateRegistry *existing_state_registry = state.get_registry(); + assert(existing_state_registry); + if (!state_registry) { + /* + issue1227: + * avoid const cast? + -> currently needed because we only get the registry from states + passed to this evaluator instead of knowing the registry in the + constructor already. We only get a const registry pointer from + the state but need it to be mutable in the delegating registry. + * avoid mutable state_registry? + -> currently this is used for the delayed initialization which can + happen inside const methods. + * avoid delayed initialization alltogether + Both of the above problems would go away if we could create the + registry in the constructor. + */ + state_registry = make_unique( + task, const_cast(*existing_state_registry)); + } else { + state_registry->assert_nested_is(*existing_state_registry); + } + return state_registry->convert_state(state); +} + +bool SimpleTaskTransformingEvaluator::dead_ends_are_reliable() const { + return nested->dead_ends_are_reliable(); +} + +void SimpleTaskTransformingEvaluator::get_path_dependent_evaluators( + set &evals) { + nested->get_path_dependent_evaluators(evals); +} + +void SimpleTaskTransformingEvaluator::notify_initial_state( + const State &initial_state) { + nested->notify_initial_state(convert_state(initial_state)); +} + +void SimpleTaskTransformingEvaluator::notify_state_transition( + const State &parent_state, OperatorID op_id, const State &state) { + nested->notify_state_transition( + convert_state(parent_state), op_id, convert_state(state)); +} + +EvaluationResult SimpleTaskTransformingEvaluator::compute_result( + EvaluationContext &eval_context) { + State translated_state = convert_state(eval_context.get_state()); + EvaluationContext translated_context(eval_context, translated_state); + /* + Note that we do not need to update the cache inside eval_context. + the call below can only set and access evaluators that use the transformed + task and users of eval_context cannot know such evaluators. + */ + return nested->compute_result(translated_context); +} + +bool SimpleTaskTransformingEvaluator::does_cache_estimates() const { + return nested->does_cache_estimates(); +} + +bool SimpleTaskTransformingEvaluator::is_estimate_cached( + const State &state) const { + return nested->is_estimate_cached(convert_state(state)); +} + +int SimpleTaskTransformingEvaluator::get_cached_estimate( + const State &state) const { + return nested->get_cached_estimate(convert_state(state)); +} +} diff --git a/src/search/evaluators/simple_task_transforming_evaluator.h b/src/search/evaluators/simple_task_transforming_evaluator.h new file mode 100644 index 0000000000..00d35d3852 --- /dev/null +++ b/src/search/evaluators/simple_task_transforming_evaluator.h @@ -0,0 +1,45 @@ +#ifndef EVALUATORS_SIMPLE_TASK_TRANSFORMING_EVALUATOR_H +#define EVALUATORS_SIMPLE_TASK_TRANSFORMING_EVALUATOR_H + +#include "../evaluator.h" +#include "../state_registry.h" + +#include + +namespace simple_task_transforming_evaluator { +/* + This evaluator evaluates a nested evaluator on a transformed version of its + own task. The assumption is that the nested evaluator uses a task that has + the same set of states as the task used to instantiate the transforming + evaluator. States are thus not translated to the nested task before passing + them on, but instead only re-interpreted as states of the nested task without + storing them in a dedicated state registry. They keep their state IDs but are + passed on to the nested evaluator as states of the nested task, so the nested + evaluator does not have to know about the task transformation. +*/ +class SimpleTaskTransformingEvaluator : public Evaluator { + std::shared_ptr nested; + mutable std::unique_ptr state_registry; + State convert_state(const State &state) const; +public: + SimpleTaskTransformingEvaluator( + const std::shared_ptr &task, + const std::shared_ptr &nested, + const std::string &description, utils::Verbosity verbosity); + + virtual bool dead_ends_are_reliable() const override; + virtual void get_path_dependent_evaluators( + std::set &evals) override; + virtual void notify_initial_state(const State &initial_state) override; + virtual void notify_state_transition( + const State &parent_state, OperatorID op_id, + const State &state) override; + virtual EvaluationResult compute_result( + EvaluationContext &eval_context) override; + virtual bool does_cache_estimates() const override; + virtual bool is_estimate_cached(const State &state) const override; + virtual int get_cached_estimate(const State &state) const override; +}; +} + +#endif From 0e2abeb3861b74bc66d4309acaab7392f2a15fbd Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 13:44:39 +0200 Subject: [PATCH 13/29] use correct task --- src/search/evaluators/axiom_handling_evaluator.cc | 2 +- src/search/evaluators/modify_costs_evaluator.cc | 2 +- .../evaluators/simple_task_transforming_evaluator.cc | 10 +++++++--- .../evaluators/simple_task_transforming_evaluator.h | 2 ++ 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/search/evaluators/axiom_handling_evaluator.cc b/src/search/evaluators/axiom_handling_evaluator.cc index a72140a440..77694756cf 100644 --- a/src/search/evaluators/axiom_handling_evaluator.cc +++ b/src/search/evaluators/axiom_handling_evaluator.cc @@ -19,7 +19,7 @@ TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( shared_ptr eval = nested->bind_task(axioms_task); return make_shared( - task, eval, description, verbosity); + task, axioms_task, eval, description, verbosity); } else { /* If the task has no axioms, there is no need to wrap the evaluator diff --git a/src/search/evaluators/modify_costs_evaluator.cc b/src/search/evaluators/modify_costs_evaluator.cc index 4162e567c3..9db0352367 100644 --- a/src/search/evaluators/modify_costs_evaluator.cc +++ b/src/search/evaluators/modify_costs_evaluator.cc @@ -23,7 +23,7 @@ TaskIndependentModifyCostsEvaluator::create_task_specific_component( shared_ptr eval = nested->bind_task(cost_adapted_task); return make_shared( - task, eval, description, verbosity); + task, cost_adapted_task, eval, description, verbosity); } } diff --git a/src/search/evaluators/simple_task_transforming_evaluator.cc b/src/search/evaluators/simple_task_transforming_evaluator.cc index 778c6f289e..f38ec9f169 100644 --- a/src/search/evaluators/simple_task_transforming_evaluator.cc +++ b/src/search/evaluators/simple_task_transforming_evaluator.cc @@ -6,9 +6,12 @@ using namespace std; namespace simple_task_transforming_evaluator { SimpleTaskTransformingEvaluator::SimpleTaskTransformingEvaluator( - const shared_ptr &task, const shared_ptr &nested, - const string &description, utils::Verbosity verbosity) + const shared_ptr &task, + const shared_ptr &transformed_task, + const shared_ptr &nested, const string &description, + utils::Verbosity verbosity) : Evaluator(task, false, false, false, description, verbosity), + transformed_task(transformed_task), nested(nested) { } @@ -31,7 +34,8 @@ State SimpleTaskTransformingEvaluator::convert_state(const State &state) const { registry in the constructor. */ state_registry = make_unique( - task, const_cast(*existing_state_registry)); + transformed_task, + const_cast(*existing_state_registry)); } else { state_registry->assert_nested_is(*existing_state_registry); } diff --git a/src/search/evaluators/simple_task_transforming_evaluator.h b/src/search/evaluators/simple_task_transforming_evaluator.h index 00d35d3852..bbd66f6b84 100644 --- a/src/search/evaluators/simple_task_transforming_evaluator.h +++ b/src/search/evaluators/simple_task_transforming_evaluator.h @@ -18,12 +18,14 @@ namespace simple_task_transforming_evaluator { evaluator does not have to know about the task transformation. */ class SimpleTaskTransformingEvaluator : public Evaluator { + const std::shared_ptr transformed_task; std::shared_ptr nested; mutable std::unique_ptr state_registry; State convert_state(const State &state) const; public: SimpleTaskTransformingEvaluator( const std::shared_ptr &task, + const std::shared_ptr &transformed_task, const std::shared_ptr &nested, const std::string &description, utils::Verbosity verbosity); From f24bb029269facf6c8719ed3706d15318bbd0da1 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 14:47:32 +0200 Subject: [PATCH 14/29] notify transitions with correct states --- src/search/evaluator.h | 12 +++++-- .../simple_task_transforming_evaluator.cc | 32 ++++++++++++++++--- .../simple_task_transforming_evaluator.h | 1 + src/search/landmarks/landmark_heuristic.cc | 3 ++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/search/evaluator.h b/src/search/evaluator.h index 1a5fc35b5c..5d15a5e636 100644 --- a/src/search/evaluator.h +++ b/src/search/evaluator.h @@ -50,12 +50,18 @@ class Evaluator : public components::TaskSpecificComponent { virtual void get_path_dependent_evaluators( std::set &evals) = 0; - virtual void notify_initial_state(const State & /*initial_state*/) { + virtual void notify_initial_state(const State & initial_state) { + assert(initial_state.get_task() == TaskProxy(*task)); + utils::unused_variable(initial_state); } virtual void notify_state_transition( - const State & /*parent_state*/, OperatorID /*op_id*/, - const State & /*state*/) { + const State & parent_state, OperatorID /*op_id*/, + const State & state) { + assert(parent_state.get_task() == TaskProxy(*task)); + assert(state.get_task() == TaskProxy(*task)); + utils::unused_variable(parent_state); + utils::unused_variable(state); } /* diff --git a/src/search/evaluators/simple_task_transforming_evaluator.cc b/src/search/evaluators/simple_task_transforming_evaluator.cc index f38ec9f169..fca1a6af77 100644 --- a/src/search/evaluators/simple_task_transforming_evaluator.cc +++ b/src/search/evaluators/simple_task_transforming_evaluator.cc @@ -13,6 +13,9 @@ SimpleTaskTransformingEvaluator::SimpleTaskTransformingEvaluator( : Evaluator(task, false, false, false, description, verbosity), transformed_task(transformed_task), nested(nested) { + set evals; + nested->get_path_dependent_evaluators(evals); + path_dependent_evaluators.assign(evals.begin(), evals.end()); } State SimpleTaskTransformingEvaluator::convert_state(const State &state) const { @@ -48,18 +51,39 @@ bool SimpleTaskTransformingEvaluator::dead_ends_are_reliable() const { void SimpleTaskTransformingEvaluator::get_path_dependent_evaluators( set &evals) { - nested->get_path_dependent_evaluators(evals); + if (!path_dependent_evaluators.empty()) { + /* + Note that we do *not* add path_dependent_evaluators to this list + because they use a differnent task. Letting some code from the outside + the scope of this task transformation notify them directly would + bypass the task transformation and thus notify the nested evaluators + with states from incorrect tasks. Instead, this evaluator adds itself + and then forwards any events with the transformed task. + */ + evals.insert(this); + } } void SimpleTaskTransformingEvaluator::notify_initial_state( const State &initial_state) { - nested->notify_initial_state(convert_state(initial_state)); + if (!path_dependent_evaluators.empty()) { + State converted_initial_state = convert_state(initial_state); + for (Evaluator *eval : path_dependent_evaluators) { + eval->notify_initial_state(converted_initial_state); + } + } } void SimpleTaskTransformingEvaluator::notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) { - nested->notify_state_transition( - convert_state(parent_state), op_id, convert_state(state)); + if (!path_dependent_evaluators.empty()) { + State converted_parent_state = convert_state(parent_state); + State converted_state = convert_state(state); + for (Evaluator *eval : path_dependent_evaluators) { + eval->notify_state_transition( + converted_parent_state, op_id, converted_state); + } + } } EvaluationResult SimpleTaskTransformingEvaluator::compute_result( diff --git a/src/search/evaluators/simple_task_transforming_evaluator.h b/src/search/evaluators/simple_task_transforming_evaluator.h index bbd66f6b84..d7eb6ecc72 100644 --- a/src/search/evaluators/simple_task_transforming_evaluator.h +++ b/src/search/evaluators/simple_task_transforming_evaluator.h @@ -21,6 +21,7 @@ class SimpleTaskTransformingEvaluator : public Evaluator { const std::shared_ptr transformed_task; std::shared_ptr nested; mutable std::unique_ptr state_registry; + std::vector path_dependent_evaluators; State convert_state(const State &state) const; public: SimpleTaskTransformingEvaluator( diff --git a/src/search/landmarks/landmark_heuristic.cc b/src/search/landmarks/landmark_heuristic.cc index f0c30a6d39..ab617566fe 100644 --- a/src/search/landmarks/landmark_heuristic.cc +++ b/src/search/landmarks/landmark_heuristic.cc @@ -198,11 +198,14 @@ int LandmarkHeuristic::compute_heuristic(const State &ancestor_state) { } void LandmarkHeuristic::notify_initial_state(const State &initial_state) { + assert(initial_state.get_task() == TaskProxy(*task)); landmark_status_manager->progress_initial_state(initial_state); } void LandmarkHeuristic::notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) { + assert(parent_state.get_task() == TaskProxy(*task)); + assert(state.get_task() == TaskProxy(*task)); landmark_status_manager->progress(parent_state, op_id, state); if (cache_evaluator_values) { /* TODO: It may be more efficient to check that the past landmark From 5e30a05f05b1b8f7afd89c0a0de772e4fce2a66a Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 21:00:34 +0200 Subject: [PATCH 15/29] rename simple_task_transforming_evaluator to state_forwarding_evaluator and fix style --- src/search/CMakeLists.txt | 8 +++--- src/search/evaluator.h | 5 ++-- .../evaluators/axiom_handling_evaluator.cc | 6 ++--- .../evaluators/modify_costs_evaluator.cc | 6 ++--- ...uator.cc => state_forwarding_evaluator.cc} | 26 +++++++++---------- ...aluator.h => state_forwarding_evaluator.h} | 10 +++---- src/search/state_registry.cc | 7 ++--- 7 files changed, 31 insertions(+), 37 deletions(-) rename src/search/evaluators/{simple_task_transforming_evaluator.cc => state_forwarding_evaluator.cc} (81%) rename src/search/evaluators/{simple_task_transforming_evaluator.h => state_forwarding_evaluator.h} (88%) diff --git a/src/search/CMakeLists.txt b/src/search/CMakeLists.txt index 09a8a6d937..eb103944a2 100644 --- a/src/search/CMakeLists.txt +++ b/src/search/CMakeLists.txt @@ -349,10 +349,10 @@ create_fast_downward_library( ) create_fast_downward_library( - NAME simple_task_transforming_evaluator + NAME state_forwarding_evaluator HELP "Evaluator that uses a transformed task with the same set of states" SOURCES - evaluators/simple_task_transforming_evaluator + evaluators/state_forwarding_evaluator ) create_fast_downward_library( @@ -362,7 +362,7 @@ create_fast_downward_library( evaluators/axiom_handling_evaluator DEPENDS default_value_axioms_task - simple_task_transforming_evaluator + state_forwarding_evaluator task_properties ) @@ -373,7 +373,7 @@ create_fast_downward_library( evaluators/modify_costs_evaluator DEPENDS core_tasks - simple_task_transforming_evaluator + state_forwarding_evaluator task_properties ) diff --git a/src/search/evaluator.h b/src/search/evaluator.h index 5d15a5e636..b5f423919c 100644 --- a/src/search/evaluator.h +++ b/src/search/evaluator.h @@ -50,14 +50,13 @@ class Evaluator : public components::TaskSpecificComponent { virtual void get_path_dependent_evaluators( std::set &evals) = 0; - virtual void notify_initial_state(const State & initial_state) { + virtual void notify_initial_state(const State &initial_state) { assert(initial_state.get_task() == TaskProxy(*task)); utils::unused_variable(initial_state); } virtual void notify_state_transition( - const State & parent_state, OperatorID /*op_id*/, - const State & state) { + const State &parent_state, OperatorID /*op_id*/, const State &state) { assert(parent_state.get_task() == TaskProxy(*task)); assert(state.get_task() == TaskProxy(*task)); utils::unused_variable(parent_state); diff --git a/src/search/evaluators/axiom_handling_evaluator.cc b/src/search/evaluators/axiom_handling_evaluator.cc index 77694756cf..74340d6902 100644 --- a/src/search/evaluators/axiom_handling_evaluator.cc +++ b/src/search/evaluators/axiom_handling_evaluator.cc @@ -1,6 +1,6 @@ #include "axiom_handling_evaluator.h" -#include "simple_task_transforming_evaluator.h" +#include "state_forwarding_evaluator.h" #include "../evaluation_context.h" @@ -17,8 +17,8 @@ TaskIndependentAxiomHandlingEvaluator::create_task_specific_component( shared_ptr axioms_task = make_shared(task, axioms); shared_ptr eval = nested->bind_task(axioms_task); - return make_shared( + return make_shared< + state_forwarding_evaluator::StateForwardingEvaluator>( task, axioms_task, eval, description, verbosity); } else { /* diff --git a/src/search/evaluators/modify_costs_evaluator.cc b/src/search/evaluators/modify_costs_evaluator.cc index 9db0352367..2b42048f1c 100644 --- a/src/search/evaluators/modify_costs_evaluator.cc +++ b/src/search/evaluators/modify_costs_evaluator.cc @@ -1,6 +1,6 @@ #include "modify_costs_evaluator.h" -#include "simple_task_transforming_evaluator.h" +#include "state_forwarding_evaluator.h" #include "../evaluation_context.h" @@ -21,8 +21,8 @@ TaskIndependentModifyCostsEvaluator::create_task_specific_component( shared_ptr cost_adapted_task = make_shared(task, cost_type); shared_ptr eval = nested->bind_task(cost_adapted_task); - return make_shared( + return make_shared< + state_forwarding_evaluator::StateForwardingEvaluator>( task, cost_adapted_task, eval, description, verbosity); } } diff --git a/src/search/evaluators/simple_task_transforming_evaluator.cc b/src/search/evaluators/state_forwarding_evaluator.cc similarity index 81% rename from src/search/evaluators/simple_task_transforming_evaluator.cc rename to src/search/evaluators/state_forwarding_evaluator.cc index fca1a6af77..0075a967a0 100644 --- a/src/search/evaluators/simple_task_transforming_evaluator.cc +++ b/src/search/evaluators/state_forwarding_evaluator.cc @@ -1,11 +1,11 @@ -#include "simple_task_transforming_evaluator.h" +#include "state_forwarding_evaluator.h" #include "../evaluation_context.h" using namespace std; -namespace simple_task_transforming_evaluator { -SimpleTaskTransformingEvaluator::SimpleTaskTransformingEvaluator( +namespace state_forwarding_evaluator { +StateForwardingEvaluator::StateForwardingEvaluator( const shared_ptr &task, const shared_ptr &transformed_task, const shared_ptr &nested, const string &description, @@ -18,7 +18,7 @@ SimpleTaskTransformingEvaluator::SimpleTaskTransformingEvaluator( path_dependent_evaluators.assign(evals.begin(), evals.end()); } -State SimpleTaskTransformingEvaluator::convert_state(const State &state) const { +State StateForwardingEvaluator::convert_state(const State &state) const { const StateRegistry *existing_state_registry = state.get_registry(); assert(existing_state_registry); if (!state_registry) { @@ -45,11 +45,11 @@ State SimpleTaskTransformingEvaluator::convert_state(const State &state) const { return state_registry->convert_state(state); } -bool SimpleTaskTransformingEvaluator::dead_ends_are_reliable() const { +bool StateForwardingEvaluator::dead_ends_are_reliable() const { return nested->dead_ends_are_reliable(); } -void SimpleTaskTransformingEvaluator::get_path_dependent_evaluators( +void StateForwardingEvaluator::get_path_dependent_evaluators( set &evals) { if (!path_dependent_evaluators.empty()) { /* @@ -64,7 +64,7 @@ void SimpleTaskTransformingEvaluator::get_path_dependent_evaluators( } } -void SimpleTaskTransformingEvaluator::notify_initial_state( +void StateForwardingEvaluator::notify_initial_state( const State &initial_state) { if (!path_dependent_evaluators.empty()) { State converted_initial_state = convert_state(initial_state); @@ -74,7 +74,7 @@ void SimpleTaskTransformingEvaluator::notify_initial_state( } } -void SimpleTaskTransformingEvaluator::notify_state_transition( +void StateForwardingEvaluator::notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) { if (!path_dependent_evaluators.empty()) { State converted_parent_state = convert_state(parent_state); @@ -86,7 +86,7 @@ void SimpleTaskTransformingEvaluator::notify_state_transition( } } -EvaluationResult SimpleTaskTransformingEvaluator::compute_result( +EvaluationResult StateForwardingEvaluator::compute_result( EvaluationContext &eval_context) { State translated_state = convert_state(eval_context.get_state()); EvaluationContext translated_context(eval_context, translated_state); @@ -98,17 +98,15 @@ EvaluationResult SimpleTaskTransformingEvaluator::compute_result( return nested->compute_result(translated_context); } -bool SimpleTaskTransformingEvaluator::does_cache_estimates() const { +bool StateForwardingEvaluator::does_cache_estimates() const { return nested->does_cache_estimates(); } -bool SimpleTaskTransformingEvaluator::is_estimate_cached( - const State &state) const { +bool StateForwardingEvaluator::is_estimate_cached(const State &state) const { return nested->is_estimate_cached(convert_state(state)); } -int SimpleTaskTransformingEvaluator::get_cached_estimate( - const State &state) const { +int StateForwardingEvaluator::get_cached_estimate(const State &state) const { return nested->get_cached_estimate(convert_state(state)); } } diff --git a/src/search/evaluators/simple_task_transforming_evaluator.h b/src/search/evaluators/state_forwarding_evaluator.h similarity index 88% rename from src/search/evaluators/simple_task_transforming_evaluator.h rename to src/search/evaluators/state_forwarding_evaluator.h index d7eb6ecc72..16739251be 100644 --- a/src/search/evaluators/simple_task_transforming_evaluator.h +++ b/src/search/evaluators/state_forwarding_evaluator.h @@ -1,12 +1,12 @@ -#ifndef EVALUATORS_SIMPLE_TASK_TRANSFORMING_EVALUATOR_H -#define EVALUATORS_SIMPLE_TASK_TRANSFORMING_EVALUATOR_H +#ifndef EVALUATORS_STATE_FORWARDING_EVALUATOR_H +#define EVALUATORS_STATE_FORWARDING_EVALUATOR_H #include "../evaluator.h" #include "../state_registry.h" #include -namespace simple_task_transforming_evaluator { +namespace state_forwarding_evaluator { /* This evaluator evaluates a nested evaluator on a transformed version of its own task. The assumption is that the nested evaluator uses a task that has @@ -17,14 +17,14 @@ namespace simple_task_transforming_evaluator { passed on to the nested evaluator as states of the nested task, so the nested evaluator does not have to know about the task transformation. */ -class SimpleTaskTransformingEvaluator : public Evaluator { +class StateForwardingEvaluator : public Evaluator { const std::shared_ptr transformed_task; std::shared_ptr nested; mutable std::unique_ptr state_registry; std::vector path_dependent_evaluators; State convert_state(const State &state) const; public: - SimpleTaskTransformingEvaluator( + StateForwardingEvaluator( const std::shared_ptr &task, const std::shared_ptr &transformed_task, const std::shared_ptr &nested, diff --git a/src/search/state_registry.cc b/src/search/state_registry.cc index a92c0efda8..bdadc17572 100644 --- a/src/search/state_registry.cc +++ b/src/search/state_registry.cc @@ -9,11 +9,9 @@ using namespace std; StateRegistry::StateRegistry(const TaskProxy &task_proxy) - : task_proxy(task_proxy), - num_variables(task_proxy.get_variables().size()) { + : task_proxy(task_proxy), num_variables(task_proxy.get_variables().size()) { } - ExplicitStateRegistry::ExplicitStateRegistry(const TaskProxy &task_proxy) : StateRegistry(task_proxy), state_packer(task_properties::g_state_packers[task_proxy]), @@ -140,9 +138,8 @@ void ExplicitStateRegistry::print_statistics(utils::LogProxy &log) const { registered_states.print_statistics(log); } - DelegatingStateRegistry::DelegatingStateRegistry( - const std::shared_ptr &task, StateRegistry &nested) + const shared_ptr &task, StateRegistry &nested) : StateRegistry(TaskProxy(*task)), task(task), nested(nested) { } From 0d949fd83de6239f70de6e20cb3c6240ef6e4d96 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 21:54:57 +0200 Subject: [PATCH 16/29] allow different registries inside transforming evaluators --- .../evaluators/state_forwarding_evaluator.cc | 53 ++++++++++++------- .../evaluators/state_forwarding_evaluator.h | 7 ++- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/search/evaluators/state_forwarding_evaluator.cc b/src/search/evaluators/state_forwarding_evaluator.cc index 0075a967a0..2ab74b591f 100644 --- a/src/search/evaluators/state_forwarding_evaluator.cc +++ b/src/search/evaluators/state_forwarding_evaluator.cc @@ -19,30 +19,43 @@ StateForwardingEvaluator::StateForwardingEvaluator( } State StateForwardingEvaluator::convert_state(const State &state) const { + /* + issue1227: + * avoid dispatching over incoming registries + -> currently using a heuristic in multiple contexts can lead to states + coming from different registries. For example + let(hcg,eval_modify_costs(cg(),cost_type=plusone), + iterated([eager_greedy([hcg]), eager_greedy([hcg])])) + * avoid const cast? + -> currently needed because we only get the registry from states + passed to this evaluator instead of knowing the registry in the + constructor already. We only get a const registry pointer from + the state but need it to be mutable in the delegating registry. + * avoid mutable state_registry? + -> currently this is used for the delayed initialization which can + happen inside const methods. + * avoid delayed initialization alltogether + Both of the above problems would go away if we could create the + registry in the constructor. + */ const StateRegistry *existing_state_registry = state.get_registry(); + DelegatingStateRegistry *delegating_registry; assert(existing_state_registry); - if (!state_registry) { - /* - issue1227: - * avoid const cast? - -> currently needed because we only get the registry from states - passed to this evaluator instead of knowing the registry in the - constructor already. We only get a const registry pointer from - the state but need it to be mutable in the delegating registry. - * avoid mutable state_registry? - -> currently this is used for the delayed initialization which can - happen inside const methods. - * avoid delayed initialization alltogether - Both of the above problems would go away if we could create the - registry in the constructor. - */ - state_registry = make_unique( - transformed_task, - const_cast(*existing_state_registry)); + if (existing_state_registry == last_state_registry_key) { + delegating_registry = last_state_registry; } else { - state_registry->assert_nested_is(*existing_state_registry); + if (!state_registries.contains(existing_state_registry)) { + state_registries.emplace( + existing_state_registry, + make_unique( + transformed_task, + const_cast(*existing_state_registry))); + } + delegating_registry = state_registries.at(existing_state_registry).get(); + last_state_registry_key = existing_state_registry; + last_state_registry = delegating_registry; } - return state_registry->convert_state(state); + return delegating_registry->convert_state(state); } bool StateForwardingEvaluator::dead_ends_are_reliable() const { diff --git a/src/search/evaluators/state_forwarding_evaluator.h b/src/search/evaluators/state_forwarding_evaluator.h index 16739251be..506e4644af 100644 --- a/src/search/evaluators/state_forwarding_evaluator.h +++ b/src/search/evaluators/state_forwarding_evaluator.h @@ -5,6 +5,7 @@ #include "../state_registry.h" #include +#include namespace state_forwarding_evaluator { /* @@ -20,7 +21,11 @@ namespace state_forwarding_evaluator { class StateForwardingEvaluator : public Evaluator { const std::shared_ptr transformed_task; std::shared_ptr nested; - mutable std::unique_ptr state_registry; + mutable std::unordered_map< + const StateRegistry *, std::unique_ptr> + state_registries; + mutable const StateRegistry *last_state_registry_key; + mutable DelegatingStateRegistry *last_state_registry; std::vector path_dependent_evaluators; State convert_state(const State &state) const; public: From 2ad1ac550dc5d085ec2927cc4b69023dbbb6c1fd Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 24 Jul 2026 22:13:11 +0200 Subject: [PATCH 17/29] style --- src/search/evaluators/state_forwarding_evaluator.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/search/evaluators/state_forwarding_evaluator.cc b/src/search/evaluators/state_forwarding_evaluator.cc index 2ab74b591f..6e278a3d86 100644 --- a/src/search/evaluators/state_forwarding_evaluator.cc +++ b/src/search/evaluators/state_forwarding_evaluator.cc @@ -51,7 +51,8 @@ State StateForwardingEvaluator::convert_state(const State &state) const { transformed_task, const_cast(*existing_state_registry))); } - delegating_registry = state_registries.at(existing_state_registry).get(); + delegating_registry = + state_registries.at(existing_state_registry).get(); last_state_registry_key = existing_state_registry; last_state_registry = delegating_registry; } From 1e784b5adc7a945dfcd42c7faf57f28cd52e9810 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 00:49:37 +0200 Subject: [PATCH 18/29] remove convert_ancestor_state from heuristics --- .../additive_cartesian_heuristic.cc | 3 +- .../additive_cartesian_heuristic.h | 2 +- src/search/heuristic.cc | 4 --- src/search/heuristic.h | 4 +-- src/search/heuristics/additive_heuristic.cc | 3 +- src/search/heuristics/additive_heuristic.h | 2 +- .../heuristics/blind_search_heuristic.cc | 3 +- .../heuristics/blind_search_heuristic.h | 2 +- src/search/heuristics/cea_heuristic.cc | 4 +-- src/search/heuristics/cea_heuristic.h | 2 +- src/search/heuristics/cg_heuristic.cc | 3 +- src/search/heuristics/cg_heuristic.h | 2 +- src/search/heuristics/ff_heuristic.cc | 3 +- src/search/heuristics/ff_heuristic.h | 2 +- src/search/heuristics/goal_count_heuristic.cc | 3 +- src/search/heuristics/goal_count_heuristic.h | 2 +- src/search/heuristics/hm_heuristic.cc | 3 +- src/search/heuristics/hm_heuristic.h | 2 +- src/search/heuristics/lm_cut_heuristic.cc | 3 +- src/search/heuristics/lm_cut_heuristic.h | 2 +- src/search/heuristics/max_heuristic.cc | 4 +-- src/search/heuristics/max_heuristic.h | 2 +- .../landmark_cost_partitioning_algorithms.cc | 16 ++++----- .../landmark_cost_partitioning_algorithms.h | 7 ++-- .../landmark_cost_partitioning_heuristic.cc | 5 ++- .../landmark_cost_partitioning_heuristic.h | 2 +- src/search/landmarks/landmark_heuristic.cc | 7 ++-- src/search/landmarks/landmark_heuristic.h | 4 +-- .../landmarks/landmark_status_manager.cc | 36 +++++++++---------- .../landmarks/landmark_status_manager.h | 12 +++---- .../landmarks/landmark_sum_heuristic.cc | 6 ++-- src/search/landmarks/landmark_sum_heuristic.h | 2 +- .../merge_and_shrink_heuristic.cc | 3 +- .../merge_and_shrink_heuristic.h | 2 +- .../operator_counting_heuristic.cc | 3 +- .../operator_counting_heuristic.h | 2 +- src/search/pdbs/canonical_pdbs_heuristic.cc | 3 +- src/search/pdbs/canonical_pdbs_heuristic.h | 2 +- src/search/pdbs/pdb_heuristic.cc | 3 +- src/search/pdbs/pdb_heuristic.h | 2 +- src/search/pdbs/zero_one_pdbs_heuristic.cc | 3 +- src/search/pdbs/zero_one_pdbs_heuristic.h | 2 +- src/search/potentials/potential_heuristic.cc | 3 +- src/search/potentials/potential_heuristic.h | 2 +- .../potentials/potential_max_heuristic.cc | 3 +- .../potentials/potential_max_heuristic.h | 2 +- 46 files changed, 78 insertions(+), 114 deletions(-) diff --git a/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc b/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc index 16543123d5..2587552e1e 100644 --- a/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc +++ b/src/search/cartesian_abstractions/additive_cartesian_heuristic.cc @@ -44,8 +44,7 @@ AdditiveCartesianHeuristic::AdditiveCartesianHeuristic( use_general_costs, random_seed, log)) { } -int AdditiveCartesianHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int AdditiveCartesianHeuristic::compute_heuristic(const State &state) { int sum_h = 0; for (const CartesianHeuristicFunction &function : heuristic_functions) { int value = function.get_value(state); diff --git a/src/search/cartesian_abstractions/additive_cartesian_heuristic.h b/src/search/cartesian_abstractions/additive_cartesian_heuristic.h index a264fbe00b..2920089a5f 100644 --- a/src/search/cartesian_abstractions/additive_cartesian_heuristic.h +++ b/src/search/cartesian_abstractions/additive_cartesian_heuristic.h @@ -18,7 +18,7 @@ class AdditiveCartesianHeuristic : public Heuristic { const std::vector heuristic_functions; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: AdditiveCartesianHeuristic( diff --git a/src/search/heuristic.cc b/src/search/heuristic.cc index 166e9115af..4721cfe120 100644 --- a/src/search/heuristic.cc +++ b/src/search/heuristic.cc @@ -27,10 +27,6 @@ void Heuristic::set_preferred(const OperatorProxy &op) { op.get_ancestor_operator_id(tasks::g_root_task.get())); } -State Heuristic::convert_ancestor_state(const State &ancestor_state) const { - return task_proxy.convert_ancestor_state(ancestor_state); -} - void add_heuristic_options_to_feature( plugins::Feature &feature, const string &description) { feature.add_option( diff --git a/src/search/heuristic.h b/src/search/heuristic.h index e0f9276779..61447ccb4d 100644 --- a/src/search/heuristic.h +++ b/src/search/heuristic.h @@ -57,7 +57,7 @@ class Heuristic : public Evaluator { NO_VALUE = -2 }; - virtual int compute_heuristic(const State &ancestor_state) = 0; + virtual int compute_heuristic(const State &state) = 0; /* Usage note: Marking the same operator as preferred multiple times @@ -66,8 +66,6 @@ class Heuristic : public Evaluator { */ void set_preferred(const OperatorProxy &op); - State convert_ancestor_state(const State &ancestor_state) const; - public: Heuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/heuristics/additive_heuristic.cc b/src/search/heuristics/additive_heuristic.cc index 2723ac3c8f..5150f6199a 100644 --- a/src/search/heuristics/additive_heuristic.cc +++ b/src/search/heuristics/additive_heuristic.cc @@ -131,8 +131,7 @@ int AdditiveHeuristic::compute_add_and_ff(const State &state) { return total_cost; } -int AdditiveHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int AdditiveHeuristic::compute_heuristic(const State &state) { int h = compute_add_and_ff(state); if (h != DEAD_END) { for (PropID goal_id : goal_propositions) diff --git a/src/search/heuristics/additive_heuristic.h b/src/search/heuristics/additive_heuristic.h index 88fe2ff8b4..8201e1714b 100644 --- a/src/search/heuristics/additive_heuristic.h +++ b/src/search/heuristics/additive_heuristic.h @@ -60,7 +60,7 @@ class AdditiveHeuristic : public relaxation_heuristic::RelaxationHeuristic { void write_overflow_warning(); protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; // Common part of h^add and h^ff computation. int compute_add_and_ff(const State &state); diff --git a/src/search/heuristics/blind_search_heuristic.cc b/src/search/heuristics/blind_search_heuristic.cc index 7c675764c4..419a8fe4d0 100644 --- a/src/search/heuristics/blind_search_heuristic.cc +++ b/src/search/heuristics/blind_search_heuristic.cc @@ -21,8 +21,7 @@ BlindSearchHeuristic::BlindSearchHeuristic( } } -int BlindSearchHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int BlindSearchHeuristic::compute_heuristic(const State &state) { if (task_properties::is_goal_state(task_proxy, state)) return 0; else diff --git a/src/search/heuristics/blind_search_heuristic.h b/src/search/heuristics/blind_search_heuristic.h index 0e51232f4e..27af4cbaf4 100644 --- a/src/search/heuristics/blind_search_heuristic.h +++ b/src/search/heuristics/blind_search_heuristic.h @@ -7,7 +7,7 @@ namespace blind_search_heuristic { class BlindSearchHeuristic : public Heuristic { int min_operator_cost; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: BlindSearchHeuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index eef1ce2ec4..a1191f4b37 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -393,9 +393,7 @@ void ContextEnhancedAdditiveHeuristic::mark_helpful_transitions( } } -int ContextEnhancedAdditiveHeuristic::compute_heuristic( - const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int ContextEnhancedAdditiveHeuristic::compute_heuristic(const State &state) { initialize_heap(); goal_problem->base_priority = -1; for (LocalProblem *problem : local_problems) diff --git a/src/search/heuristics/cea_heuristic.h b/src/search/heuristics/cea_heuristic.h index f7c37bd2b8..00bd6fbfed 100644 --- a/src/search/heuristics/cea_heuristic.h +++ b/src/search/heuristics/cea_heuristic.h @@ -51,7 +51,7 @@ class ContextEnhancedAdditiveHeuristic : public Heuristic { // Clears "reached_by" of visited nodes as a side effect to avoid // recursing to the same node again. protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: ContextEnhancedAdditiveHeuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index 9247b5f480..573b7ffd4a 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -54,8 +54,7 @@ bool CGHeuristic::dead_ends_are_reliable() const { return false; } -int CGHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int CGHeuristic::compute_heuristic(const State &state) { setup_domain_transition_graphs(); int heuristic = 0; diff --git a/src/search/heuristics/cg_heuristic.h b/src/search/heuristics/cg_heuristic.h index e7a1cf653b..60807ec617 100644 --- a/src/search/heuristics/cg_heuristic.h +++ b/src/search/heuristics/cg_heuristic.h @@ -38,7 +38,7 @@ class CGHeuristic : public Heuristic { const State &state, domain_transition_graph::DomainTransitionGraph *dtg, int to); protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: CGHeuristic( const std::shared_ptr &task, int max_cache_size, diff --git a/src/search/heuristics/ff_heuristic.cc b/src/search/heuristics/ff_heuristic.cc index ffb201ccde..2c02d30c96 100644 --- a/src/search/heuristics/ff_heuristic.cc +++ b/src/search/heuristics/ff_heuristic.cc @@ -50,8 +50,7 @@ void FFHeuristic::mark_preferred_operators_and_relaxed_plan( } } -int FFHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int FFHeuristic::compute_heuristic(const State &state) { int h_add = compute_add_and_ff(state); if (h_add == DEAD_END) return h_add; diff --git a/src/search/heuristics/ff_heuristic.h b/src/search/heuristics/ff_heuristic.h index 544a2809e8..6a905613a3 100644 --- a/src/search/heuristics/ff_heuristic.h +++ b/src/search/heuristics/ff_heuristic.h @@ -30,7 +30,7 @@ class FFHeuristic : public additive_heuristic::AdditiveHeuristic { void mark_preferred_operators_and_relaxed_plan( const State &state, PropID goal_id); protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: FFHeuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/heuristics/goal_count_heuristic.cc b/src/search/heuristics/goal_count_heuristic.cc index bc1eee7efc..60c546d5f3 100644 --- a/src/search/heuristics/goal_count_heuristic.cc +++ b/src/search/heuristics/goal_count_heuristic.cc @@ -16,8 +16,7 @@ GoalCountHeuristic::GoalCountHeuristic( } } -int GoalCountHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int GoalCountHeuristic::compute_heuristic(const State &state) { int unsatisfied_goal_count = 0; for (FactProxy goal : task_proxy.get_goals()) { diff --git a/src/search/heuristics/goal_count_heuristic.h b/src/search/heuristics/goal_count_heuristic.h index be421b0594..4683a26879 100644 --- a/src/search/heuristics/goal_count_heuristic.h +++ b/src/search/heuristics/goal_count_heuristic.h @@ -6,7 +6,7 @@ namespace goal_count_heuristic { class GoalCountHeuristic : public Heuristic { protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: GoalCountHeuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/heuristics/hm_heuristic.cc b/src/search/heuristics/hm_heuristic.cc index fde0d7ba75..aba6a1ad92 100644 --- a/src/search/heuristics/hm_heuristic.cc +++ b/src/search/heuristics/hm_heuristic.cc @@ -31,8 +31,7 @@ bool HMHeuristic::dead_ends_are_reliable() const { return !task_properties::has_axioms(task_proxy) && !has_cond_effects; } -int HMHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int HMHeuristic::compute_heuristic(const State &state) { if (task_properties::is_goal_state(task_proxy, state)) { return 0; } else { diff --git a/src/search/heuristics/hm_heuristic.h b/src/search/heuristics/hm_heuristic.h index b1eae993b1..6f32fb75ce 100644 --- a/src/search/heuristics/hm_heuristic.h +++ b/src/search/heuristics/hm_heuristic.h @@ -53,7 +53,7 @@ class HMHeuristic : public Heuristic { void dump_table() const; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: HMHeuristic( diff --git a/src/search/heuristics/lm_cut_heuristic.cc b/src/search/heuristics/lm_cut_heuristic.cc index ad490f1ce7..898d35cf5d 100644 --- a/src/search/heuristics/lm_cut_heuristic.cc +++ b/src/search/heuristics/lm_cut_heuristic.cc @@ -23,8 +23,7 @@ LandmarkCutHeuristic::LandmarkCutHeuristic( } } -int LandmarkCutHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int LandmarkCutHeuristic::compute_heuristic(const State &state) { int total_cost = 0; bool dead_end = landmark_generator->compute_landmarks( state, [&total_cost](int cut_cost) { total_cost += cut_cost; }, diff --git a/src/search/heuristics/lm_cut_heuristic.h b/src/search/heuristics/lm_cut_heuristic.h index 517a145f6b..7fced564e8 100644 --- a/src/search/heuristics/lm_cut_heuristic.h +++ b/src/search/heuristics/lm_cut_heuristic.h @@ -11,7 +11,7 @@ class LandmarkCutLandmarks; class LandmarkCutHeuristic : public Heuristic { std::unique_ptr landmark_generator; - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: LandmarkCutHeuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/heuristics/max_heuristic.cc b/src/search/heuristics/max_heuristic.cc index 7ddcf911ee..29f91b1085 100644 --- a/src/search/heuristics/max_heuristic.cc +++ b/src/search/heuristics/max_heuristic.cc @@ -83,9 +83,7 @@ void HSPMaxHeuristic::relaxed_exploration() { } } -int HSPMaxHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); - +int HSPMaxHeuristic::compute_heuristic(const State &state) { setup_exploration_queue(); setup_exploration_queue_state(state); relaxed_exploration(); diff --git a/src/search/heuristics/max_heuristic.h b/src/search/heuristics/max_heuristic.h index 44bbccd74c..b6d8703858 100644 --- a/src/search/heuristics/max_heuristic.h +++ b/src/search/heuristics/max_heuristic.h @@ -31,7 +31,7 @@ class HSPMaxHeuristic : public relaxation_heuristic::RelaxationHeuristic { assert(prop->cost != -1 && prop->cost <= cost); } protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: HSPMaxHeuristic( const std::shared_ptr &task, bool cache_estimates, diff --git a/src/search/landmarks/landmark_cost_partitioning_algorithms.cc b/src/search/landmarks/landmark_cost_partitioning_algorithms.cc index 18a1c0b4bc..e7b04e4c86 100644 --- a/src/search/landmarks/landmark_cost_partitioning_algorithms.cc +++ b/src/search/landmarks/landmark_cost_partitioning_algorithms.cc @@ -132,15 +132,13 @@ double UniformCostPartitioningAlgorithm::third_pass( } double UniformCostPartitioningAlgorithm::get_cost_partitioned_heuristic_value( - const LandmarkStatusManager &landmark_status_manager, - const State &ancestor_state) { + const LandmarkStatusManager &landmark_status_manager, const State &state) { vector landmarks_achieved_by_operator(operator_costs.size(), 0); vector action_landmarks(operator_costs.size(), false); - ConstBitsetView past = - landmark_status_manager.get_past_landmarks(ancestor_state); + ConstBitsetView past = landmark_status_manager.get_past_landmarks(state); ConstBitsetView future = - landmark_status_manager.get_future_landmarks(ancestor_state); + landmark_status_manager.get_future_landmarks(state); const double cost_of_action_landmarks = first_pass( landmarks_achieved_by_operator, action_landmarks, past, future); @@ -262,15 +260,13 @@ bool OptimalCostPartitioningAlgorithm::define_constraint_matrix( } double OptimalCostPartitioningAlgorithm::get_cost_partitioned_heuristic_value( - const LandmarkStatusManager &landmark_status_manager, - const State &ancestor_state) { + const LandmarkStatusManager &landmark_status_manager, const State &state) { /* TODO: We could also do the same thing with action landmarks we do in the uniform cost partitioning case. */ - ConstBitsetView past = - landmark_status_manager.get_past_landmarks(ancestor_state); + ConstBitsetView past = landmark_status_manager.get_past_landmarks(state); ConstBitsetView future = - landmark_status_manager.get_future_landmarks(ancestor_state); + landmark_status_manager.get_future_landmarks(state); const int num_cols = landmark_graph.get_num_landmarks(); set_lp_bounds(future, num_cols); diff --git a/src/search/landmarks/landmark_cost_partitioning_algorithms.h b/src/search/landmarks/landmark_cost_partitioning_algorithms.h index 7961b77d34..5255fcfa94 100644 --- a/src/search/landmarks/landmark_cost_partitioning_algorithms.h +++ b/src/search/landmarks/landmark_cost_partitioning_algorithms.h @@ -28,8 +28,7 @@ class CostPartitioningAlgorithm { virtual ~CostPartitioningAlgorithm() = default; virtual double get_cost_partitioned_heuristic_value( - const LandmarkStatusManager &lm_status_manager, - const State &ancestor_state) = 0; + const LandmarkStatusManager &lm_status_manager, const State &state) = 0; }; class UniformCostPartitioningAlgorithm : public CostPartitioningAlgorithm { @@ -63,7 +62,7 @@ class UniformCostPartitioningAlgorithm : public CostPartitioningAlgorithm { virtual double get_cost_partitioned_heuristic_value( const LandmarkStatusManager &landmark_status_manager, - const State &ancestor_state) override; + const State &state) override; }; class OptimalCostPartitioningAlgorithm : public CostPartitioningAlgorithm { @@ -90,7 +89,7 @@ class OptimalCostPartitioningAlgorithm : public CostPartitioningAlgorithm { virtual double get_cost_partitioned_heuristic_value( const LandmarkStatusManager &landmark_status_manager, - const State &ancestor_state) override; + const State &state) override; }; } diff --git a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc index 50c25ddae9..99f9d87749 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.cc +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.cc @@ -63,13 +63,12 @@ void LandmarkCostPartitioningHeuristic::set_cost_partitioning_algorithm( } } -int LandmarkCostPartitioningHeuristic::get_heuristic_value( - const State &ancestor_state) { +int LandmarkCostPartitioningHeuristic::get_heuristic_value(const State &state) { constexpr double epsilon = 0.01; double h_val = cost_partitioning_algorithm->get_cost_partitioned_heuristic_value( - *landmark_status_manager, ancestor_state); + *landmark_status_manager, 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 57da9a89f6..496325a6d0 100644 --- a/src/search/landmarks/landmark_cost_partitioning_heuristic.h +++ b/src/search/landmarks/landmark_cost_partitioning_heuristic.h @@ -22,7 +22,7 @@ class LandmarkCostPartitioningHeuristic : public LandmarkHeuristic { CostPartitioningMethod cost_partitioning, lp::LPSolverType lpsolver, bool use_action_landmarks); - int get_heuristic_value(const State &ancestor_state) override; + int get_heuristic_value(const State &state) override; public: LandmarkCostPartitioningHeuristic( const std::shared_ptr &task, diff --git a/src/search/landmarks/landmark_heuristic.cc b/src/search/landmarks/landmark_heuristic.cc index ab617566fe..03892e6937 100644 --- a/src/search/landmarks/landmark_heuristic.cc +++ b/src/search/landmarks/landmark_heuristic.cc @@ -162,7 +162,7 @@ void LandmarkHeuristic::generate_preferred_operators( } } -int LandmarkHeuristic::compute_heuristic(const State &ancestor_state) { +int LandmarkHeuristic::compute_heuristic(const State &state) { /* The path-dependent landmark heuristics are somewhat ill-defined for states not reachable from the initial state of a planning task. We therefore @@ -187,11 +187,10 @@ int LandmarkHeuristic::compute_heuristic(const State &ancestor_state) { if (initial_landmark_graph_has_cycle_of_natural_orderings) { return DEAD_END; } - int h = get_heuristic_value(ancestor_state); + int h = get_heuristic_value(state); if (use_preferred_operators) { ConstBitsetView future = - landmark_status_manager->get_future_landmarks(ancestor_state); - State state = convert_ancestor_state(ancestor_state); + landmark_status_manager->get_future_landmarks(state); generate_preferred_operators(state, future); } return h; diff --git a/src/search/landmarks/landmark_heuristic.h b/src/search/landmarks/landmark_heuristic.h index 1dd24e798c..ce9f963c89 100644 --- a/src/search/landmarks/landmark_heuristic.h +++ b/src/search/landmarks/landmark_heuristic.h @@ -37,14 +37,14 @@ class LandmarkHeuristic : public Heuristic { void compute_landmark_graph( const std::shared_ptr &landmark_factory); - virtual int get_heuristic_value(const State &ancestor_state) = 0; + virtual int get_heuristic_value(const State &state) = 0; bool operator_is_preferred( const OperatorProxy &op, const State &state, ConstBitsetView &future); void compute_landmarks_achieved_by_atom(); void generate_preferred_operators( const State &state, ConstBitsetView &future); - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: LandmarkHeuristic( const std::shared_ptr &task, bool use_preferred_operators, diff --git a/src/search/landmarks/landmark_status_manager.cc b/src/search/landmarks/landmark_status_manager.cc index 9cd20f13c1..e78d0f2bab 100644 --- a/src/search/landmarks/landmark_status_manager.cc +++ b/src/search/landmarks/landmark_status_manager.cc @@ -133,18 +133,17 @@ void LandmarkStatusManager::progress_initial_state(const State &initial_state) { } void LandmarkStatusManager::progress( - const State &parent_ancestor_state, OperatorID, - const State &ancestor_state) { - if (ancestor_state == parent_ancestor_state) { + const State &parent_state, OperatorID, const State &state) { + if (state == parent_state) { // This can happen, e.g., in Satellite-01. return; } - ConstBitsetView parent_past = get_past_landmarks(parent_ancestor_state); - BitsetView past = get_past_landmarks(ancestor_state); + ConstBitsetView parent_past = get_past_landmarks(parent_state); + BitsetView past = get_past_landmarks(state); - ConstBitsetView parent_future = get_future_landmarks(parent_ancestor_state); - BitsetView future = get_future_landmarks(ancestor_state); + ConstBitsetView parent_future = get_future_landmarks(parent_state); + BitsetView future = get_future_landmarks(state); assert(past.size() == landmark_graph.get_num_landmarks()); assert(parent_past.size() == landmark_graph.get_num_landmarks()); @@ -152,22 +151,21 @@ void LandmarkStatusManager::progress( assert(parent_future.size() == landmark_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); + parent_past, parent_future, parent_state, past, future, state); + progress_goals(state, future); + progress_greedy_necessary_orderings(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) { + const State &parent_state, BitsetView &past, BitsetView &future, + const State &state) { for (const auto &node : landmark_graph) { int id = node->get_id(); const Landmark &landmark = node->get_landmark(); if (parent_future.test(id)) { - if (!landmark.is_true_in_state(ancestor_state)) { + if (!landmark.is_true_in_state(state)) { /* A landmark that is future in the parent remains future if it does not hold in the current state. If it also @@ -177,7 +175,7 @@ void LandmarkStatusManager::progress_landmarks( if (!parent_past.test(id)) { past.reset(id); } - } else if (landmark.is_true_in_state(parent_ancestor_state)) { + } else if (landmark.is_true_in_state(parent_state)) { /* If the landmark held in the parent already, then it was not added by this transition and should remain @@ -191,22 +189,22 @@ void LandmarkStatusManager::progress_landmarks( } void LandmarkStatusManager::progress_goals( - const State &ancestor_state, BitsetView &future) { + const State &state, BitsetView &future) { for (auto &node : goal_landmarks) { - if (!node->get_landmark().is_true_in_state(ancestor_state)) { + if (!node->get_landmark().is_true_in_state(state)) { future.set(node->get_id()); } } } void LandmarkStatusManager::progress_greedy_necessary_orderings( - const State &ancestor_state, const BitsetView &past, BitsetView &future) { + const State &state, const BitsetView &past, BitsetView &future) { for (auto &[tail, children] : greedy_necessary_children) { const Landmark &landmark = tail->get_landmark(); assert(!children.empty()); for (auto &child : children) { if (!past.test(child->get_id()) && - !landmark.is_true_in_state(ancestor_state)) { + !landmark.is_true_in_state(state)) { future.set(tail->get_id()); break; } diff --git a/src/search/landmarks/landmark_status_manager.h b/src/search/landmarks/landmark_status_manager.h index 7ab9e9e4a0..10cb92cfad 100644 --- a/src/search/landmarks/landmark_status_manager.h +++ b/src/search/landmarks/landmark_status_manager.h @@ -24,12 +24,11 @@ class LandmarkStatusManager { 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); + const State &parent_state, BitsetView &past, BitsetView &future, + const State &state); + void progress_goals(const State &state, BitsetView &future); void progress_greedy_necessary_orderings( - const State &ancestor_state, const BitsetView &past, - BitsetView &future); + const State &state, const BitsetView &past, BitsetView &future); void progress_reasonable_orderings( const BitsetView &past, BitsetView &future); public: @@ -45,8 +44,7 @@ class LandmarkStatusManager { void progress_initial_state(const State &initial_state); void progress( - const State &parent_ancestor_state, OperatorID op_id, - const State &ancestor_state); + const State &parent_state, OperatorID op_id, const State &state); }; } diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 503cf454ea..5dfc2c41b5 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -86,12 +86,12 @@ void LandmarkSumHeuristic::compute_landmark_costs() { } } -int LandmarkSumHeuristic::get_heuristic_value(const State &ancestor_state) { +int LandmarkSumHeuristic::get_heuristic_value(const State &state) { int h = 0; ConstBitsetView past = - landmark_status_manager->get_past_landmarks(ancestor_state); + landmark_status_manager->get_past_landmarks(state); ConstBitsetView future = - landmark_status_manager->get_future_landmarks(ancestor_state); + landmark_status_manager->get_future_landmarks(state); for (int id = 0; id < landmark_graph->get_num_landmarks(); ++id) { if (future.test(id)) { const int min_achiever_cost = past.test(id) diff --git a/src/search/landmarks/landmark_sum_heuristic.h b/src/search/landmarks/landmark_sum_heuristic.h index 615acc80e9..b6377d9742 100644 --- a/src/search/landmarks/landmark_sum_heuristic.h +++ b/src/search/landmarks/landmark_sum_heuristic.h @@ -20,7 +20,7 @@ class LandmarkSumHeuristic : public LandmarkHeuristic { const std::unordered_set &achievers) const; void compute_landmark_costs(); - int get_heuristic_value(const State &ancestor_state) override; + int get_heuristic_value(const State &state) override; public: LandmarkSumHeuristic( const std::shared_ptr &task, diff --git a/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc b/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc index 38b51ea609..1e320cf46a 100644 --- a/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc +++ b/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc @@ -123,8 +123,7 @@ void MergeAndShrinkHeuristic::extract_factors(FactoredTransitionSystem &fts) { } } -int MergeAndShrinkHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int MergeAndShrinkHeuristic::compute_heuristic(const State &state) { int heuristic = 0; for (const unique_ptr &mas_representation : mas_representations) { diff --git a/src/search/merge_and_shrink/merge_and_shrink_heuristic.h b/src/search/merge_and_shrink/merge_and_shrink_heuristic.h index 07e7bb473d..1f03bc22b5 100644 --- a/src/search/merge_and_shrink/merge_and_shrink_heuristic.h +++ b/src/search/merge_and_shrink/merge_and_shrink_heuristic.h @@ -23,7 +23,7 @@ class MergeAndShrinkHeuristic : public Heuristic { void extract_nontrivial_factors(FactoredTransitionSystem &fts); void extract_factors(FactoredTransitionSystem &fts); protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: MergeAndShrinkHeuristic( const std::shared_ptr &task, diff --git a/src/search/operator_counting/operator_counting_heuristic.cc b/src/search/operator_counting/operator_counting_heuristic.cc index 7dee9c117e..5bf5df23ab 100644 --- a/src/search/operator_counting/operator_counting_heuristic.cc +++ b/src/search/operator_counting/operator_counting_heuristic.cc @@ -41,8 +41,7 @@ OperatorCountingHeuristic::OperatorCountingHeuristic( lp_solver.load_problem(lp); } -int OperatorCountingHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int OperatorCountingHeuristic::compute_heuristic(const State &state) { assert(!lp_solver.has_temporary_constraints()); for (const auto &generator : constraint_generators) { bool dead_end = generator->update_constraints(state, lp_solver); diff --git a/src/search/operator_counting/operator_counting_heuristic.h b/src/search/operator_counting/operator_counting_heuristic.h index a10f926e76..5cc1e06624 100644 --- a/src/search/operator_counting/operator_counting_heuristic.h +++ b/src/search/operator_counting/operator_counting_heuristic.h @@ -15,7 +15,7 @@ class OperatorCountingHeuristic : public Heuristic { std::vector> constraint_generators; lp::LPSolver lp_solver; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: OperatorCountingHeuristic( const std::shared_ptr &task, diff --git a/src/search/pdbs/canonical_pdbs_heuristic.cc b/src/search/pdbs/canonical_pdbs_heuristic.cc index af19a69bb5..42c11e5a34 100644 --- a/src/search/pdbs/canonical_pdbs_heuristic.cc +++ b/src/search/pdbs/canonical_pdbs_heuristic.cc @@ -66,8 +66,7 @@ CanonicalPDBsHeuristic::CanonicalPDBsHeuristic( get_canonical_pdbs(task, patterns, max_time_dominance_pruning, log)) { } -int CanonicalPDBsHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int CanonicalPDBsHeuristic::compute_heuristic(const State &state) { int h = canonical_pdbs.get_value(state); if (h == numeric_limits::max()) { return DEAD_END; diff --git a/src/search/pdbs/canonical_pdbs_heuristic.h b/src/search/pdbs/canonical_pdbs_heuristic.h index f10d883fd5..5668d962be 100644 --- a/src/search/pdbs/canonical_pdbs_heuristic.h +++ b/src/search/pdbs/canonical_pdbs_heuristic.h @@ -16,7 +16,7 @@ class CanonicalPDBsHeuristic : public Heuristic { CanonicalPDBs canonical_pdbs; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: CanonicalPDBsHeuristic( diff --git a/src/search/pdbs/pdb_heuristic.cc b/src/search/pdbs/pdb_heuristic.cc index 1704ba9345..cf5d26bcb3 100644 --- a/src/search/pdbs/pdb_heuristic.cc +++ b/src/search/pdbs/pdb_heuristic.cc @@ -26,8 +26,7 @@ PDBHeuristic::PDBHeuristic( pdb(get_pdb_from_generator(task, pattern)) { } -int PDBHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int PDBHeuristic::compute_heuristic(const State &state) { int h = pdb->get_value(state.get_unpacked_values()); if (h == numeric_limits::max()) return DEAD_END; diff --git a/src/search/pdbs/pdb_heuristic.h b/src/search/pdbs/pdb_heuristic.h index 828cc81ada..a2172ac109 100644 --- a/src/search/pdbs/pdb_heuristic.h +++ b/src/search/pdbs/pdb_heuristic.h @@ -12,7 +12,7 @@ class PatternDatabase; class PDBHeuristic : public Heuristic { std::shared_ptr pdb; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: /* Important: It is assumed that the pattern (passed via diff --git a/src/search/pdbs/zero_one_pdbs_heuristic.cc b/src/search/pdbs/zero_one_pdbs_heuristic.cc index 2f83bc5d3e..796ee8690e 100644 --- a/src/search/pdbs/zero_one_pdbs_heuristic.cc +++ b/src/search/pdbs/zero_one_pdbs_heuristic.cc @@ -26,8 +26,7 @@ ZeroOnePDBsHeuristic::ZeroOnePDBsHeuristic( zero_one_pdbs(get_zero_one_pdbs_from_generator(task, patterns)) { } -int ZeroOnePDBsHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int ZeroOnePDBsHeuristic::compute_heuristic(const State &state) { int h = zero_one_pdbs.get_value(state); if (h == numeric_limits::max()) return DEAD_END; diff --git a/src/search/pdbs/zero_one_pdbs_heuristic.h b/src/search/pdbs/zero_one_pdbs_heuristic.h index 6249497b12..dcc937bb0c 100644 --- a/src/search/pdbs/zero_one_pdbs_heuristic.h +++ b/src/search/pdbs/zero_one_pdbs_heuristic.h @@ -12,7 +12,7 @@ class PatternDatabase; class ZeroOnePDBsHeuristic : public Heuristic { ZeroOnePDBs zero_one_pdbs; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: ZeroOnePDBsHeuristic( const std::shared_ptr &task, diff --git a/src/search/potentials/potential_heuristic.cc b/src/search/potentials/potential_heuristic.cc index 6e7e6f4598..f49f891ef4 100644 --- a/src/search/potentials/potential_heuristic.cc +++ b/src/search/potentials/potential_heuristic.cc @@ -15,8 +15,7 @@ PotentialHeuristic::PotentialHeuristic( function(move(function)) { } -int PotentialHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int PotentialHeuristic::compute_heuristic(const State &state) { return max(0, function->get_value(state)); } } diff --git a/src/search/potentials/potential_heuristic.h b/src/search/potentials/potential_heuristic.h index f2f4365635..a9bc10e718 100644 --- a/src/search/potentials/potential_heuristic.h +++ b/src/search/potentials/potential_heuristic.h @@ -15,7 +15,7 @@ class PotentialHeuristic : public Heuristic { std::unique_ptr function; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: PotentialHeuristic( diff --git a/src/search/potentials/potential_max_heuristic.cc b/src/search/potentials/potential_max_heuristic.cc index 1a07154e5f..e56c474d7f 100644 --- a/src/search/potentials/potential_max_heuristic.cc +++ b/src/search/potentials/potential_max_heuristic.cc @@ -15,8 +15,7 @@ PotentialMaxHeuristic::PotentialMaxHeuristic( functions(move(functions)) { } -int PotentialMaxHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); +int PotentialMaxHeuristic::compute_heuristic(const State &state) { int value = 0; for (auto &function : functions) { value = max(value, function->get_value(state)); diff --git a/src/search/potentials/potential_max_heuristic.h b/src/search/potentials/potential_max_heuristic.h index c2bad50648..687f7c091c 100644 --- a/src/search/potentials/potential_max_heuristic.h +++ b/src/search/potentials/potential_max_heuristic.h @@ -16,7 +16,7 @@ class PotentialMaxHeuristic : public Heuristic { std::vector> functions; protected: - virtual int compute_heuristic(const State &ancestor_state) override; + virtual int compute_heuristic(const State &state) override; public: PotentialMaxHeuristic( From 52250d3234119dca0a22cce5be29e8fab2b67f32 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 00:58:44 +0200 Subject: [PATCH 19/29] remove get_ancestor_operator_id --- src/search/heuristic.cc | 3 +-- src/search/task_proxy.h | 11 ----------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/search/heuristic.cc b/src/search/heuristic.cc index 4721cfe120..c722e8f51b 100644 --- a/src/search/heuristic.cc +++ b/src/search/heuristic.cc @@ -23,8 +23,7 @@ Heuristic::Heuristic( } void Heuristic::set_preferred(const OperatorProxy &op) { - preferred_operators.insert( - op.get_ancestor_operator_id(tasks::g_root_task.get())); + preferred_operators.insert(OperatorID(op.get_id())); } void add_heuristic_options_to_feature( diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 7f8088e406..d8ee05927f 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -479,17 +479,6 @@ class OperatorProxy { int get_id() const { return index; } - - /* - Eventually, this method should perhaps not be part of OperatorProxy but - live in a class that handles the task transformation and known about both - the original and the transformed task. - */ - OperatorID get_ancestor_operator_id( - const AbstractTask *ancestor_task) const { - assert(!is_an_axiom); - return OperatorID(task->convert_operator_index(index, ancestor_task)); - } }; class OperatorsProxy { From 47479bd5bd02b95e980c9702fb977d94615a4073 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 12:56:12 +0200 Subject: [PATCH 20/29] change concept of global root task to local explicit task read from file --- src/search/heuristic.cc | 11 +--- src/search/landmarks/landmark_heuristic.cc | 2 - src/search/planner.cc | 8 +-- src/search/task_proxy.h | 43 ++++++--------- src/search/tasks/root_task.cc | 64 +++++++++++----------- src/search/tasks/root_task.h | 3 +- 6 files changed, 53 insertions(+), 78 deletions(-) diff --git a/src/search/heuristic.cc b/src/search/heuristic.cc index c722e8f51b..8fc4d9097e 100644 --- a/src/search/heuristic.cc +++ b/src/search/heuristic.cc @@ -5,12 +5,9 @@ #include "plugins/plugin.h" #include "task_utils/task_properties.h" -#include "tasks/cost_adapted_task.h" -#include "tasks/root_task.h" #include #include -#include using namespace std; Heuristic::Heuristic( @@ -46,7 +43,7 @@ EvaluationResult Heuristic::compute_result(EvaluationContext &eval_context) { assert(preferred_operators.empty()); const State &state = eval_context.get_state(); - assert(state.get_task() == TaskProxy(*task)); + assert(state.get_task() == task_proxy); bool calculate_preferred = eval_context.get_calculate_preferred(); int heuristic = NO_VALUE; @@ -78,12 +75,10 @@ EvaluationResult Heuristic::compute_result(EvaluationContext &eval_context) { } #ifndef NDEBUG - TaskProxy global_task_proxy = state.get_task(); - OperatorsProxy global_operators = global_task_proxy.get_operators(); + OperatorsProxy operators = task_proxy.get_operators(); if (heuristic != EvaluationResult::INFTY) { for (OperatorID op_id : preferred_operators) - assert( - task_properties::is_applicable(global_operators[op_id], state)); + assert(task_properties::is_applicable(operators[op_id], state)); } #endif diff --git a/src/search/landmarks/landmark_heuristic.cc b/src/search/landmarks/landmark_heuristic.cc index 03892e6937..2c47c816ab 100644 --- a/src/search/landmarks/landmark_heuristic.cc +++ b/src/search/landmarks/landmark_heuristic.cc @@ -6,8 +6,6 @@ #include "../plugins/plugin.h" #include "../task_utils/successor_generator.h" -#include "../tasks/cost_adapted_task.h" -#include "../tasks/root_task.h" #include "../utils/markup.h" using namespace std; diff --git a/src/search/planner.cc b/src/search/planner.cc index 7aee206d4b..7248e84779 100644 --- a/src/search/planner.cc +++ b/src/search/planner.cc @@ -35,13 +35,7 @@ int main(int argc, const char **argv) { if (static_cast(argv[1]) != "--help") { utils::g_log << get_revision_info() << endl; utils::g_log << "reading input..." << endl; - tasks::read_root_task(cin); - task = tasks::g_root_task; - /* - TODO once we get rid of g_root_task, the two lines above should - be replaced by something like - task = tasks::read_task(cin) - */ + task = tasks::read_task(cin); utils::g_log << "done reading input!" << endl; TaskProxy task_proxy(*task); unit_cost = task_properties::is_unit_cost(task_proxy); diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index d8ee05927f..1d78aea395 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -47,13 +47,12 @@ using PackedStateBin = int_packer::IntPacker::Bin; for accessing task information (TaskProxy, OperatorProxy, etc.) and task implementations (subclasses of AbstractTask). Each proxy class knows which AbstractTask it belongs to and uses its methods to retrieve - information about the task. RootTask is the AbstractTask that - encapsulates the unmodified original task that the planner received - as input. + information about the task. - Example code for creating a new task object and accessing its operators: + Example code for parsing a task from cin and accessing its operators: - TaskProxy task_proxy(*g_root_task()); + shared_ptr task = tasks::read_task(cin); + TaskProxy task_proxy(*task); for (OperatorProxy op : task->get_operators()) utils::g_log << op.get_name() << endl; @@ -67,30 +66,22 @@ using PackedStateBin = int_packer::IntPacker::Bin; expensive to create. If performance is absolutely critical, the values of a state can be unpacked and accessed as a vector. - For now, heuristics work with a TaskProxy that can represent a transformed - view of the original task. The search algorithms work on the unmodified root - task. We therefore need to do two conversions between the search and the - heuristics: converting states of the root task to states of the task used in - the heuristic computation and converting operators of the task used by the - heuristic to operators of the task used by the search for reporting preferred - operators. - These conversions are done by the Heuristic base class with - Heuristic::convert_ancestor_state() and Heuristic::set_preferred(). - - int FantasyHeuristic::compute_heuristic(const State &ancestor_state) { - State state = convert_ancestor_state(ancestor_state); - set_preferred(task->get_operators()[42]); - int sum = 0; - for (FactProxy fact : state) - sum += fact.get_value(); - return sum; - } + Some evaluators internally perform task transformations and evaluate a nested + evaluator on the transformed task. They are reponsible for two kinds of + transformations: + + 1) Incoming states (e.g., in compute_heuristic, notify_initial_state, etc.) + have to be transformed to states of the transformed task. Note that even + if the transformed task uses the same set of states as the original task + this requires a modification as states know which tasks they belong to. + 2) Operators marked as preferred by the nested evaluator are referring to the + transformed task and their IDs may have to be translated back. + + See evaluators/modify_costs_evaluator.* and + evaluators/state_forwarding_evaluator.* for examples. For helper functions that work on task related objects, please see the task_properties.h module. - - TODO(issue1208): update this after we get rid of convert_ancestor_state and - get_ancestor_operator_id. */ template diff --git a/src/search/tasks/root_task.cc b/src/search/tasks/root_task.cc index 2dcf7cfa38..e8a5a1502a 100644 --- a/src/search/tasks/root_task.cc +++ b/src/search/tasks/root_task.cc @@ -16,7 +16,6 @@ using utils::ExitCode; namespace tasks { static const int PRE_FILE_VERSION = 3; -shared_ptr g_root_task = nullptr; struct ExplicitVariable { int domain_size; @@ -39,7 +38,7 @@ struct ExplicitOperator { bool is_an_axiom; }; -class RootTask : public AbstractTask { +class ExplicitTask : public AbstractTask { vector variables; // TODO: think about using hash sets here. vector>> mutexes; @@ -55,7 +54,7 @@ class RootTask : public AbstractTask { int index, bool is_axiom) const; public: - RootTask( + ExplicitTask( vector &&variables, vector>> &&mutexes, vector &&operators, vector &&axioms, @@ -711,7 +710,7 @@ shared_ptr TaskParser::read_task() { then evaluate the axioms on that state after construction. This requires mutable access to the values, though. */ - shared_ptr task = make_shared( + shared_ptr task = make_shared( move(variables), move(mutexes), move(operators), move(axioms), move(initial_state_values), move(goals)); AxiomEvaluator &axiom_evaluator = g_axiom_evaluators[TaskProxy(*task)]; @@ -729,7 +728,7 @@ shared_ptr TaskParser::parse() { } } -RootTask::RootTask( +ExplicitTask::ExplicitTask( vector &&variables, vector>> &&mutexes, vector &&operators, vector &&axioms, @@ -742,19 +741,19 @@ RootTask::RootTask( goals(move(goals)) { } -const ExplicitVariable &RootTask::get_variable(int var) const { +const ExplicitVariable &ExplicitTask::get_variable(int var) const { assert(utils::in_bounds(var, variables)); return variables[var]; } -const ExplicitEffect &RootTask::get_effect( +const ExplicitEffect &ExplicitTask::get_effect( int op_id, int effect_id, bool is_axiom) const { const ExplicitOperator &op = get_operator_or_axiom(op_id, is_axiom); assert(utils::in_bounds(effect_id, op.effects)); return op.effects[effect_id]; } -const ExplicitOperator &RootTask::get_operator_or_axiom( +const ExplicitOperator &ExplicitTask::get_operator_or_axiom( int index, bool is_axiom) const { if (is_axiom) { assert(utils::in_bounds(index, axioms)); @@ -765,32 +764,32 @@ const ExplicitOperator &RootTask::get_operator_or_axiom( } } -int RootTask::get_num_variables() const { +int ExplicitTask::get_num_variables() const { return variables.size(); } -string RootTask::get_variable_name(int var) const { +string ExplicitTask::get_variable_name(int var) const { return get_variable(var).name; } -int RootTask::get_variable_domain_size(int var) const { +int ExplicitTask::get_variable_domain_size(int var) const { return get_variable(var).domain_size; } -int RootTask::get_variable_axiom_layer(int var) const { +int ExplicitTask::get_variable_axiom_layer(int var) const { return get_variable(var).axiom_layer; } -int RootTask::get_variable_default_axiom_value(int var) const { +int ExplicitTask::get_variable_default_axiom_value(int var) const { return get_variable(var).axiom_default_value; } -string RootTask::get_fact_name(const FactPair &fact) const { +string ExplicitTask::get_fact_name(const FactPair &fact) const { assert(utils::in_bounds(fact.value, get_variable(fact.var).fact_names)); return get_variable(fact.var).fact_names[fact.value]; } -bool RootTask::are_facts_mutex( +bool ExplicitTask::are_facts_mutex( const FactPair &fact1, const FactPair &fact2) const { if (fact1.var == fact2.var) { // Same variable: mutex iff different value. @@ -801,51 +800,51 @@ bool RootTask::are_facts_mutex( return bool(mutexes[fact1.var][fact1.value].count(fact2)); } -int RootTask::get_operator_cost(int index, bool is_axiom) const { +int ExplicitTask::get_operator_cost(int index, bool is_axiom) const { return get_operator_or_axiom(index, is_axiom).cost; } -string RootTask::get_operator_name(int index, bool is_axiom) const { +string ExplicitTask::get_operator_name(int index, bool is_axiom) const { return get_operator_or_axiom(index, is_axiom).name; } -int RootTask::get_num_operators() const { +int ExplicitTask::get_num_operators() const { return operators.size(); } -int RootTask::get_num_operator_preconditions(int index, bool is_axiom) const { +int ExplicitTask::get_num_operator_preconditions(int index, bool is_axiom) const { return get_operator_or_axiom(index, is_axiom).preconditions.size(); } -FactPair RootTask::get_operator_precondition( +FactPair ExplicitTask::get_operator_precondition( int op_index, int fact_index, bool is_axiom) const { const ExplicitOperator &op = get_operator_or_axiom(op_index, is_axiom); assert(utils::in_bounds(fact_index, op.preconditions)); return op.preconditions[fact_index]; } -int RootTask::get_num_operator_effects(int op_index, bool is_axiom) const { +int ExplicitTask::get_num_operator_effects(int op_index, bool is_axiom) const { return get_operator_or_axiom(op_index, is_axiom).effects.size(); } -int RootTask::get_num_operator_effect_conditions( +int ExplicitTask::get_num_operator_effect_conditions( int op_index, int eff_index, bool is_axiom) const { return get_effect(op_index, eff_index, is_axiom).conditions.size(); } -FactPair RootTask::get_operator_effect_condition( +FactPair ExplicitTask::get_operator_effect_condition( int op_index, int eff_index, int cond_index, bool is_axiom) const { const ExplicitEffect &effect = get_effect(op_index, eff_index, is_axiom); assert(utils::in_bounds(cond_index, effect.conditions)); return effect.conditions[cond_index]; } -FactPair RootTask::get_operator_effect( +FactPair ExplicitTask::get_operator_effect( int op_index, int eff_index, bool is_axiom) const { return get_effect(op_index, eff_index, is_axiom).fact; } -int RootTask::convert_operator_index( +int ExplicitTask::convert_operator_index( int index, const AbstractTask *ancestor_task) const { if (this != ancestor_task) { ABORT("Invalid operator ID conversion"); @@ -853,35 +852,34 @@ int RootTask::convert_operator_index( return index; } -int RootTask::get_num_axioms() const { +int ExplicitTask::get_num_axioms() const { return axioms.size(); } -int RootTask::get_num_goals() const { +int ExplicitTask::get_num_goals() const { return goals.size(); } -FactPair RootTask::get_goal_fact(int index) const { +FactPair ExplicitTask::get_goal_fact(int index) const { assert(utils::in_bounds(index, goals)); return goals[index]; } -vector RootTask::get_initial_state_values() const { +vector ExplicitTask::get_initial_state_values() const { return initial_state_values; } -void RootTask::convert_ancestor_state_values( +void ExplicitTask::convert_ancestor_state_values( vector &, const AbstractTask *ancestor_task) const { if (this != ancestor_task) { ABORT("Invalid state conversion"); } } -void read_root_task(istream &in) { - assert(!g_root_task); +std::shared_ptr read_task(istream &in) { utils::TaskLexer lexer(in); // TODO: construct lexer in TaskParser TaskParser parser(move(lexer)); - g_root_task = parser.parse(); + return parser.parse(); } } diff --git a/src/search/tasks/root_task.h b/src/search/tasks/root_task.h index 15961d1e87..2cbc5914c2 100644 --- a/src/search/tasks/root_task.h +++ b/src/search/tasks/root_task.h @@ -4,7 +4,6 @@ #include "../abstract_task.h" namespace tasks { -extern std::shared_ptr g_root_task; -extern void read_root_task(std::istream &in); +extern std::shared_ptr read_task(std::istream &in); } #endif From 7053aa894b352926f5931e81cac7eeba0399d937 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 13:17:25 +0200 Subject: [PATCH 21/29] rename root_task to explicit_task --- src/search/CMakeLists.txt | 2 +- src/search/planner.cc | 2 +- src/search/tasks/{root_task.cc => explicit_task.cc} | 2 +- src/search/tasks/{root_task.h => explicit_task.h} | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename src/search/tasks/{root_task.cc => explicit_task.cc} (99%) rename src/search/tasks/{root_task.h => explicit_task.h} (100%) diff --git a/src/search/CMakeLists.txt b/src/search/CMakeLists.txt index eb103944a2..75e5040c73 100644 --- a/src/search/CMakeLists.txt +++ b/src/search/CMakeLists.txt @@ -750,7 +750,7 @@ create_fast_downward_library( SOURCES tasks/cost_adapted_task tasks/delegating_task - tasks/root_task + tasks/explicit_task CORE_LIBRARY ) diff --git a/src/search/planner.cc b/src/search/planner.cc index 7248e84779..1f2b9be8c4 100644 --- a/src/search/planner.cc +++ b/src/search/planner.cc @@ -3,7 +3,7 @@ #include "search_algorithm.h" #include "task_utils/task_properties.h" -#include "tasks/root_task.h" +#include "tasks/explicit_task.h" #include "utils/logging.h" #include "utils/system.h" #include "utils/timer.h" diff --git a/src/search/tasks/root_task.cc b/src/search/tasks/explicit_task.cc similarity index 99% rename from src/search/tasks/root_task.cc rename to src/search/tasks/explicit_task.cc index e8a5a1502a..b83ab0ae78 100644 --- a/src/search/tasks/root_task.cc +++ b/src/search/tasks/explicit_task.cc @@ -1,4 +1,4 @@ -#include "root_task.h" +#include "explicit_task.h" #include "../axioms.h" diff --git a/src/search/tasks/root_task.h b/src/search/tasks/explicit_task.h similarity index 100% rename from src/search/tasks/root_task.h rename to src/search/tasks/explicit_task.h From 9ae7b85647b27973d31d8b90d0be8c62d1c888f4 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 13:42:33 +0200 Subject: [PATCH 22/29] remove unused includes --- src/search/evaluation_context.h | 2 -- src/search/heuristics/additive_heuristic.h | 1 - src/search/heuristics/cea_heuristic.h | 1 - src/search/heuristics/cg_heuristic.cc | 1 - src/search/landmarks/landmark_factory.cc | 2 -- src/search/search_algorithm.h | 2 -- src/search/state_registry.cc | 1 - src/search/task_id.h | 1 - src/search/task_proxy.cc | 2 -- src/search/tasks/default_value_axioms_task.cc | 2 -- 10 files changed, 15 deletions(-) diff --git a/src/search/evaluation_context.h b/src/search/evaluation_context.h index a2d9b0649e..6745bb44c4 100644 --- a/src/search/evaluation_context.h +++ b/src/search/evaluation_context.h @@ -6,8 +6,6 @@ #include "operator_id.h" #include "task_proxy.h" -#include - class Evaluator; class SearchStatistics; diff --git a/src/search/heuristics/additive_heuristic.h b/src/search/heuristics/additive_heuristic.h index 8201e1714b..dc581b711f 100644 --- a/src/search/heuristics/additive_heuristic.h +++ b/src/search/heuristics/additive_heuristic.h @@ -4,7 +4,6 @@ #include "relaxation_heuristic.h" #include "../algorithms/priority_queues.h" -#include "../utils/collections.h" #include diff --git a/src/search/heuristics/cea_heuristic.h b/src/search/heuristics/cea_heuristic.h index 00bd6fbfed..bb8205e780 100644 --- a/src/search/heuristics/cea_heuristic.h +++ b/src/search/heuristics/cea_heuristic.h @@ -6,7 +6,6 @@ #include "../heuristic.h" #include "../algorithms/priority_queues.h" -#include "../tasks/default_value_axioms_task.h" #include diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index 573b7ffd4a..029b73371d 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -8,7 +8,6 @@ #include "../task_utils/task_properties.h" #include "../utils/logging.h" -#include #include #include #include diff --git a/src/search/landmarks/landmark_factory.cc b/src/search/landmarks/landmark_factory.cc index 050373086b..fce4098d60 100644 --- a/src/search/landmarks/landmark_factory.cc +++ b/src/search/landmarks/landmark_factory.cc @@ -1,6 +1,5 @@ #include "landmark_factory.h" -#include "landmark.h" #include "landmark_graph.h" #include "util.h" @@ -10,7 +9,6 @@ #include "../utils/logging.h" #include "../utils/timer.h" -#include #include using namespace std; diff --git a/src/search/search_algorithm.h b/src/search/search_algorithm.h index e5b5683411..f9559821f2 100644 --- a/src/search/search_algorithm.h +++ b/src/search/search_algorithm.h @@ -14,8 +14,6 @@ #include "utils/logging.h" -#include - namespace plugins { class Options; class Feature; diff --git a/src/search/state_registry.cc b/src/search/state_registry.cc index bdadc17572..21916b2b0c 100644 --- a/src/search/state_registry.cc +++ b/src/search/state_registry.cc @@ -1,6 +1,5 @@ #include "state_registry.h" -#include "per_state_information.h" #include "task_proxy.h" #include "task_utils/task_properties.h" diff --git a/src/search/task_id.h b/src/search/task_id.h index 45d73709dd..341344ce56 100644 --- a/src/search/task_id.h +++ b/src/search/task_id.h @@ -4,7 +4,6 @@ #include "utils/hash.h" #include -#include class AbstractTask; diff --git a/src/search/task_proxy.cc b/src/search/task_proxy.cc index 41590e8eaa..b6f89246ca 100644 --- a/src/search/task_proxy.cc +++ b/src/search/task_proxy.cc @@ -6,8 +6,6 @@ #include "task_utils/causal_graph.h" #include "task_utils/task_properties.h" -#include - using namespace std; State::State( diff --git a/src/search/tasks/default_value_axioms_task.cc b/src/search/tasks/default_value_axioms_task.cc index e71e301c0c..9f8fb91798 100644 --- a/src/search/tasks/default_value_axioms_task.cc +++ b/src/search/tasks/default_value_axioms_task.cc @@ -3,10 +3,8 @@ #include "../task_proxy.h" #include "../algorithms/sccs.h" -#include "../task_utils/task_properties.h" #include -#include #include #include From 0357ed4f08358c21393a3e985f57940e5a62bb6a Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 13:56:20 +0200 Subject: [PATCH 23/29] remove unused includes --- src/search/heuristics/blind_search_heuristic.cc | 4 ---- src/search/heuristics/lm_cut_heuristic.cc | 1 - src/search/landmarks/landmark_cost_partitioning_algorithms.h | 1 - src/search/landmarks/landmark_heuristic.h | 1 - src/search/merge_and_shrink/merge_and_shrink_heuristic.cc | 1 - src/search/operator_counting/operator_counting_heuristic.cc | 1 - src/search/potentials/diverse_potential_heuristics.cc | 3 --- src/search/potentials/potential_heuristic.cc | 2 -- src/search/potentials/potential_max_heuristic.cc | 2 -- src/search/potentials/potential_optimizer.cc | 1 - 10 files changed, 17 deletions(-) diff --git a/src/search/heuristics/blind_search_heuristic.cc b/src/search/heuristics/blind_search_heuristic.cc index 419a8fe4d0..a5064f2591 100644 --- a/src/search/heuristics/blind_search_heuristic.cc +++ b/src/search/heuristics/blind_search_heuristic.cc @@ -4,10 +4,6 @@ #include "../task_utils/task_properties.h" #include "../utils/logging.h" -#include -#include -#include - using namespace std; namespace blind_search_heuristic { diff --git a/src/search/heuristics/lm_cut_heuristic.cc b/src/search/heuristics/lm_cut_heuristic.cc index 898d35cf5d..0b081194b2 100644 --- a/src/search/heuristics/lm_cut_heuristic.cc +++ b/src/search/heuristics/lm_cut_heuristic.cc @@ -5,7 +5,6 @@ #include "../task_proxy.h" #include "../plugins/plugin.h" -#include "../task_utils/task_properties.h" #include "../utils/logging.h" #include diff --git a/src/search/landmarks/landmark_cost_partitioning_algorithms.h b/src/search/landmarks/landmark_cost_partitioning_algorithms.h index 5255fcfa94..4959e6e494 100644 --- a/src/search/landmarks/landmark_cost_partitioning_algorithms.h +++ b/src/search/landmarks/landmark_cost_partitioning_algorithms.h @@ -6,7 +6,6 @@ #include "../lp/lp_solver.h" -#include #include class ConstBitsetView; diff --git a/src/search/landmarks/landmark_heuristic.h b/src/search/landmarks/landmark_heuristic.h index ce9f963c89..a1575bcbbf 100644 --- a/src/search/landmarks/landmark_heuristic.h +++ b/src/search/landmarks/landmark_heuristic.h @@ -5,7 +5,6 @@ #include "../heuristic.h" -#include "../tasks/default_value_axioms_task.h" #include "../utils/hash.h" class ConstBitsetView; diff --git a/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc b/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc index 1e320cf46a..423aac9e42 100644 --- a/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc +++ b/src/search/merge_and_shrink/merge_and_shrink_heuristic.cc @@ -8,7 +8,6 @@ #include "types.h" #include "../plugins/plugin.h" -#include "../task_utils/task_properties.h" #include "../utils/markup.h" #include "../utils/system.h" diff --git a/src/search/operator_counting/operator_counting_heuristic.cc b/src/search/operator_counting/operator_counting_heuristic.cc index 5bf5df23ab..db979b12fc 100644 --- a/src/search/operator_counting/operator_counting_heuristic.cc +++ b/src/search/operator_counting/operator_counting_heuristic.cc @@ -5,7 +5,6 @@ #include "../plugins/plugin.h" #include "../utils/component_errors.h" #include "../utils/markup.h" -#include "../utils/strings.h" #include diff --git a/src/search/potentials/diverse_potential_heuristics.cc b/src/search/potentials/diverse_potential_heuristics.cc index cdd46afc13..b5d61a5ac6 100644 --- a/src/search/potentials/diverse_potential_heuristics.cc +++ b/src/search/potentials/diverse_potential_heuristics.cc @@ -6,12 +6,9 @@ #include "../plugins/plugin.h" #include "../utils/logging.h" -#include "../utils/rng.h" #include "../utils/rng_options.h" #include "../utils/timer.h" -#include - using namespace std; namespace potentials { diff --git a/src/search/potentials/potential_heuristic.cc b/src/search/potentials/potential_heuristic.cc index f49f891ef4..f977854f87 100644 --- a/src/search/potentials/potential_heuristic.cc +++ b/src/search/potentials/potential_heuristic.cc @@ -2,8 +2,6 @@ #include "potential_function.h" -#include "../plugins/plugin.h" - using namespace std; namespace potentials { diff --git a/src/search/potentials/potential_max_heuristic.cc b/src/search/potentials/potential_max_heuristic.cc index e56c474d7f..2c11e261f1 100644 --- a/src/search/potentials/potential_max_heuristic.cc +++ b/src/search/potentials/potential_max_heuristic.cc @@ -2,8 +2,6 @@ #include "potential_function.h" -#include "../plugins/plugin.h" - using namespace std; namespace potentials { diff --git a/src/search/potentials/potential_optimizer.cc b/src/search/potentials/potential_optimizer.cc index 8dda7f43fa..18df3d84b8 100644 --- a/src/search/potentials/potential_optimizer.cc +++ b/src/search/potentials/potential_optimizer.cc @@ -2,7 +2,6 @@ #include "potential_function.h" -#include "../plugins/plugin.h" #include "../task_utils/task_properties.h" #include "../utils/collections.h" #include "../utils/system.h" From f53354ec1025b4f5d3c48b1a08df2a2f09cd91a4 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 14:20:16 +0200 Subject: [PATCH 24/29] unpack state before accessing unpacked data --- src/search/pdbs/pattern_collection_generator_hillclimbing.cc | 1 + src/search/pdbs/pdb_heuristic.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/search/pdbs/pattern_collection_generator_hillclimbing.cc b/src/search/pdbs/pattern_collection_generator_hillclimbing.cc index eed9d8f8cb..f3620e0b68 100644 --- a/src/search/pdbs/pattern_collection_generator_hillclimbing.cc +++ b/src/search/pdbs/pattern_collection_generator_hillclimbing.cc @@ -368,6 +368,7 @@ void PatternCollectionGeneratorHillclimbing::hill_climbing( samples_h_values.clear(); sample_states(sampler, init_h, samples); for (const State &sample : samples) { + sample.unpack(); samples_h_values.push_back(current_pdbs->get_value(sample)); } diff --git a/src/search/pdbs/pdb_heuristic.cc b/src/search/pdbs/pdb_heuristic.cc index cf5d26bcb3..e10ea38555 100644 --- a/src/search/pdbs/pdb_heuristic.cc +++ b/src/search/pdbs/pdb_heuristic.cc @@ -27,6 +27,7 @@ PDBHeuristic::PDBHeuristic( } int PDBHeuristic::compute_heuristic(const State &state) { + state.unpack(); int h = pdb->get_value(state.get_unpacked_values()); if (h == numeric_limits::max()) return DEAD_END; From 3e90f7daca5498047d66126aa24dbfbcb32868e5 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 14:38:53 +0200 Subject: [PATCH 25/29] style --- src/search/landmarks/landmark_sum_heuristic.cc | 3 +-- src/search/tasks/explicit_task.cc | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/search/landmarks/landmark_sum_heuristic.cc b/src/search/landmarks/landmark_sum_heuristic.cc index 5dfc2c41b5..86ee5104cc 100644 --- a/src/search/landmarks/landmark_sum_heuristic.cc +++ b/src/search/landmarks/landmark_sum_heuristic.cc @@ -88,8 +88,7 @@ void LandmarkSumHeuristic::compute_landmark_costs() { int LandmarkSumHeuristic::get_heuristic_value(const State &state) { int h = 0; - ConstBitsetView past = - landmark_status_manager->get_past_landmarks(state); + ConstBitsetView past = landmark_status_manager->get_past_landmarks(state); ConstBitsetView future = landmark_status_manager->get_future_landmarks(state); for (int id = 0; id < landmark_graph->get_num_landmarks(); ++id) { diff --git a/src/search/tasks/explicit_task.cc b/src/search/tasks/explicit_task.cc index b83ab0ae78..37484a271e 100644 --- a/src/search/tasks/explicit_task.cc +++ b/src/search/tasks/explicit_task.cc @@ -812,7 +812,8 @@ int ExplicitTask::get_num_operators() const { return operators.size(); } -int ExplicitTask::get_num_operator_preconditions(int index, bool is_axiom) const { +int ExplicitTask::get_num_operator_preconditions( + int index, bool is_axiom) const { return get_operator_or_axiom(index, is_axiom).preconditions.size(); } From afa0ca4617190e8c73d134e10a2394d371618a3d Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 25 Jul 2026 14:50:44 +0200 Subject: [PATCH 26/29] style --- src/search/tasks/explicit_task.cc | 2 +- src/search/tasks/explicit_task.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/search/tasks/explicit_task.cc b/src/search/tasks/explicit_task.cc index 37484a271e..03c06801e1 100644 --- a/src/search/tasks/explicit_task.cc +++ b/src/search/tasks/explicit_task.cc @@ -877,7 +877,7 @@ void ExplicitTask::convert_ancestor_state_values( } } -std::shared_ptr read_task(istream &in) { +shared_ptr read_task(istream &in) { utils::TaskLexer lexer(in); // TODO: construct lexer in TaskParser TaskParser parser(move(lexer)); diff --git a/src/search/tasks/explicit_task.h b/src/search/tasks/explicit_task.h index 2cbc5914c2..ca42299c73 100644 --- a/src/search/tasks/explicit_task.h +++ b/src/search/tasks/explicit_task.h @@ -1,5 +1,5 @@ -#ifndef TASKS_ROOT_TASK_H -#define TASKS_ROOT_TASK_H +#ifndef TASKS_EXPLICIT_TASK_H +#define TASKS_EXPLICIT_TASK_H #include "../abstract_task.h" From 5e6e6281a9f407815bc0699fc364ee03c8227ff8 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 29 Jul 2026 11:39:11 +0200 Subject: [PATCH 27/29] Introduce EvaluatorCall class to represent the part of the evaluation that is limited to one (transformed) state --- src/search/CMakeLists.txt | 1 + src/search/evaluation_context.cc | 20 +++++---- src/search/evaluation_context.h | 2 + src/search/evaluator.h | 5 +-- src/search/evaluator_call.cc | 45 +++++++++++++++++++ src/search/evaluator_call.h | 23 ++++++++++ src/search/evaluators/combining_evaluator.cc | 8 ++-- src/search/evaluators/combining_evaluator.h | 3 +- src/search/evaluators/const_evaluator.cc | 2 +- src/search/evaluators/const_evaluator.h | 3 +- src/search/evaluators/g_evaluator.cc | 6 +-- src/search/evaluators/g_evaluator.h | 3 +- src/search/evaluators/pref_evaluator.cc | 7 ++- src/search/evaluators/pref_evaluator.h | 3 +- .../evaluators/state_forwarding_evaluator.cc | 14 +++--- .../evaluators/state_forwarding_evaluator.h | 3 +- src/search/evaluators/weighted_evaluator.cc | 7 ++- src/search/evaluators/weighted_evaluator.h | 3 +- src/search/heuristic.cc | 8 ++-- src/search/heuristic.h | 3 +- 20 files changed, 114 insertions(+), 55 deletions(-) create mode 100644 src/search/evaluator_call.cc create mode 100644 src/search/evaluator_call.h diff --git a/src/search/CMakeLists.txt b/src/search/CMakeLists.txt index 75e5040c73..0442a8db2e 100644 --- a/src/search/CMakeLists.txt +++ b/src/search/CMakeLists.txt @@ -97,6 +97,7 @@ create_fast_downward_library( evaluation_context evaluation_result evaluator + evaluator_call evaluator_cache heuristic open_list diff --git a/src/search/evaluation_context.cc b/src/search/evaluation_context.cc index e0b5065683..5b6f732419 100644 --- a/src/search/evaluation_context.cc +++ b/src/search/evaluation_context.cc @@ -2,6 +2,7 @@ #include "evaluation_result.h" #include "evaluator.h" +#include "evaluator_call.h" #include "search_statistics.h" #include @@ -50,21 +51,22 @@ EvaluationContext::EvaluationContext( } const EvaluationResult &EvaluationContext::get_result(Evaluator *evaluator) { - EvaluationResult &result = cache[evaluator]; - if (result.is_uninitialized()) { - result = evaluator->compute_result(*this); - if (statistics && evaluator->is_used_for_counting_evaluations() && - result.get_count_evaluation()) { - statistics->inc_evaluations(); - } - } - return result; + EvaluatorCall call(*this, state); + return call[evaluator]; } const EvaluatorCache &EvaluationContext::get_cache() const { return cache; } +EvaluatorCache &EvaluationContext::get_cache() { + return cache; +} + +SearchStatistics *EvaluationContext::get_statistics() { + return statistics; +} + const State &EvaluationContext::get_state() const { return state; } diff --git a/src/search/evaluation_context.h b/src/search/evaluation_context.h index 6745bb44c4..74ef1e1dcc 100644 --- a/src/search/evaluation_context.h +++ b/src/search/evaluation_context.h @@ -100,6 +100,8 @@ class EvaluationContext { const EvaluationResult &get_result(Evaluator *eval); const EvaluatorCache &get_cache() const; + EvaluatorCache &get_cache(); + SearchStatistics *get_statistics(); const State &get_state() const; int get_g_value() const; bool is_preferred() const; diff --git a/src/search/evaluator.h b/src/search/evaluator.h index b5f423919c..ea0414210e 100644 --- a/src/search/evaluator.h +++ b/src/search/evaluator.h @@ -8,7 +8,7 @@ #include -class EvaluationContext; +class EvaluatorCall; class State; namespace plugins { @@ -83,8 +83,7 @@ class Evaluator : public components::TaskSpecificComponent { EvaluationContext. We need to think of a clean way to achieve this. */ - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) = 0; + virtual EvaluationResult compute_result(EvaluatorCall &call) = 0; void report_value_for_initial_state(const EvaluationResult &result) const; void report_new_minimum_value(const EvaluationResult &result) const; diff --git a/src/search/evaluator_call.cc b/src/search/evaluator_call.cc new file mode 100644 index 0000000000..b6026f4458 --- /dev/null +++ b/src/search/evaluator_call.cc @@ -0,0 +1,45 @@ +#include "evaluator_call.h" + +#include "evaluation_context.h" +#include "evaluation_result.h" +#include "evaluator.h" +#include "evaluator_cache.h" +#include "search_statistics.h" +#include "task_proxy.h" + +EvaluatorCall::EvaluatorCall(EvaluationContext &context, const State &state) + : context(context), state(state) { +} + +const EvaluationResult &EvaluatorCall::operator[](Evaluator *evaluator) { + EvaluationResult &result = context.get_cache()[evaluator]; + if (result.is_uninitialized()) { + SearchStatistics *statistics = context.get_statistics(); + result = evaluator->compute_result(*this); + if (statistics && evaluator->is_used_for_counting_evaluations() && + result.get_count_evaluation()) { + statistics->inc_evaluations(); + } + } + return result; +} + +EvaluatorCall EvaluatorCall::get_subcall(const State &state) const { + return EvaluatorCall(context, state); +} + +const State &EvaluatorCall::get_state() const { + return state; +} + +int EvaluatorCall::get_g_value() const { + return context.get_g_value(); +} + +bool EvaluatorCall::get_calculate_preferred() const { + return context.get_calculate_preferred(); +} + +bool EvaluatorCall::is_preferred() const { + return context.is_preferred(); +} diff --git a/src/search/evaluator_call.h b/src/search/evaluator_call.h new file mode 100644 index 0000000000..461d67ad48 --- /dev/null +++ b/src/search/evaluator_call.h @@ -0,0 +1,23 @@ +#ifndef EVALUATOR_CALL_H +#define EVALUATOR_CALL_H + +class EvaluationContext; +class EvaluationResult; +class Evaluator; +class State; + +class EvaluatorCall { + EvaluationContext &context; + const State &state; +public: + EvaluatorCall(EvaluationContext &context, const State &state); + const EvaluationResult &operator[](Evaluator *evaluator); + + EvaluatorCall get_subcall(const State &state) const; + const State &get_state() const; + int get_g_value() const; + bool get_calculate_preferred() const; + bool is_preferred() const; +}; + +#endif diff --git a/src/search/evaluators/combining_evaluator.cc b/src/search/evaluators/combining_evaluator.cc index d5408560a4..90d72f2438 100644 --- a/src/search/evaluators/combining_evaluator.cc +++ b/src/search/evaluators/combining_evaluator.cc @@ -1,7 +1,7 @@ #include "combining_evaluator.h" -#include "../evaluation_context.h" #include "../evaluation_result.h" +#include "../evaluator_call.h" #include "../plugins/plugin.h" #include "../utils/component_errors.h" @@ -26,8 +26,7 @@ bool CombiningEvaluator::dead_ends_are_reliable() const { return all_dead_ends_are_reliable; } -EvaluationResult CombiningEvaluator::compute_result( - EvaluationContext &eval_context) { +EvaluationResult CombiningEvaluator::compute_result(EvaluatorCall &call) { // This marks no preferred operators. EvaluationResult result; vector values; @@ -35,8 +34,7 @@ EvaluationResult CombiningEvaluator::compute_result( // Collect component values. Return infinity if any is infinite. for (const shared_ptr &subevaluator : subevaluators) { - int value = - eval_context.get_evaluator_value_or_infinity(subevaluator.get()); + int value = call[subevaluator.get()].get_evaluator_value(); if (value == EvaluationResult::INFTY) { result.set_evaluator_value(value); return result; diff --git a/src/search/evaluators/combining_evaluator.h b/src/search/evaluators/combining_evaluator.h index 46b5dbf91a..0661ccc0f9 100644 --- a/src/search/evaluators/combining_evaluator.h +++ b/src/search/evaluators/combining_evaluator.h @@ -38,8 +38,7 @@ class CombiningEvaluator : public Evaluator { */ virtual bool dead_ends_are_reliable() const override; - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; virtual void get_path_dependent_evaluators( std::set &evals) override; diff --git a/src/search/evaluators/const_evaluator.cc b/src/search/evaluators/const_evaluator.cc index a848330767..725a717b0d 100644 --- a/src/search/evaluators/const_evaluator.cc +++ b/src/search/evaluators/const_evaluator.cc @@ -12,7 +12,7 @@ ConstEvaluator::ConstEvaluator( value(value) { } -EvaluationResult ConstEvaluator::compute_result(EvaluationContext &) { +EvaluationResult ConstEvaluator::compute_result(EvaluatorCall &) { EvaluationResult result; result.set_evaluator_value(value); return result; diff --git a/src/search/evaluators/const_evaluator.h b/src/search/evaluators/const_evaluator.h index 458aeea914..d7efc4ef75 100644 --- a/src/search/evaluators/const_evaluator.h +++ b/src/search/evaluators/const_evaluator.h @@ -8,8 +8,7 @@ class ConstEvaluator : public Evaluator { int value; protected: - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; public: ConstEvaluator( diff --git a/src/search/evaluators/g_evaluator.cc b/src/search/evaluators/g_evaluator.cc index 0b8e012c59..7be5c4987e 100644 --- a/src/search/evaluators/g_evaluator.cc +++ b/src/search/evaluators/g_evaluator.cc @@ -1,6 +1,6 @@ #include "g_evaluator.h" -#include "../evaluation_context.h" +#include "../evaluator_call.h" #include "../evaluation_result.h" #include "../plugins/plugin.h" @@ -14,9 +14,9 @@ GEvaluator::GEvaluator( : Evaluator(task, false, false, false, description, verbosity) { } -EvaluationResult GEvaluator::compute_result(EvaluationContext &eval_context) { +EvaluationResult GEvaluator::compute_result(EvaluatorCall &call) { EvaluationResult result; - result.set_evaluator_value(eval_context.get_g_value()); + result.set_evaluator_value(call.get_g_value()); return result; } diff --git a/src/search/evaluators/g_evaluator.h b/src/search/evaluators/g_evaluator.h index 556e66f3dd..753514dd42 100644 --- a/src/search/evaluators/g_evaluator.h +++ b/src/search/evaluators/g_evaluator.h @@ -10,8 +10,7 @@ class GEvaluator : public Evaluator { const std::shared_ptr &task, const std::string &description, utils::Verbosity verbosity); - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; virtual void get_path_dependent_evaluators( std::set &) override { diff --git a/src/search/evaluators/pref_evaluator.cc b/src/search/evaluators/pref_evaluator.cc index 429ce930af..2f1d47e6ed 100644 --- a/src/search/evaluators/pref_evaluator.cc +++ b/src/search/evaluators/pref_evaluator.cc @@ -1,7 +1,7 @@ #include "pref_evaluator.h" -#include "../evaluation_context.h" #include "../evaluation_result.h" +#include "../evaluator_call.h" #include "../plugins/plugin.h" @@ -14,10 +14,9 @@ PrefEvaluator::PrefEvaluator( : Evaluator(task, false, false, false, description, verbosity) { } -EvaluationResult PrefEvaluator::compute_result( - EvaluationContext &eval_context) { +EvaluationResult PrefEvaluator::compute_result(EvaluatorCall &call) { EvaluationResult result; - if (eval_context.is_preferred()) + if (call.is_preferred()) result.set_evaluator_value(0); else result.set_evaluator_value(1); diff --git a/src/search/evaluators/pref_evaluator.h b/src/search/evaluators/pref_evaluator.h index cb9445722b..6da4302e01 100644 --- a/src/search/evaluators/pref_evaluator.h +++ b/src/search/evaluators/pref_evaluator.h @@ -13,8 +13,7 @@ class PrefEvaluator : public Evaluator { const std::shared_ptr &task, const std::string &description, utils::Verbosity verbosity); - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; virtual void get_path_dependent_evaluators( std::set &) override { } diff --git a/src/search/evaluators/state_forwarding_evaluator.cc b/src/search/evaluators/state_forwarding_evaluator.cc index 6e278a3d86..165feefee3 100644 --- a/src/search/evaluators/state_forwarding_evaluator.cc +++ b/src/search/evaluators/state_forwarding_evaluator.cc @@ -1,6 +1,6 @@ #include "state_forwarding_evaluator.h" -#include "../evaluation_context.h" +#include "../evaluator_call.h" using namespace std; @@ -101,15 +101,13 @@ void StateForwardingEvaluator::notify_state_transition( } EvaluationResult StateForwardingEvaluator::compute_result( - EvaluationContext &eval_context) { - State translated_state = convert_state(eval_context.get_state()); - EvaluationContext translated_context(eval_context, translated_state); + EvaluatorCall &call) { + State translated_state = convert_state(call.get_state()); + EvaluatorCall subcall = call.get_subcall(translated_state); /* - Note that we do not need to update the cache inside eval_context. - the call below can only set and access evaluators that use the transformed - task and users of eval_context cannot know such evaluators. + TODO issue1208: this copies the result. Does this make sense? */ - return nested->compute_result(translated_context); + return subcall[nested.get()]; } bool StateForwardingEvaluator::does_cache_estimates() const { diff --git a/src/search/evaluators/state_forwarding_evaluator.h b/src/search/evaluators/state_forwarding_evaluator.h index 506e4644af..276df7a22d 100644 --- a/src/search/evaluators/state_forwarding_evaluator.h +++ b/src/search/evaluators/state_forwarding_evaluator.h @@ -42,8 +42,7 @@ class StateForwardingEvaluator : public Evaluator { virtual void notify_state_transition( const State &parent_state, OperatorID op_id, const State &state) override; - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; virtual bool does_cache_estimates() const override; virtual bool is_estimate_cached(const State &state) const override; virtual int get_cached_estimate(const State &state) const override; diff --git a/src/search/evaluators/weighted_evaluator.cc b/src/search/evaluators/weighted_evaluator.cc index aa9b673684..a7ca8b44f7 100644 --- a/src/search/evaluators/weighted_evaluator.cc +++ b/src/search/evaluators/weighted_evaluator.cc @@ -1,7 +1,7 @@ #include "weighted_evaluator.h" -#include "../evaluation_context.h" #include "../evaluation_result.h" +#include "../evaluator_call.h" #include "../plugins/plugin.h" #include "../utils/component_errors.h" @@ -25,11 +25,10 @@ bool WeightedEvaluator::dead_ends_are_reliable() const { return evaluator->dead_ends_are_reliable(); } -EvaluationResult WeightedEvaluator::compute_result( - EvaluationContext &eval_context) { +EvaluationResult WeightedEvaluator::compute_result(EvaluatorCall &call) { // Note that this produces no preferred operators. EvaluationResult result; - int value = eval_context.get_evaluator_value_or_infinity(evaluator.get()); + int value = call[evaluator.get()].get_evaluator_value(); if (value != EvaluationResult::INFTY) { // TODO: Check for overflow? value *= weight; diff --git a/src/search/evaluators/weighted_evaluator.h b/src/search/evaluators/weighted_evaluator.h index ce51d740fb..54abffbe57 100644 --- a/src/search/evaluators/weighted_evaluator.h +++ b/src/search/evaluators/weighted_evaluator.h @@ -17,8 +17,7 @@ class WeightedEvaluator : public Evaluator { const std::string &description, utils::Verbosity verbosity); virtual bool dead_ends_are_reliable() const override; - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; virtual void get_path_dependent_evaluators( std::set &evals) override; }; diff --git a/src/search/heuristic.cc b/src/search/heuristic.cc index 8fc4d9097e..ac8315cded 100644 --- a/src/search/heuristic.cc +++ b/src/search/heuristic.cc @@ -1,7 +1,7 @@ #include "heuristic.h" -#include "evaluation_context.h" #include "evaluation_result.h" +#include "evaluator_call.h" #include "plugins/plugin.h" #include "task_utils/task_properties.h" @@ -37,14 +37,14 @@ tuple get_heuristic_arguments_from_options( get_evaluator_arguments_from_options(opts)); } -EvaluationResult Heuristic::compute_result(EvaluationContext &eval_context) { +EvaluationResult Heuristic::compute_result(EvaluatorCall &call) { EvaluationResult result; assert(preferred_operators.empty()); - const State &state = eval_context.get_state(); + const State &state = call.get_state(); assert(state.get_task() == task_proxy); - bool calculate_preferred = eval_context.get_calculate_preferred(); + bool calculate_preferred = call.get_calculate_preferred(); int heuristic = NO_VALUE; diff --git a/src/search/heuristic.h b/src/search/heuristic.h index 61447ccb4d..b99bd0f06d 100644 --- a/src/search/heuristic.h +++ b/src/search/heuristic.h @@ -75,8 +75,7 @@ class Heuristic : public Evaluator { std::set & /*evals*/) override { } - virtual EvaluationResult compute_result( - EvaluationContext &eval_context) override; + virtual EvaluationResult compute_result(EvaluatorCall &call) override; virtual bool does_cache_estimates() const override; virtual bool is_estimate_cached(const State &state) const override; From e3c607e5762de1255bcc8c6c2ed95a4778c091ea Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 29 Jul 2026 21:30:10 +0200 Subject: [PATCH 28/29] style --- src/search/evaluators/g_evaluator.cc | 2 +- src/search/evaluators/state_forwarding_evaluator.cc | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/search/evaluators/g_evaluator.cc b/src/search/evaluators/g_evaluator.cc index 7be5c4987e..9f5d5c9cd0 100644 --- a/src/search/evaluators/g_evaluator.cc +++ b/src/search/evaluators/g_evaluator.cc @@ -1,7 +1,7 @@ #include "g_evaluator.h" -#include "../evaluator_call.h" #include "../evaluation_result.h" +#include "../evaluator_call.h" #include "../plugins/plugin.h" diff --git a/src/search/evaluators/state_forwarding_evaluator.cc b/src/search/evaluators/state_forwarding_evaluator.cc index 7494b8085f..c5f74ab54d 100644 --- a/src/search/evaluators/state_forwarding_evaluator.cc +++ b/src/search/evaluators/state_forwarding_evaluator.cc @@ -100,8 +100,7 @@ void StateForwardingEvaluator::notify_state_transition( } } -EvaluationResult StateForwardingEvaluator::compute_result( - EvaluatorCall &call) { +EvaluationResult StateForwardingEvaluator::compute_result(EvaluatorCall &call) { State translated_state = convert_state(call.get_state()); EvaluatorCall subcall = call.get_subcall(translated_state); /* From e5f80c27b3addcb813ca634b29d561db1d8ada52 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 30 Jul 2026 15:53:01 +0200 Subject: [PATCH 29/29] unpack state before heuristic evaluation --- src/search/heuristic.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/search/heuristic.cc b/src/search/heuristic.cc index ac8315cded..6c73f27578 100644 --- a/src/search/heuristic.cc +++ b/src/search/heuristic.cc @@ -44,6 +44,8 @@ EvaluationResult Heuristic::compute_result(EvaluatorCall &call) { const State &state = call.get_state(); assert(state.get_task() == task_proxy); + // TODO issue1236: let each heuristic decide whether to unpack the state. + state.unpack(); bool calculate_preferred = call.get_calculate_preferred(); int heuristic = NO_VALUE;