Skip to content

Commit

Permalink
[issue1170] SCC merge strategy: remove option to use a merge tree.
Browse files Browse the repository at this point in the history
Conceptually, it doesn't make too much sense to use a precomputed merge strategy for a subset of factors, and the option was never used in a paper. We remove it to simplify the code and facilitate the integration of other options.
---------

Co-authored-by: Silvan Sievers <[email protected]>
  • Loading branch information
silvansievers and Silvan Sievers authored Feb 6, 2025
1 parent b1af91a commit 794d0f5
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 78 deletions.
46 changes: 6 additions & 40 deletions src/search/merge_and_shrink/merge_strategy_factory_sccs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@ static bool compare_sccs_decreasing(const vector<int> &lhs, const vector<int> &r

MergeStrategyFactorySCCs::MergeStrategyFactorySCCs(
const OrderOfSCCs &order_of_sccs,
const shared_ptr<MergeTreeFactory> &merge_tree,
const shared_ptr<MergeSelector> &merge_selector,
utils::Verbosity verbosity)
: MergeStrategyFactory(verbosity),
order_of_sccs(order_of_sccs),
merge_tree_factory(merge_tree),
merge_selector(merge_selector) {
}

Expand Down Expand Up @@ -99,26 +97,16 @@ unique_ptr<MergeStrategy> MergeStrategyFactorySCCs::compute_merge_strategy(

return utils::make_unique_ptr<MergeStrategySCCs>(
fts,
task_proxy,
merge_tree_factory,
merge_selector,
move(non_singleton_cg_sccs));
}

bool MergeStrategyFactorySCCs::requires_init_distances() const {
if (merge_tree_factory) {
return merge_tree_factory->requires_init_distances();
} else {
return merge_selector->requires_init_distances();
}
return merge_selector->requires_init_distances();
}

bool MergeStrategyFactorySCCs::requires_goal_distances() const {
if (merge_tree_factory) {
return merge_tree_factory->requires_goal_distances();
} else {
return merge_selector->requires_goal_distances();
}
return merge_selector->requires_goal_distances();
}

void MergeStrategyFactorySCCs::dump_strategy_specific_options() const {
Expand All @@ -141,12 +129,7 @@ void MergeStrategyFactorySCCs::dump_strategy_specific_options() const {
log << endl;

log << "Merge strategy for merging within sccs: " << endl;
if (merge_tree_factory) {
merge_tree_factory->dump_options(log);
}
if (merge_selector) {
merge_selector->dump_options(log);
}
merge_selector->dump_options(log);
}
}

Expand Down Expand Up @@ -182,35 +165,18 @@ class MergeStrategyFactorySCCsFeature
"order_of_sccs",
"how the SCCs should be ordered",
"topological");
add_option<shared_ptr<MergeTreeFactory>>(
"merge_tree",
"the fallback merge strategy to use if a precomputed strategy should "
"be used.",
plugins::ArgumentInfo::NO_DEFAULT);
add_option<shared_ptr<MergeSelector>>(
"merge_selector",
"the fallback merge strategy to use if a stateless strategy should "
"be used.",
plugins::ArgumentInfo::NO_DEFAULT);
"the fallback merge strategy to use");
add_merge_strategy_options_to_feature(*this);
}

virtual shared_ptr<MergeStrategyFactorySCCs> create_component(
const plugins::Options &opts,
const utils::Context &context) const override {
bool merge_tree = opts.contains("merge_tree");
bool merge_selector = opts.contains("merge_selector");
if ((merge_tree && merge_selector) || (!merge_tree && !merge_selector)) {
context.error(
"You have to specify exactly one of the options merge_tree "
"and merge_selector!");
}
const utils::Context &) const override {
return plugins::make_shared_from_arg_tuples<MergeStrategyFactorySCCs>(
opts.get<OrderOfSCCs>("order_of_sccs"),
opts.get<shared_ptr<MergeTreeFactory>> (
"merge_tree", nullptr),
opts.get<shared_ptr<MergeSelector>> (
"merge_selector", nullptr),
opts.get<shared_ptr<MergeSelector>> ("merge_selector"),
get_merge_strategy_arguments_from_options(opts)
);
}
Expand Down
3 changes: 0 additions & 3 deletions src/search/merge_and_shrink/merge_strategy_factory_sccs.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "merge_strategy_factory.h"

namespace merge_and_shrink {
class MergeTreeFactory;
class MergeSelector;

enum class OrderOfSCCs {
Expand All @@ -16,15 +15,13 @@ enum class OrderOfSCCs {

class MergeStrategyFactorySCCs : public MergeStrategyFactory {
OrderOfSCCs order_of_sccs;
std::shared_ptr<MergeTreeFactory> merge_tree_factory;
std::shared_ptr<MergeSelector> merge_selector;
protected:
virtual std::string name() const override;
virtual void dump_strategy_specific_options() const override;
public:
MergeStrategyFactorySCCs(
const OrderOfSCCs &order_of_sccs,
const std::shared_ptr<MergeTreeFactory> &merge_tree,
const std::shared_ptr<MergeSelector> &merge_selector,
utils::Verbosity verbosity);
virtual std::unique_ptr<MergeStrategy> compute_merge_strategy(
Expand Down
29 changes: 3 additions & 26 deletions src/search/merge_and_shrink/merge_strategy_sccs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,11 @@ using namespace std;
namespace merge_and_shrink {
MergeStrategySCCs::MergeStrategySCCs(
const FactoredTransitionSystem &fts,
const TaskProxy &task_proxy,
const shared_ptr<MergeTreeFactory> &merge_tree_factory,
const shared_ptr<MergeSelector> &merge_selector,
vector<vector<int>> &&non_singleton_cg_sccs)
: MergeStrategy(fts),
task_proxy(task_proxy),
merge_tree_factory(merge_tree_factory),
merge_selector(merge_selector),
non_singleton_cg_sccs(move(non_singleton_cg_sccs)),
current_merge_tree(nullptr) {
non_singleton_cg_sccs(move(non_singleton_cg_sccs)) {
}

MergeStrategySCCs::~MergeStrategySCCs() {
Expand Down Expand Up @@ -53,31 +48,13 @@ pair<int, int> MergeStrategySCCs::get_next() {
current_ts_indices = move(current_scc);
non_singleton_cg_sccs.erase(non_singleton_cg_sccs.begin());
}

// If using a merge tree factory, compute a merge tree for this set.
if (merge_tree_factory) {
current_merge_tree = merge_tree_factory->compute_merge_tree(
task_proxy, fts, current_ts_indices);
}
} else {
// Add the most recent product to the current index set.
current_ts_indices.push_back(fts.get_size() - 1);
}

// Select the next merge from the current index set, either using the
// tree or the selector.
pair<int, int > next_pair;
int merged_ts_index = fts.get_size();
if (current_merge_tree) {
assert(!current_merge_tree->done());
next_pair = current_merge_tree->get_next_merge(merged_ts_index);
if (current_merge_tree->done()) {
current_merge_tree = nullptr;
}
} else {
assert(merge_selector);
next_pair = merge_selector->select_merge(fts, current_ts_indices);
}
// Select the next merge for the current set of indices.
pair<int, int > next_pair = merge_selector->select_merge(fts, current_ts_indices);

// Remove the two merged indices from the current index set.
for (vector<int>::iterator it = current_ts_indices.begin();
Expand Down
9 changes: 0 additions & 9 deletions src/search/merge_and_shrink/merge_strategy_sccs.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,16 @@
#include <memory>
#include <vector>

class TaskProxy;

namespace merge_and_shrink {
class MergeSelector;
class MergeTreeFactory;
class MergeTree;
class MergeStrategySCCs : public MergeStrategy {
const TaskProxy &task_proxy;
std::shared_ptr<MergeTreeFactory> merge_tree_factory;
std::shared_ptr<MergeSelector> merge_selector;
std::vector<std::vector<int>> non_singleton_cg_sccs;

std::unique_ptr<MergeTree> current_merge_tree;
std::vector<int> current_ts_indices;
public:
MergeStrategySCCs(
const FactoredTransitionSystem &fts,
const TaskProxy &task_proxy,
const std::shared_ptr<MergeTreeFactory> &merge_tree_factory,
const std::shared_ptr<MergeSelector> &merge_selector,
std::vector<std::vector<int>> &&non_singleton_cg_sccs);
virtual ~MergeStrategySCCs() override;
Expand Down

0 comments on commit 794d0f5

Please sign in to comment.