From 1a724ebb74fae06792edf83473804e1129bd711e Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Mon, 3 Mar 2025 18:29:51 +0100 Subject: [PATCH 01/23] use prototype implementation as a basis --- .../refinement_hierarchy.cc | 2 +- .../subtask_generators.cc | 2 +- src/search/heuristics/cea_heuristic.cc | 7 +- src/search/heuristics/cg_cache.cc | 2 +- src/search/heuristics/cg_heuristic.cc | 6 +- src/search/merge_and_shrink/fts_factory.cc | 2 +- .../merge_and_shrink_representation.cc | 2 +- .../state_equation_constraints.cc | 2 +- src/search/pdbs/cegar.cc | 2 +- src/search/pruning/stubborn_sets.h | 2 +- .../pruning/stubborn_sets_atom_centric.cc | 6 +- src/search/pruning/stubborn_sets_ec.cc | 6 +- src/search/state_registry.cc | 2 +- src/search/task_proxy.h | 65 +++++++++++++++---- 14 files changed, 75 insertions(+), 33 deletions(-) diff --git a/src/search/cartesian_abstractions/refinement_hierarchy.cc b/src/search/cartesian_abstractions/refinement_hierarchy.cc index 7431b118ef..704f54c48c 100644 --- a/src/search/cartesian_abstractions/refinement_hierarchy.cc +++ b/src/search/cartesian_abstractions/refinement_hierarchy.cc @@ -60,7 +60,7 @@ NodeID RefinementHierarchy::get_node_id(const State &state) const { NodeID id = 0; while (nodes[id].is_split()) { const Node &node = nodes[id]; - id = node.get_child(state[node.get_var()].get_value()); + id = node.get_child(state[node.get_var()]); } return id; } diff --git a/src/search/cartesian_abstractions/subtask_generators.cc b/src/search/cartesian_abstractions/subtask_generators.cc index 0229992891..dcde99f3aa 100644 --- a/src/search/cartesian_abstractions/subtask_generators.cc +++ b/src/search/cartesian_abstractions/subtask_generators.cc @@ -52,7 +52,7 @@ static void remove_initial_state_facts( const TaskProxy &task_proxy, Facts &facts) { State initial_state = task_proxy.get_initial_state(); facts.erase(remove_if(facts.begin(), facts.end(), [&](FactPair fact) { - return initial_state[fact.var].get_value() == fact.value; + return initial_state[fact.var] == fact.value; }), facts.end()); } diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index 12aba20f85..e070afa7ef 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -228,7 +228,8 @@ void ContextEnhancedAdditiveHeuristic::set_up_local_problem( LocalProblemNode *start = &problem->nodes[start_value]; start->cost = 0; for (size_t i = 0; i < problem->context_variables->size(); ++i) - start->context[i] = state[(*problem->context_variables)[i]].get_value(); + // TODO issue997: is casting from int to a short here fine? + start->context[i] = static_cast(state[(*problem->context_variables)[i]]); add_to_heap(start); } @@ -379,10 +380,10 @@ void ContextEnhancedAdditiveHeuristic::mark_helpful_transitions( int precond_value = assignment.value; int local_var = assignment.local_var; int precond_var_no = context_vars[local_var]; - if (state[precond_var_no].get_value() == precond_value) + if (state[precond_var_no] == precond_value) continue; LocalProblem *subproblem = get_local_problem( - precond_var_no, state[precond_var_no].get_value()); + precond_var_no, state[precond_var_no]); LocalProblemNode *subnode = &subproblem->nodes[precond_value]; mark_helpful_transitions(subproblem, subnode, state); } diff --git a/src/search/heuristics/cg_cache.cc b/src/search/heuristics/cg_cache.cc index 2479136d34..c431e51312 100644 --- a/src/search/heuristics/cg_cache.cc +++ b/src/search/heuristics/cg_cache.cc @@ -119,7 +119,7 @@ int CGCache::get_index(int var, const State &state, int index = from_val; int multiplier = task_proxy.get_variables()[var].get_domain_size(); for (int dep_var : depends_on[var]) { - index += state[dep_var].get_value() * multiplier; + index += state[dep_var] * multiplier; multiplier *= task_proxy.get_variables()[dep_var].get_domain_size(); } if (to_val > from_val) diff --git a/src/search/heuristics/cg_heuristic.cc b/src/search/heuristics/cg_heuristic.cc index 8ec18a3deb..d8047bb53d 100644 --- a/src/search/heuristics/cg_heuristic.cc +++ b/src/search/heuristics/cg_heuristic.cc @@ -58,7 +58,7 @@ int CGHeuristic::compute_heuristic(const State &ancestor_state) { for (FactProxy goal : task_proxy.get_goals()) { const VariableProxy var = goal.get_variable(); int var_no = var.get_id(); - int from = state[var_no].get_value(), to = goal.get_value(); + int from = state[var_no], to = goal.get_value(); DomainTransitionGraph *dtg = transition_graphs[var_no].get(); int cost_for_goal = get_transition_cost(state, dtg, from, to); if (cost_for_goal == numeric_limits::max()) { @@ -114,7 +114,7 @@ int CGHeuristic::get_transition_cost(const State &state, start->children_state.resize(dtg->local_to_global_child.size()); for (size_t i = 0; i < dtg->local_to_global_child.size(); ++i) { start->children_state[i] = - state[dtg->local_to_global_child[i]].get_value(); + state[dtg->local_to_global_child[i]]; } // Initialize Heap for Dijkstra's algorithm. @@ -226,7 +226,7 @@ int CGHeuristic::get_transition_cost(const State &state, void CGHeuristic::mark_helpful_transitions(const State &state, DomainTransitionGraph *dtg, int to) { int var_no = dtg->var; - int from = state[var_no].get_value(); + int from = state[var_no]; if (from == to) return; diff --git a/src/search/merge_and_shrink/fts_factory.cc b/src/search/merge_and_shrink/fts_factory.cc index 48604f1680..9528b1ed79 100644 --- a/src/search/merge_and_shrink/fts_factory.cc +++ b/src/search/merge_and_shrink/fts_factory.cc @@ -118,7 +118,7 @@ unique_ptr FTSFactory::create_labels() { void FTSFactory::build_state_data(VariableProxy var) { int var_id = var.get_id(); TransitionSystemData &ts_data = transition_system_data_by_var[var_id]; - ts_data.init_state = task_proxy.get_initial_state()[var_id].get_value(); + ts_data.init_state = task_proxy.get_initial_state()[var_id]; int range = task_proxy.get_variables()[var_id].get_domain_size(); ts_data.num_states = range; diff --git a/src/search/merge_and_shrink/merge_and_shrink_representation.cc b/src/search/merge_and_shrink/merge_and_shrink_representation.cc index 18731721ca..7fcd08657f 100644 --- a/src/search/merge_and_shrink/merge_and_shrink_representation.cc +++ b/src/search/merge_and_shrink/merge_and_shrink_representation.cc @@ -58,7 +58,7 @@ void MergeAndShrinkRepresentationLeaf::apply_abstraction_to_lookup_table( } int MergeAndShrinkRepresentationLeaf::get_value(const State &state) const { - int value = state[var_id].get_value(); + int value = state[var_id]; return lookup_table[value]; } diff --git a/src/search/operator_counting/state_equation_constraints.cc b/src/search/operator_counting/state_equation_constraints.cc index 877056d8b7..51c4531185 100644 --- a/src/search/operator_counting/state_equation_constraints.cc +++ b/src/search/operator_counting/state_equation_constraints.cc @@ -103,7 +103,7 @@ bool StateEquationConstraints::update_constraints(const State &state, double lower_bound = 0; /* If we consider the current value of var, there must be an additional consumer. */ - if (state[var].get_value() == value) { + if (state[var] == value) { --lower_bound; } /* If we consider the goal value of var, there must be an diff --git a/src/search/pdbs/cegar.cc b/src/search/pdbs/cegar.cc index 0c2c5e179c..911b6f3356 100644 --- a/src/search/pdbs/cegar.cc +++ b/src/search/pdbs/cegar.cc @@ -387,7 +387,7 @@ bool CEGAR::get_flaws_for_pattern( bool raise_goal_flaw = false; for (const FactPair &goal : goals) { int goal_var_id = goal.var; - if (final_state[goal_var_id].get_value() != goal.value && + if (final_state[goal_var_id] != goal.value && !blacklisted_variables.count(goal_var_id)) { flaws.emplace_back(collection_index, goal_var_id); raise_goal_flaw = true; diff --git a/src/search/pruning/stubborn_sets.h b/src/search/pruning/stubborn_sets.h index 402395ddec..43807046f0 100644 --- a/src/search/pruning/stubborn_sets.h +++ b/src/search/pruning/stubborn_sets.h @@ -62,7 +62,7 @@ class StubbornSets : public PruningMethod { inline FactPair find_unsatisfied_condition( const std::vector &conditions, const State &state) { for (const FactPair &condition : conditions) { - if (state[condition.var].get_value() != condition.value) + if (state[condition.var] != condition.value) return condition; } return FactPair::no_fact; diff --git a/src/search/pruning/stubborn_sets_atom_centric.cc b/src/search/pruning/stubborn_sets_atom_centric.cc index c3b6fc1027..4f78f757be 100644 --- a/src/search/pruning/stubborn_sets_atom_centric.cc +++ b/src/search/pruning/stubborn_sets_atom_centric.cc @@ -138,7 +138,7 @@ FactPair StubbornSetsAtomCentric::select_fact( choose it. Otherwise, choose the first unsatisfied fact. */ for (const FactPair &condition : facts) { - if (state[condition.var].get_value() != condition.value) { + if (state[condition.var] != condition.value) { if (marked_producers[condition.var][condition.value]) { fact = condition; break; @@ -150,7 +150,7 @@ FactPair StubbornSetsAtomCentric::select_fact( } else if (atom_selection_strategy == AtomSelectionStrategy::STATIC_SMALL) { int min_count = numeric_limits::max(); for (const FactPair &condition : facts) { - if (state[condition.var].get_value() != condition.value) { + if (state[condition.var] != condition.value) { int count = achievers[condition.var][condition.value].size(); if (count < min_count) { fact = condition; @@ -161,7 +161,7 @@ FactPair StubbornSetsAtomCentric::select_fact( } else if (atom_selection_strategy == AtomSelectionStrategy::DYNAMIC_SMALL) { int min_count = numeric_limits::max(); for (const FactPair &condition : facts) { - if (state[condition.var].get_value() != condition.value) { + if (state[condition.var] != condition.value) { const vector &ops = achievers[condition.var][condition.value]; int count = count_if( ops.begin(), ops.end(), [&](int op) {return !stubborn[op];}); diff --git a/src/search/pruning/stubborn_sets_ec.cc b/src/search/pruning/stubborn_sets_ec.cc index d6b2972fde..bc6f0a953b 100644 --- a/src/search/pruning/stubborn_sets_ec.cc +++ b/src/search/pruning/stubborn_sets_ec.cc @@ -18,7 +18,7 @@ static inline bool is_v_applicable(int var, const State &state, vector> &preconditions) { int precondition_on_var = preconditions[op_no][var]; - return precondition_on_var == -1 || precondition_on_var == state[var].get_value(); + return precondition_on_var == -1 || precondition_on_var == state[var]; } static vector build_dtgs(TaskProxy task_proxy) { @@ -164,7 +164,7 @@ void StubbornSetsEC::compute_active_operators(const State &state) { for (const FactPair &precondition : sorted_op_preconditions[op_no]) { int var_id = precondition.var; - int current_value = state[var_id].get_value(); + int current_value = state[var_id]; const vector &reachable_values = reachability_map[var_id][current_value]; if (!reachable_values[precondition.value]) { @@ -258,7 +258,7 @@ void StubbornSetsEC::get_disabled_vars( void StubbornSetsEC::apply_s5(int op_no, const State &state) { // Find a violated state variable and check if stubborn contains a writer for this variable. for (const FactPair &pre : sorted_op_preconditions[op_no]) { - if (state[pre.var].get_value() != pre.value && written_vars[pre.var]) { + if (state[pre.var] != pre.value && written_vars[pre.var]) { if (!nes_computed[pre.var][pre.value]) { add_nes_for_fact(pre, state); } diff --git a/src/search/state_registry.cc b/src/search/state_registry.cc index a494c05023..47a07b6374 100644 --- a/src/search/state_registry.cc +++ b/src/search/state_registry.cc @@ -56,7 +56,7 @@ const State &StateRegistry::get_initial_state() { State initial_state = task_proxy.get_initial_state(); for (size_t i = 0; i < initial_state.size(); ++i) { - state_packer.set(buffer.get(), i, initial_state[i].get_value()); + state_packer.set(buffer.get(), i, initial_state[i]); } state_data_pool.push_back(buffer.get()); StateID id = insert_id_or_pop_state(); diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 0a134a93b6..75b0827e02 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -91,15 +91,12 @@ using PackedStateBin = int_packer::IntPacker::Bin; task_properties.h module. */ -template -concept has_item_type = requires { - typename T::ItemType; -}; /* Basic iterator support for proxy collections. */ -template +template::value>::type* = nullptr> class ProxyIterator { /* We store a pointer to collection instead of a reference because iterators have to be copy assignable. */ @@ -586,8 +583,6 @@ class State { const int_packer::IntPacker *state_packer; int num_variables; public: - using ItemType = FactProxy; - // Construct a registered state with only packed data. State(const AbstractTask &task, const StateRegistry ®istry, StateID id, const PackedStateBin *buffer); @@ -605,7 +600,7 @@ class State { void unpack() const; std::size_t size() const; - FactProxy operator[](std::size_t var_id) const; + int operator[](std::size_t var_id) const; FactProxy operator[](VariableProxy var) const; TaskProxy get_task() const; @@ -805,19 +800,19 @@ inline std::size_t State::size() const { return num_variables; } -inline FactProxy State::operator[](std::size_t var_id) const { +inline int State::operator[](std::size_t var_id) const { assert(var_id < size()); if (values) { - return FactProxy(*task, var_id, (*values)[var_id]); + return (*values)[var_id]; } else { assert(buffer); assert(state_packer); - return FactProxy(*task, var_id, state_packer->get(buffer, var_id)); + return state_packer->get(buffer, var_id); } } inline FactProxy State::operator[](VariableProxy var) const { - return (*this)[var.get_id()]; + return FactProxy(*task, var.get_id(), (*this)[var.get_id()]); } inline TaskProxy State::get_task() const { @@ -857,4 +852,50 @@ inline const std::vector &State::get_unpacked_values() const { } return *values; } + +class StateIterator { + ProxyIterator variables_iterator; + const State *state; +public: + using iterator_category = std::input_iterator_tag; + using value_type = FactProxy; + using difference_type = int; + using pointer = const value_type *; + using reference = value_type; + + StateIterator(const State &state, std::size_t pos) + : variables_iterator(state.get_task().get_variables(), pos), state(&state) { + } + + reference operator*() const { + return (*state)[*variables_iterator]; + } + + value_type operator++(int) { + value_type value(**this); + ++(*this); + return value; + } + + StateIterator &operator++() { + ++variables_iterator; + return *this; + } + + bool operator==(const StateIterator &other) const { + return variables_iterator == other.variables_iterator; + } + + bool operator!=(const StateIterator &other) const { + return !(*this == other); + } +}; + +inline StateIterator begin(const State &state) { + return StateIterator(state, 0); +} + +inline StateIterator end(const State &state) { + return StateIterator(state, state.size()); +} #endif From 513f233f98033650b551be5d0b48ef4d690ebad9 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 10 Apr 2025 11:53:29 +0200 Subject: [PATCH 02/23] too complex attempt to solve this with concepts --- src/search/task_proxy.h | 118 ++++++++++++++++++++++------------------ 1 file changed, 65 insertions(+), 53 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 75b0827e02..45ab57b199 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -7,7 +7,6 @@ #include "task_id.h" #include "algorithms/int_packer.h" -#include "utils/collections.h" #include "utils/hash.h" #include "utils/system.h" @@ -91,30 +90,51 @@ using PackedStateBin = int_packer::IntPacker::Bin; task_properties.h module. */ +template +concept can_be_dereferenced = requires (Pos pos) { + { *pos }; +}; + +template +constexpr auto dereference_if_necessary(Pos p) { + if constexpr (can_be_dereferenced) { + return *p; + } else { + return p; + } +} + +template +concept indexable_with = requires (Container container, Pos pos) { + requires std::same_as>; + { container.size() } -> std::integral; + { container[dereference_if_necessary(pos)] } + -> std::same_as>; +}; /* Basic iterator support for proxy collections. */ -template::value>::type* = nullptr> +template + requires indexable_with class ProxyIterator { /* We store a pointer to collection instead of a reference because iterators have to be copy assignable. */ const ProxyCollection *collection; - std::size_t pos; + Pos pos; public: using iterator_category = std::input_iterator_tag; - using value_type = typename ProxyCollection::ItemType; + using value_type = decltype((*collection)[0]); using difference_type = int; using pointer = const value_type *; using reference = value_type; - ProxyIterator(const ProxyCollection &collection, std::size_t pos) + ProxyIterator(const ProxyCollection &collection, Pos pos) : collection(&collection), pos(pos) { } reference operator*() const { - return (*collection)[pos]; + return (*collection)[dereference_if_necessary(pos)]; } value_type operator++(int) { @@ -138,17 +158,6 @@ class ProxyIterator { } }; -template -inline ProxyIterator begin(ProxyCollection &collection) { - return ProxyIterator(collection, 0); -} - -template -inline ProxyIterator end(ProxyCollection &collection) { - return ProxyIterator(collection, collection.size()); -} - - class FactProxy { const AbstractTask *task; FactPair fact; @@ -853,49 +862,52 @@ inline const std::vector &State::get_unpacked_values() const { return *values; } -class StateIterator { - ProxyIterator variables_iterator; - const State *state; -public: - using iterator_category = std::input_iterator_tag; - using value_type = FactProxy; - using difference_type = int; - using pointer = const value_type *; - using reference = value_type; +inline ProxyIterator> begin(const State &state); +inline ProxyIterator> end(const State &state); - StateIterator(const State &state, std::size_t pos) - : variables_iterator(state.get_task().get_variables(), pos), state(&state) { - } +template + requires (!std::same_as) +inline ProxyIterator begin(ProxyCollection &collection) { + return ProxyIterator(collection, 0); +} - reference operator*() const { - return (*state)[*variables_iterator]; - } +template + requires (!std::same_as) +inline ProxyIterator begin(const ProxyCollection &collection) { + return ProxyIterator(collection, 0); +} - value_type operator++(int) { - value_type value(**this); - ++(*this); - return value; - } +template +inline ProxyIterator end(ProxyCollection &collection) { + return ProxyIterator(collection, collection.size()); +} - StateIterator &operator++() { - ++variables_iterator; - return *this; - } +template +inline ProxyIterator end(const ProxyCollection &collection) { + return ProxyIterator(collection, collection.size()); +} - bool operator==(const StateIterator &other) const { - return variables_iterator == other.variables_iterator; - } +static_assert(std::input_iterator>); +static_assert(std::input_iterator>>); +inline ProxyIterator> begin(const State &state) { + ProxyIterator variables_it = begin(state.get_task().get_variables()); + return ProxyIterator>(state, variables_it); +} - bool operator!=(const StateIterator &other) const { - return !(*this == other); - } -}; +inline ProxyIterator> end(const State &state) { + ProxyIterator variables_it = end(state.get_task().get_variables()); + return ProxyIterator>(state, variables_it); +} -inline StateIterator begin(const State &state) { - return StateIterator(state, 0); +inline ProxyIterator> begin(State &state) { + ProxyIterator variables_it = begin(state.get_task().get_variables()); + return ProxyIterator>(state, variables_it); } -inline StateIterator end(const State &state) { - return StateIterator(state, state.size()); +inline ProxyIterator> end(State &state) { + ProxyIterator variables_it = end(state.get_task().get_variables()); + return ProxyIterator>(state, variables_it); } + + #endif From 45177ca39b47dce93fcbb5e54720cf6f4f47d613 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 10 Apr 2025 13:02:59 +0200 Subject: [PATCH 03/23] use template specialization --- src/search/task_proxy.h | 110 ++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 65 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 45ab57b199..c6a760d6bf 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -7,6 +7,7 @@ #include "task_id.h" #include "algorithms/int_packer.h" +#include "utils/collections.h" #include "utils/hash.h" #include "utils/system.h" @@ -90,38 +91,22 @@ using PackedStateBin = int_packer::IntPacker::Bin; task_properties.h module. */ -template -concept can_be_dereferenced = requires (Pos pos) { - { *pos }; -}; - -template -constexpr auto dereference_if_necessary(Pos p) { - if constexpr (can_be_dereferenced) { - return *p; - } else { - return p; - } -} - -template -concept indexable_with = requires (Container container, Pos pos) { +template +concept indexable = requires (Container container, std::size_t i) { requires std::same_as>; { container.size() } -> std::integral; - { container[dereference_if_necessary(pos)] } - -> std::same_as>; + { container[i] }; }; /* Basic iterator support for proxy collections. */ -template - requires indexable_with +template class ProxyIterator { /* We store a pointer to collection instead of a reference because iterators have to be copy assignable. */ const ProxyCollection *collection; - Pos pos; + std::size_t pos; public: using iterator_category = std::input_iterator_tag; using value_type = decltype((*collection)[0]); @@ -129,12 +114,12 @@ class ProxyIterator { using pointer = const value_type *; using reference = value_type; - ProxyIterator(const ProxyCollection &collection, Pos pos) + ProxyIterator(const ProxyCollection &collection, std::size_t pos) : collection(&collection), pos(pos) { } reference operator*() const { - return (*collection)[dereference_if_necessary(pos)]; + return (*collection)[pos]; } value_type operator++(int) { @@ -158,6 +143,18 @@ class ProxyIterator { } }; +template +inline ProxyIterator begin(const ProxyCollection &collection) { + return ProxyIterator(collection, 0); +} + +template +inline ProxyIterator end(const ProxyCollection &collection) { + return ProxyIterator(collection, collection.size()); +} + + + class FactProxy { const AbstractTask *task; FactPair fact; @@ -862,52 +859,35 @@ inline const std::vector &State::get_unpacked_values() const { return *values; } -inline ProxyIterator> begin(const State &state); -inline ProxyIterator> end(const State &state); - -template - requires (!std::same_as) -inline ProxyIterator begin(ProxyCollection &collection) { - return ProxyIterator(collection, 0); -} - -template - requires (!std::same_as) -inline ProxyIterator begin(const ProxyCollection &collection) { - return ProxyIterator(collection, 0); -} - -template -inline ProxyIterator end(ProxyCollection &collection) { - return ProxyIterator(collection, collection.size()); -} - -template -inline ProxyIterator end(const ProxyCollection &collection) { - return ProxyIterator(collection, collection.size()); -} +template<> +class ProxyIterator { + const State *state; + const VariablesProxy variables; + int var_id; +public: + ProxyIterator(const State &state, int var_id) + : state(&state), variables(state.get_task().get_variables()), var_id(var_id) { + } -static_assert(std::input_iterator>); -static_assert(std::input_iterator>>); -inline ProxyIterator> begin(const State &state) { - ProxyIterator variables_it = begin(state.get_task().get_variables()); - return ProxyIterator>(state, variables_it); -} + FactProxy operator*() const { + return (*state)[variables[var_id]]; + } -inline ProxyIterator> end(const State &state) { - ProxyIterator variables_it = end(state.get_task().get_variables()); - return ProxyIterator>(state, variables_it); -} + ProxyIterator &operator++() { + assert(var_id < variables.size()); + ++var_id; + return *this; + } -inline ProxyIterator> begin(State &state) { - ProxyIterator variables_it = begin(state.get_task().get_variables()); - return ProxyIterator>(state, variables_it); -} + bool operator==(const ProxyIterator &other) const { + assert(state == other.state); + return var_id == other.var_id; + } -inline ProxyIterator> end(State &state) { - ProxyIterator variables_it = end(state.get_task().get_variables()); - return ProxyIterator>(state, variables_it); -} + bool operator!=(const ProxyIterator &other) const { + return !(*this == other); + } +}; #endif From 7da6461ec0c2c1f537d848e4eb283d0df5d6d4b6 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 10 Apr 2025 14:21:21 +0200 Subject: [PATCH 04/23] fix style --- src/search/task_proxy.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index c6a760d6bf..342ac577bd 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -92,10 +92,15 @@ using PackedStateBin = int_packer::IntPacker::Bin; */ template -concept indexable = requires (Container container, std::size_t i) { +concept indexable = requires(Container container, std::size_t i) { requires std::same_as>; - { container.size() } -> std::integral; - { container[i] }; + { + container.size() + } + ->std::integral; + { + container[i] + } }; /* @@ -143,12 +148,12 @@ class ProxyIterator { } }; -template +template inline ProxyIterator begin(const ProxyCollection &collection) { return ProxyIterator(collection, 0); } -template +template inline ProxyIterator end(const ProxyCollection &collection) { return ProxyIterator(collection, collection.size()); } From 6d3a39634c6aea520f0ea3a35c991b134b2cd050 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 10 Apr 2025 14:37:13 +0200 Subject: [PATCH 05/23] remove item type --- src/search/task_proxy.h | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 342ac577bd..a86a314097 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -100,7 +100,7 @@ concept indexable = requires(Container container, std::size_t i) { ->std::integral; { container[i] - } + }; }; /* @@ -263,7 +263,6 @@ class ConditionsProxy { protected: const AbstractTask *task; public: - using ItemType = FactProxy; explicit ConditionsProxy(const AbstractTask &task) : task(&task) {} virtual ~ConditionsProxy() = default; @@ -337,7 +336,6 @@ class VariableProxy { class VariablesProxy { const AbstractTask *task; public: - using ItemType = VariableProxy; explicit VariablesProxy(const AbstractTask &task) : task(&task) {} ~VariablesProxy() = default; @@ -425,7 +423,6 @@ class EffectsProxy { int op_index; bool is_axiom; public: - using ItemType = EffectProxy; EffectsProxy(const AbstractTask &task, int op_index, bool is_axiom) : task(&task), op_index(op_index), is_axiom(is_axiom) {} ~EffectsProxy() = default; @@ -498,7 +495,6 @@ class OperatorProxy { class OperatorsProxy { const AbstractTask *task; public: - using ItemType = OperatorProxy; explicit OperatorsProxy(const AbstractTask &task) : task(&task) {} ~OperatorsProxy() = default; @@ -525,7 +521,6 @@ class OperatorsProxy { class AxiomsProxy { const AbstractTask *task; public: - using ItemType = OperatorProxy; explicit AxiomsProxy(const AbstractTask &task) : task(&task) {} ~AxiomsProxy() = default; From b0392a336e58ea8e44b1abb1251e66fdff212684 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 12 Apr 2025 12:06:00 +0200 Subject: [PATCH 06/23] change uncrustify config to accept the concept definition --- .uncrustify.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.uncrustify.cfg b/.uncrustify.cfg index ab316995b7..9bb2f29c10 100644 --- a/.uncrustify.cfg +++ b/.uncrustify.cfg @@ -98,7 +98,7 @@ eat_blanks_after_open_brace=true eat_blanks_before_close_brace=true mod_pawn_semicolon=false mod_full_paren_if_bool=false -mod_remove_extra_semicolon=true +mod_remove_extra_semicolon=false mod_sort_import=false mod_sort_using=false mod_sort_include=false From b75d2776e817cf686addd41fbadeffbecfddd56f Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 12 Apr 2025 13:22:33 +0200 Subject: [PATCH 07/23] fix concept definition --- src/search/task_proxy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index a86a314097..28cea153a1 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -92,7 +92,7 @@ using PackedStateBin = int_packer::IntPacker::Bin; */ template -concept indexable = requires(Container container, std::size_t i) { +concept indexable = requires(Container &container, std::size_t i) { requires std::same_as>; { container.size() From d7975aa346948fcc64c95fa044ec677fbeac2da5 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 12 Apr 2025 13:25:33 +0200 Subject: [PATCH 08/23] 'fix' style --- src/search/task_proxy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 28cea153a1..6319ce404d 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -92,7 +92,7 @@ using PackedStateBin = int_packer::IntPacker::Bin; */ template -concept indexable = requires(Container &container, std::size_t i) { +concept indexable = requires(Container & container, std::size_t i) { requires std::same_as>; { container.size() From b80dec24378511b1fc298f11ef2745b56abe4b79 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 12 Apr 2025 18:58:10 +0200 Subject: [PATCH 09/23] turn iterator classes into true iterators --- src/search/task_proxy.h | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 6319ce404d..7a67f03931 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -113,17 +113,14 @@ class ProxyIterator { const ProxyCollection *collection; std::size_t pos; public: - using iterator_category = std::input_iterator_tag; using value_type = decltype((*collection)[0]); using difference_type = int; - using pointer = const value_type *; - using reference = value_type; ProxyIterator(const ProxyCollection &collection, std::size_t pos) : collection(&collection), pos(pos) { } - reference operator*() const { + value_type operator*() const { return (*collection)[pos]; } @@ -202,11 +199,13 @@ class FactsProxyIterator { int var_id; int value; public: + using value_type = FactProxy; + using difference_type = int; + FactsProxyIterator(const AbstractTask &task, int var_id, int value) : task(&task), var_id(var_id), value(value) {} - ~FactsProxyIterator() = default; - FactProxy operator*() const { + value_type operator*() const { return FactProxy(*task, var_id, value); } @@ -222,6 +221,12 @@ class FactsProxyIterator { return *this; } + value_type operator++(int) { + value_type fact(**this); + ++(*this); + return fact; + } + bool operator==(const FactsProxyIterator &other) const { assert(task == other.task); return var_id == other.var_id && value == other.value; @@ -862,14 +867,17 @@ inline const std::vector &State::get_unpacked_values() const { template<> class ProxyIterator { const State *state; - const VariablesProxy variables; + VariablesProxy variables; int var_id; public: + using difference_type = int; + using value_type = FactProxy; + ProxyIterator(const State &state, int var_id) : state(&state), variables(state.get_task().get_variables()), var_id(var_id) { } - FactProxy operator*() const { + value_type operator*() const { return (*state)[variables[var_id]]; } @@ -879,6 +887,12 @@ class ProxyIterator { return *this; } + value_type operator++(int) { + value_type fact(**this); + ++(*this); + return fact; + } + bool operator==(const ProxyIterator &other) const { assert(state == other.state); return var_id == other.var_id; @@ -889,5 +903,16 @@ class ProxyIterator { } }; +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); +static_assert(std::input_iterator); +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); +static_assert(std::input_iterator>); + #endif From a6e2a89dcc6170185a3d9a761f120e013590ffc7 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 12 Apr 2025 20:13:10 +0200 Subject: [PATCH 10/23] C++-20 supports default implementations of == and !=. --- src/search/task_proxy.h | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 7a67f03931..8553c46901 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -135,14 +135,7 @@ class ProxyIterator { return *this; } - bool operator==(const ProxyIterator &other) const { - assert(collection == other.collection); - return pos == other.pos; - } - - bool operator!=(const ProxyIterator &other) const { - return !(*this == other); - } + bool operator==(const ProxyIterator &other) const = default; }; template @@ -227,14 +220,7 @@ class FactsProxyIterator { return fact; } - bool operator==(const FactsProxyIterator &other) const { - assert(task == other.task); - return var_id == other.var_id && value == other.value; - } - - bool operator!=(const FactsProxyIterator &other) const { - return !(*this == other); - } + bool operator==(const FactsProxyIterator &other) const = default; }; @@ -897,10 +883,6 @@ class ProxyIterator { assert(state == other.state); return var_id == other.var_id; } - - bool operator!=(const ProxyIterator &other) const { - return !(*this == other); - } }; static_assert(std::input_iterator>); From 53f0dabdd13a43c0d282096a5f618022a6bc8b5a Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sat, 12 Apr 2025 20:29:54 +0200 Subject: [PATCH 11/23] use nested iterator in state iterator --- src/search/task_proxy.h | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 8553c46901..4082c23265 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -853,23 +853,21 @@ inline const std::vector &State::get_unpacked_values() const { template<> class ProxyIterator { const State *state; - VariablesProxy variables; - int var_id; + ProxyIterator pos; public: using difference_type = int; using value_type = FactProxy; ProxyIterator(const State &state, int var_id) - : state(&state), variables(state.get_task().get_variables()), var_id(var_id) { + : state(&state), pos(state.get_task().get_variables(), var_id) { } value_type operator*() const { - return (*state)[variables[var_id]]; + return (*state)[*pos]; } ProxyIterator &operator++() { - assert(var_id < variables.size()); - ++var_id; + ++pos; return *this; } @@ -879,10 +877,7 @@ class ProxyIterator { return fact; } - bool operator==(const ProxyIterator &other) const { - assert(state == other.state); - return var_id == other.var_id; - } + bool operator==(const ProxyIterator &other) const = default; }; static_assert(std::input_iterator>); From 6a394066e188e44642e81df8ddada518cddef739 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Sun, 13 Apr 2025 00:40:51 +0200 Subject: [PATCH 12/23] avoid stale reference --- src/search/task_proxy.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 4082c23265..45a3dc25e7 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -599,6 +599,7 @@ class State { std::size_t size() const; int operator[](std::size_t var_id) const; FactProxy operator[](VariableProxy var) const; + FactProxy get_fact(std::size_t var_id) const; TaskProxy get_task() const; @@ -809,7 +810,11 @@ inline int State::operator[](std::size_t var_id) const { } inline FactProxy State::operator[](VariableProxy var) const { - return FactProxy(*task, var.get_id(), (*this)[var.get_id()]); + return get_fact(var.get_id()); +} + +inline FactProxy State::get_fact(std::size_t var_id) const { + return FactProxy(*task, var_id, (*this)[var_id]); } inline TaskProxy State::get_task() const { @@ -853,21 +858,21 @@ inline const std::vector &State::get_unpacked_values() const { template<> class ProxyIterator { const State *state; - ProxyIterator pos; + int var_id; public: using difference_type = int; using value_type = FactProxy; ProxyIterator(const State &state, int var_id) - : state(&state), pos(state.get_task().get_variables(), var_id) { + : state(&state), var_id(var_id) { } value_type operator*() const { - return (*state)[*pos]; + return state->get_fact(var_id); } ProxyIterator &operator++() { - ++pos; + ++var_id; return *this; } From 0fb546bbbaf8ef509825c1bc9f17227e1dd09cef Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Mon, 14 Apr 2025 18:44:22 +0200 Subject: [PATCH 13/23] try reintroducing reference and pointer typedefs for MSVC. --- src/search/task_proxy.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 45a3dc25e7..14dbd53388 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -114,7 +114,9 @@ class ProxyIterator { std::size_t pos; public: using value_type = decltype((*collection)[0]); - using difference_type = int; + using difference_type = int; // unused but required by the iterator concept + using reference = value_type; // unused but required by older MSVC versions + using pointer = value_type*; // unused but required by older MSVC versions ProxyIterator(const ProxyCollection &collection, std::size_t pos) : collection(&collection), pos(pos) { @@ -193,7 +195,9 @@ class FactsProxyIterator { int value; public: using value_type = FactProxy; - using difference_type = int; + using difference_type = int; // unused but required by the iterator concept + using reference = FactProxy; // unused but required by older MSVC versions + using pointer = FactProxy*; // unused but required by older MSVC versions FactsProxyIterator(const AbstractTask &task, int var_id, int value) : task(&task), var_id(var_id), value(value) {} @@ -860,8 +864,10 @@ class ProxyIterator { const State *state; int var_id; public: - using difference_type = int; + using difference_type = int; // unused but required by the iterator concept using value_type = FactProxy; + using reference = FactProxy; // unused but required by older MSVC versions + using pointer = FactProxy*; // unused but required by older MSVC versions ProxyIterator(const State &state, int var_id) : state(&state), var_id(var_id) { From 56204efa710e22addb8fdc06005eda30b92c9007 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Tue, 22 Apr 2025 15:53:47 +0200 Subject: [PATCH 14/23] use windows 2025 instead of 2019 --- .github/workflows/windows.yml | 4 ++-- src/search/task_proxy.h | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 7c631a8970..5a87339cba 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -26,8 +26,8 @@ jobs: strategy: matrix: platform: - - {os: "windows-2022", vc: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} - - {os: "windows-2019", vc: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} + - {os: "windows-2022", vc: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} + - {os: "windows-2025", vc: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} python-version: [3.8] steps: - name: Clone repository diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 14dbd53388..58c2036965 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -115,8 +115,6 @@ class ProxyIterator { public: using value_type = decltype((*collection)[0]); using difference_type = int; // unused but required by the iterator concept - using reference = value_type; // unused but required by older MSVC versions - using pointer = value_type*; // unused but required by older MSVC versions ProxyIterator(const ProxyCollection &collection, std::size_t pos) : collection(&collection), pos(pos) { @@ -196,8 +194,6 @@ class FactsProxyIterator { public: using value_type = FactProxy; using difference_type = int; // unused but required by the iterator concept - using reference = FactProxy; // unused but required by older MSVC versions - using pointer = FactProxy*; // unused but required by older MSVC versions FactsProxyIterator(const AbstractTask &task, int var_id, int value) : task(&task), var_id(var_id), value(value) {} @@ -866,8 +862,6 @@ class ProxyIterator { public: using difference_type = int; // unused but required by the iterator concept using value_type = FactProxy; - using reference = FactProxy; // unused but required by older MSVC versions - using pointer = FactProxy*; // unused but required by older MSVC versions ProxyIterator(const State &state, int var_id) : state(&state), var_id(var_id) { From 80f892cfd2270bfe373ef93c95adee3e4d6b120e Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 25 Apr 2025 02:35:34 +0200 Subject: [PATCH 15/23] turn proxy classes into ranges --- src/search/task_proxy.h | 29 ++++++++++++++++++++++++++++- src/search/utils/collections.h | 3 +-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 58c2036965..f0ee5ae08c 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -116,6 +116,8 @@ class ProxyIterator { using value_type = decltype((*collection)[0]); using difference_type = int; // unused but required by the iterator concept + ProxyIterator() = default; + ProxyIterator(const ProxyIterator &other) = default; ProxyIterator(const ProxyCollection &collection, std::size_t pos) : collection(&collection), pos(pos) { } @@ -148,6 +150,15 @@ inline ProxyIterator end(const ProxyCollection &collection) { return ProxyIterator(collection, collection.size()); } +template +inline ProxyIterator begin(ProxyCollection &collection) { + return ProxyIterator(collection, 0); +} + +template +inline ProxyIterator end(ProxyCollection &collection) { + return ProxyIterator(collection, collection.size()); +} class FactProxy { @@ -195,6 +206,8 @@ class FactsProxyIterator { using value_type = FactProxy; using difference_type = int; // unused but required by the iterator concept + FactsProxyIterator() = default; + FactsProxyIterator(const FactsProxyIterator &) = default; FactsProxyIterator(const AbstractTask &task, int var_id, int value) : task(&task), var_id(var_id), value(value) {} @@ -863,6 +876,8 @@ class ProxyIterator { using difference_type = int; // unused but required by the iterator concept using value_type = FactProxy; + ProxyIterator() = default; + ProxyIterator(const ProxyIterator &other) = default; ProxyIterator(const State &state, int var_id) : state(&state), var_id(var_id) { } @@ -889,12 +904,24 @@ static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); -static_assert(std::input_iterator); static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); +static_assert(std::ranges::range); + +static_assert(std::input_iterator); +static_assert(std::ranges::range); + #endif diff --git a/src/search/utils/collections.h b/src/search/utils/collections.h index 5d13592ffa..c78f80ac3d 100644 --- a/src/search/utils/collections.h +++ b/src/search/utils/collections.h @@ -77,8 +77,7 @@ template std::vector map_vector(const Collection &collection, MapFunc map_func) { std::vector transformed; transformed.reserve(collection.size()); - std::transform(begin(collection), end(collection), - std::back_inserter(transformed), map_func); + std::ranges::transform(collection, std::back_inserter(transformed), map_func); return transformed; } From ec33f38f8dd4c138eee7760269e286670ff2c1b8 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 25 Apr 2025 14:08:39 +0200 Subject: [PATCH 16/23] code cleanup (remove implicitly defined special constructors, destructors, revert workflow) --- .github/workflows/windows.yml | 4 ++-- src/search/heuristics/cea_heuristic.cc | 1 - src/search/task_proxy.h | 16 ---------------- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 5a87339cba..7c631a8970 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -26,8 +26,8 @@ jobs: strategy: matrix: platform: - - {os: "windows-2022", vc: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} - - {os: "windows-2025", vc: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} + - {os: "windows-2022", vc: "C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} + - {os: "windows-2019", vc: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Auxiliary\\Build\\vcvarsall.bat"} python-version: [3.8] steps: - name: Clone repository diff --git a/src/search/heuristics/cea_heuristic.cc b/src/search/heuristics/cea_heuristic.cc index e070afa7ef..807a5b42bf 100644 --- a/src/search/heuristics/cea_heuristic.cc +++ b/src/search/heuristics/cea_heuristic.cc @@ -228,7 +228,6 @@ void ContextEnhancedAdditiveHeuristic::set_up_local_problem( LocalProblemNode *start = &problem->nodes[start_value]; start->cost = 0; for (size_t i = 0; i < problem->context_variables->size(); ++i) - // TODO issue997: is casting from int to a short here fine? start->context[i] = static_cast(state[(*problem->context_variables)[i]]); add_to_heap(start); diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index f0ee5ae08c..691ee44f84 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -117,7 +117,6 @@ class ProxyIterator { using difference_type = int; // unused but required by the iterator concept ProxyIterator() = default; - ProxyIterator(const ProxyIterator &other) = default; ProxyIterator(const ProxyCollection &collection, std::size_t pos) : collection(&collection), pos(pos) { } @@ -167,7 +166,6 @@ class FactProxy { public: FactProxy(const AbstractTask &task, int var_id, int value); FactProxy(const AbstractTask &task, const FactPair &fact); - ~FactProxy() = default; VariableProxy get_variable() const; @@ -207,7 +205,6 @@ class FactsProxyIterator { using difference_type = int; // unused but required by the iterator concept FactsProxyIterator() = default; - FactsProxyIterator(const FactsProxyIterator &) = default; FactsProxyIterator(const AbstractTask &task, int var_id, int value) : task(&task), var_id(var_id), value(value) {} @@ -251,7 +248,6 @@ class FactsProxy { public: explicit FactsProxy(const AbstractTask &task) : task(&task) {} - ~FactsProxy() = default; FactsProxyIterator begin() const { return FactsProxyIterator(*task, 0, 0); @@ -286,7 +282,6 @@ class VariableProxy { public: VariableProxy(const AbstractTask &task, int id) : task(&task), id(id) {} - ~VariableProxy() = default; bool operator==(const VariableProxy &other) const { assert(task == other.task); @@ -342,7 +337,6 @@ class VariablesProxy { public: explicit VariablesProxy(const AbstractTask &task) : task(&task) {} - ~VariablesProxy() = default; std::size_t size() const { return task->get_num_variables(); @@ -365,7 +359,6 @@ class PreconditionsProxy : public ConditionsProxy { public: PreconditionsProxy(const AbstractTask &task, int op_index, bool is_axiom) : ConditionsProxy(task), op_index(op_index), is_axiom(is_axiom) {} - ~PreconditionsProxy() = default; std::size_t size() const override { return task->get_num_operator_preconditions(op_index, is_axiom); @@ -387,7 +380,6 @@ class EffectConditionsProxy : public ConditionsProxy { EffectConditionsProxy( const AbstractTask &task, int op_index, int eff_index, bool is_axiom) : ConditionsProxy(task), op_index(op_index), eff_index(eff_index), is_axiom(is_axiom) {} - ~EffectConditionsProxy() = default; std::size_t size() const override { return task->get_num_operator_effect_conditions(op_index, eff_index, is_axiom); @@ -409,7 +401,6 @@ class EffectProxy { public: EffectProxy(const AbstractTask &task, int op_index, int eff_index, bool is_axiom) : task(&task), op_index(op_index), eff_index(eff_index), is_axiom(is_axiom) {} - ~EffectProxy() = default; EffectConditionsProxy get_conditions() const { return EffectConditionsProxy(*task, op_index, eff_index, is_axiom); @@ -429,7 +420,6 @@ class EffectsProxy { public: EffectsProxy(const AbstractTask &task, int op_index, bool is_axiom) : task(&task), op_index(op_index), is_axiom(is_axiom) {} - ~EffectsProxy() = default; std::size_t size() const { return task->get_num_operator_effects(op_index, is_axiom); @@ -449,7 +439,6 @@ class OperatorProxy { public: OperatorProxy(const AbstractTask &task, int index, bool is_axiom) : task(&task), index(index), is_an_axiom(is_axiom) {} - ~OperatorProxy() = default; bool operator==(const OperatorProxy &other) const { assert(task == other.task); @@ -501,7 +490,6 @@ class OperatorsProxy { public: explicit OperatorsProxy(const AbstractTask &task) : task(&task) {} - ~OperatorsProxy() = default; std::size_t size() const { return task->get_num_operators(); @@ -527,7 +515,6 @@ class AxiomsProxy { public: explicit AxiomsProxy(const AbstractTask &task) : task(&task) {} - ~AxiomsProxy() = default; std::size_t size() const { return task->get_num_axioms(); @@ -548,7 +535,6 @@ class GoalsProxy : public ConditionsProxy { public: explicit GoalsProxy(const AbstractTask &task) : ConditionsProxy(task) {} - ~GoalsProxy() = default; std::size_t size() const override { return task->get_num_goals(); @@ -665,7 +651,6 @@ class TaskProxy { public: explicit TaskProxy(const AbstractTask &task) : task(&task) {} - ~TaskProxy() = default; TaskID get_id() const { return TaskID(task); @@ -877,7 +862,6 @@ class ProxyIterator { using value_type = FactProxy; ProxyIterator() = default; - ProxyIterator(const ProxyIterator &other) = default; ProxyIterator(const State &state, int var_id) : state(&state), var_id(var_id) { } From 4f9a44bd122215cda2adf9a794ab6bc6947ae4db Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Wed, 18 Jun 2025 17:24:47 +0200 Subject: [PATCH 17/23] Make ProxyIterator opt-in --- src/search/task_proxy.h | 55 +++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 691ee44f84..1113c3fbe1 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -92,21 +92,23 @@ using PackedStateBin = int_packer::IntPacker::Bin; */ template -concept indexable = requires(Container & container, std::size_t i) { +concept proxy_iterator_enabled = requires(Container & container, std::size_t i) { + typename Container::ItemType; requires std::same_as>; { container.size() } - ->std::integral; + -> std::integral; { container[i] - }; + } + -> std::same_as; }; /* Basic iterator support for proxy collections. */ -template +template class ProxyIterator { /* We store a pointer to collection instead of a reference because iterators have to be copy assignable. */ @@ -139,22 +141,22 @@ class ProxyIterator { bool operator==(const ProxyIterator &other) const = default; }; -template +template inline ProxyIterator begin(const ProxyCollection &collection) { return ProxyIterator(collection, 0); } -template +template inline ProxyIterator end(const ProxyCollection &collection) { return ProxyIterator(collection, collection.size()); } -template +template inline ProxyIterator begin(ProxyCollection &collection) { return ProxyIterator(collection, 0); } -template +template inline ProxyIterator end(ProxyCollection &collection) { return ProxyIterator(collection, collection.size()); } @@ -263,6 +265,7 @@ class ConditionsProxy { protected: const AbstractTask *task; public: + using ItemType = FactProxy; explicit ConditionsProxy(const AbstractTask &task) : task(&task) {} virtual ~ConditionsProxy() = default; @@ -335,6 +338,7 @@ class VariableProxy { class VariablesProxy { const AbstractTask *task; public: + using ItemType = VariableProxy; explicit VariablesProxy(const AbstractTask &task) : task(&task) {} @@ -418,6 +422,7 @@ class EffectsProxy { int op_index; bool is_axiom; public: + using ItemType = EffectProxy; EffectsProxy(const AbstractTask &task, int op_index, bool is_axiom) : task(&task), op_index(op_index), is_axiom(is_axiom) {} @@ -488,6 +493,7 @@ class OperatorProxy { class OperatorsProxy { const AbstractTask *task; public: + using ItemType = OperatorProxy; explicit OperatorsProxy(const AbstractTask &task) : task(&task) {} @@ -513,6 +519,7 @@ class OperatorsProxy { class AxiomsProxy { const AbstractTask *task; public: + using ItemType = OperatorProxy; explicit AxiomsProxy(const AbstractTask &task) : task(&task) {} @@ -853,16 +860,15 @@ inline const std::vector &State::get_unpacked_values() const { return *values; } -template<> -class ProxyIterator { +class StateFactsIterator { const State *state; int var_id; public: using difference_type = int; // unused but required by the iterator concept using value_type = FactProxy; - ProxyIterator() = default; - ProxyIterator(const State &state, int var_id) + StateFactsIterator() = default; + StateFactsIterator(const State &state, int var_id) : state(&state), var_id(var_id) { } @@ -870,7 +876,7 @@ class ProxyIterator { return state->get_fact(var_id); } - ProxyIterator &operator++() { + StateFactsIterator &operator++() { ++var_id; return *this; } @@ -881,9 +887,25 @@ class ProxyIterator { return fact; } - bool operator==(const ProxyIterator &other) const = default; + bool operator==(const StateFactsIterator &other) const = default; }; +inline StateFactsIterator begin(const State &state) { + return StateFactsIterator(state, 0); +} + +inline StateFactsIterator end(const State &state) { + return StateFactsIterator(state, state.size()); +} + +inline StateFactsIterator begin(State &state) { + return StateFactsIterator(state, 0); +} + +inline StateFactsIterator end(State &state) { + return StateFactsIterator(state, state.size()); +} + static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); @@ -891,7 +913,6 @@ static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::input_iterator>); -static_assert(std::input_iterator>); static_assert(std::input_iterator>); static_assert(std::ranges::range); @@ -901,9 +922,11 @@ static_assert(std::ranges::range); static_assert(std::ranges::range); static_assert(std::ranges::range); static_assert(std::ranges::range); -static_assert(std::ranges::range); static_assert(std::ranges::range); +static_assert(std::input_iterator); +static_assert(std::ranges::range); + static_assert(std::input_iterator); static_assert(std::ranges::range); From 69406db33c4f53fb352f6e8bad5a607bcd1f91a3 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Tue, 15 Jul 2025 11:30:27 +0200 Subject: [PATCH 18/23] code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clemens Büchner --- src/search/task_proxy.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 1113c3fbe1..5519502519 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -92,7 +92,7 @@ using PackedStateBin = int_packer::IntPacker::Bin; */ template -concept proxy_iterator_enabled = requires(Container & container, std::size_t i) { +concept proxy_iterator_enabled = requires(Container &container, std::size_t i) { typename Container::ItemType; requires std::same_as>; { From 04bd50cfbad515933b957a2239750d6850a8a211 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Tue, 15 Jul 2025 12:07:35 +0200 Subject: [PATCH 19/23] changes after rebase --- src/search/landmarks/landmark.cc | 2 +- src/search/landmarks/landmark_factory_rpg_sasp.cc | 10 +++++----- src/search/landmarks/util.cc | 4 +++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/search/landmarks/landmark.cc b/src/search/landmarks/landmark.cc index f9fb0f787c..0a19b1b6bc 100644 --- a/src/search/landmarks/landmark.cc +++ b/src/search/landmarks/landmark.cc @@ -7,7 +7,7 @@ using namespace std; namespace landmarks { bool Landmark::is_true_in_state(const State &state) const { auto is_atom_true_in_state = [&](const FactPair &atom) { - return state[atom.var].get_value() == atom.value; + return state[atom.var] == atom.value; }; if (type == DISJUNCTIVE) { return ranges::any_of(atoms, is_atom_true_in_state); diff --git a/src/search/landmarks/landmark_factory_rpg_sasp.cc b/src/search/landmarks/landmark_factory_rpg_sasp.cc index 7608ac696e..88cc50bc30 100644 --- a/src/search/landmarks/landmark_factory_rpg_sasp.cc +++ b/src/search/landmarks/landmark_factory_rpg_sasp.cc @@ -120,12 +120,12 @@ static void add_binary_variable_conditions( effect_atom.get_variable().get_domain_size() == 2) { for (const FactPair &atom : landmark.atoms) { if (atom.var == var_id && - initial_state[var_id].get_value() != atom.value) { + initial_state[var_id] != atom.value) { assert(ranges::none_of(result, [&](const FactPair &result_atom) { return result_atom.var == var_id; })); - result.insert(initial_state[var_id].get_pair()); + result.insert(FactPair(var_id, initial_state[var_id])); break; } } @@ -530,7 +530,7 @@ void LandmarkFactoryRpgSasp::generate_disjunctive_precondition_landmarks( they should not hold in the initial state. */ if (preconditions.size() < 5 && ranges::none_of( preconditions, [&](const FactPair &atom) { - return initial_state[atom.var].get_value() == atom.value; + return initial_state[atom.var] == atom.value; })) { add_disjunctive_landmark_and_ordering( preconditions, *node, OrderingType::GREEDY_NECESSARY); @@ -653,8 +653,8 @@ void LandmarkFactoryRpgSasp::approximate_lookahead_orderings( assert(landmark.atoms.size() == 1); const FactPair landmark_atom = landmark.atoms[0]; - const FactPair init_atom = - task_proxy.get_initial_state()[landmark_atom.var].get_pair(); + const FactPair init_atom = FactPair(landmark_atom.var, + task_proxy.get_initial_state()[landmark_atom.var]); vector critical_predecessors = get_critical_dtg_predecessors( init_atom.value, landmark_atom.value, reached[landmark_atom.var], dtg_successors[landmark_atom.var]); diff --git a/src/search/landmarks/util.cc b/src/search/landmarks/util.cc index 280f2b3e1a..80a2c4d714 100644 --- a/src/search/landmarks/util.cc +++ b/src/search/landmarks/util.cc @@ -6,6 +6,8 @@ #include "../task_proxy.h" #include "../utils/logging.h" +#include + using namespace std; namespace landmarks { @@ -32,7 +34,7 @@ bool possibly_reaches_landmark(const OperatorProxy &op, // Check whether an effect of `op` reaches an atom in `landmark`. EffectsProxy effects = op.get_effects(); - return any_of(begin(effects), end(effects), [&](const EffectProxy &effect) { + return ranges::any_of(effects, [&](const EffectProxy &effect) { return landmark.contains(effect.get_fact().get_pair()) && condition_is_reachable(effect.get_conditions(), reached); }); From 299ddd4db326c02777f90cf558ff75e72f909a7b Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Tue, 15 Jul 2025 12:33:30 +0200 Subject: [PATCH 20/23] fix style --- src/search/landmarks/landmark_factory_rpg_sasp.cc | 4 ++-- src/search/landmarks/util.cc | 6 +++--- src/search/task_proxy.h | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/search/landmarks/landmark_factory_rpg_sasp.cc b/src/search/landmarks/landmark_factory_rpg_sasp.cc index 88cc50bc30..db9579a513 100644 --- a/src/search/landmarks/landmark_factory_rpg_sasp.cc +++ b/src/search/landmarks/landmark_factory_rpg_sasp.cc @@ -653,8 +653,8 @@ void LandmarkFactoryRpgSasp::approximate_lookahead_orderings( assert(landmark.atoms.size() == 1); const FactPair landmark_atom = landmark.atoms[0]; - const FactPair init_atom = FactPair(landmark_atom.var, - task_proxy.get_initial_state()[landmark_atom.var]); + const FactPair init_atom = FactPair( + landmark_atom.var, task_proxy.get_initial_state()[landmark_atom.var]); vector critical_predecessors = get_critical_dtg_predecessors( init_atom.value, landmark_atom.value, reached[landmark_atom.var], dtg_successors[landmark_atom.var]); diff --git a/src/search/landmarks/util.cc b/src/search/landmarks/util.cc index 80a2c4d714..a211e718c5 100644 --- a/src/search/landmarks/util.cc +++ b/src/search/landmarks/util.cc @@ -35,9 +35,9 @@ bool possibly_reaches_landmark(const OperatorProxy &op, // Check whether an effect of `op` reaches an atom in `landmark`. EffectsProxy effects = op.get_effects(); return ranges::any_of(effects, [&](const EffectProxy &effect) { - return landmark.contains(effect.get_fact().get_pair()) && - condition_is_reachable(effect.get_conditions(), reached); - }); + return landmark.contains(effect.get_fact().get_pair()) && + condition_is_reachable(effect.get_conditions(), reached); + }); } utils::HashSet get_intersection( diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 5519502519..95842a618a 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -92,17 +92,17 @@ using PackedStateBin = int_packer::IntPacker::Bin; */ template -concept proxy_iterator_enabled = requires(Container &container, std::size_t i) { +concept proxy_iterator_enabled = requires(Container & container, std::size_t i) { typename Container::ItemType; requires std::same_as>; { container.size() } - -> std::integral; + ->std::integral; { container[i] } - -> std::same_as; + ->std::same_as; }; /* From 4c0251830c9c799778b1898c8229a8bce193a04e Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Thu, 17 Jul 2025 10:42:20 +0200 Subject: [PATCH 21/23] switch back to legacy iterators --- src/search/landmarks/util.cc | 10 ++++------ src/search/task_proxy.h | 3 +++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/search/landmarks/util.cc b/src/search/landmarks/util.cc index a211e718c5..280f2b3e1a 100644 --- a/src/search/landmarks/util.cc +++ b/src/search/landmarks/util.cc @@ -6,8 +6,6 @@ #include "../task_proxy.h" #include "../utils/logging.h" -#include - using namespace std; namespace landmarks { @@ -34,10 +32,10 @@ bool possibly_reaches_landmark(const OperatorProxy &op, // Check whether an effect of `op` reaches an atom in `landmark`. EffectsProxy effects = op.get_effects(); - return ranges::any_of(effects, [&](const EffectProxy &effect) { - return landmark.contains(effect.get_fact().get_pair()) && - condition_is_reachable(effect.get_conditions(), reached); - }); + return any_of(begin(effects), end(effects), [&](const EffectProxy &effect) { + return landmark.contains(effect.get_fact().get_pair()) && + condition_is_reachable(effect.get_conditions(), reached); + }); } utils::HashSet get_intersection( diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 95842a618a..4d190835a6 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -117,6 +117,9 @@ class ProxyIterator { public: using value_type = decltype((*collection)[0]); using difference_type = int; // unused but required by the iterator concept + using iterator_category = std::input_iterator_tag; // required for legacy iterators + using pointer = value_type*; // required for legacy iterators + using reference = value_type; // required for legacy iterators ProxyIterator() = default; ProxyIterator(const ProxyCollection &collection, std::size_t pos) From de5d1997cbed18541c6dad723f56e75121796ca2 Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Fri, 18 Jul 2025 18:21:03 +0200 Subject: [PATCH 22/23] move construction of FactPair back to taskproxy header --- src/search/landmarks/landmark_factory_rpg_sasp.cc | 6 +++--- src/search/task_proxy.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/search/landmarks/landmark_factory_rpg_sasp.cc b/src/search/landmarks/landmark_factory_rpg_sasp.cc index db9579a513..ed23d64321 100644 --- a/src/search/landmarks/landmark_factory_rpg_sasp.cc +++ b/src/search/landmarks/landmark_factory_rpg_sasp.cc @@ -125,7 +125,7 @@ static void add_binary_variable_conditions( [&](const FactPair &result_atom) { return result_atom.var == var_id; })); - result.insert(FactPair(var_id, initial_state[var_id])); + result.insert(initial_state.get_fact(var_id).get_pair()); break; } } @@ -653,8 +653,8 @@ void LandmarkFactoryRpgSasp::approximate_lookahead_orderings( assert(landmark.atoms.size() == 1); const FactPair landmark_atom = landmark.atoms[0]; - const FactPair init_atom = FactPair( - landmark_atom.var, task_proxy.get_initial_state()[landmark_atom.var]); + const FactPair init_atom = + task_proxy.get_initial_state().get_fact(landmark_atom.var).get_pair(); vector critical_predecessors = get_critical_dtg_predecessors( init_atom.value, landmark_atom.value, reached[landmark_atom.var], dtg_successors[landmark_atom.var]); diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 4d190835a6..88cba42505 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -118,7 +118,7 @@ class ProxyIterator { using value_type = decltype((*collection)[0]); using difference_type = int; // unused but required by the iterator concept using iterator_category = std::input_iterator_tag; // required for legacy iterators - using pointer = value_type*; // required for legacy iterators + using pointer = value_type *; // required for legacy iterators using reference = value_type; // required for legacy iterators ProxyIterator() = default; From 1a7b484613cfb6bca06c04805aaee617c4d0db9b Mon Sep 17 00:00:00 2001 From: Florian Pommerening Date: Tue, 17 Feb 2026 15:51:00 +0100 Subject: [PATCH 23/23] don't use default implementations of equality operators for proxy iterators --- src/search/task_proxy.h | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/search/task_proxy.h b/src/search/task_proxy.h index 88cba42505..a8446861f5 100644 --- a/src/search/task_proxy.h +++ b/src/search/task_proxy.h @@ -141,7 +141,14 @@ class ProxyIterator { return *this; } - bool operator==(const ProxyIterator &other) const = default; + bool operator==(const ProxyIterator &other) const { + assert(collection == other.collection); + return pos == other.pos; + } + + bool operator!=(const ProxyIterator &other) const { + return !(*this == other); + } }; template @@ -235,7 +242,14 @@ class FactsProxyIterator { return fact; } - bool operator==(const FactsProxyIterator &other) const = default; + bool operator==(const FactsProxyIterator &other) const { + assert(task == other.task); + return var_id == other.var_id && value == other.value; + } + + bool operator!=(const FactsProxyIterator &other) const { + return !(*this == other); + } };