Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue1113 #179

Merged
merged 9 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/search/parser/abstract_syntax_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ void FunctionCallNode::collect_keyword_arguments(
DecorateContext &context, CollectedArguments &arguments) const {
unordered_map<string, plugins::ArgumentInfo> argument_infos_by_key;
for (const plugins::ArgumentInfo &arg_info : argument_infos) {
assert(!argument_infos_by_key.contains(arg_info.key));
argument_infos_by_key.insert({arg_info.key, arg_info});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,6 @@ static void add_hillclimbing_options(plugins::Feature &feature) {
"infinity",
plugins::Bounds("0.0", "infinity"));
utils::add_rng_options(feature);
add_generator_options_to_feature(feature);
}

static void check_hillclimbing_options(
Expand Down Expand Up @@ -596,6 +595,7 @@ class PatternCollectionGeneratorHillclimbingFeature : public plugins::TypedFeatu
"optimized for the Evaluator#Canonical_PDB heuristic. It it described "
"in the following paper:" + paper_references());
add_hillclimbing_options(*this);
add_generator_options_to_feature(*this);
}

virtual shared_ptr<PatternCollectionGeneratorHillclimbing> create_component(const plugins::Options &options, const utils::Context &context) const override {
Expand Down
40 changes: 26 additions & 14 deletions src/search/plugins/raw_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,56 +118,68 @@ SubcategoryPlugins RawRegistry::collect_subcategory_plugins(
Features RawRegistry::collect_features(
const SubcategoryPlugins &subcategory_plugins, vector<string> &errors) const {
Features features;
unordered_map<string, int> key_occurrences;
unordered_map<string, int> feature_key_occurrences;
for (const Plugin *plugin : plugins) {
shared_ptr<Feature> feature = plugin->create_feature();
string key = feature->get_key();
key_occurrences[key]++;
features[key] = move(feature);
string feature_key = feature->get_key();
feature_key_occurrences[feature_key]++;
features[feature_key] = move(feature);
}

// Check that keys are unique
for (const auto &pair : key_occurrences) {
const string &key = pair.first;
// Check that feature_keys are unique
for (const auto &pair : feature_key_occurrences) {
const string &feature_key = pair.first;
int occurrences = pair.second;
if (occurrences > 1) {
errors.push_back(
to_string(occurrences) + " Features are defined for the key '" +
key + "'.");
feature_key + "'.");
}
}

// Check that all subcategories used in features are defined
for (const auto &item : features) {
const string &key = item.first;
const string &feature_key = item.first;
const Feature &feature = *item.second;
string subcategory = feature.get_subcategory();

if (!subcategory.empty() && !subcategory_plugins.count(subcategory)) {
const Type &type = feature.get_type();
errors.push_back(
"Missing SubcategoryPlugin '" + subcategory + "' for Plugin '" +
key + "' of type " + type.name());
feature_key + "' of type " + type.name());
}
}

// Check that all types used in features are defined
unordered_set<type_index> missing_types;
for (const auto &item : features) {
const string &key = item.first;
const string &feature_key = item.first;
const Feature &feature = *item.second;

const Type &type = feature.get_type();
if (type == TypeRegistry::NO_TYPE) {
errors.push_back(
"Missing Plugin for type of feature '" + key + "'.");
"Missing Plugin for type of feature '" + feature_key + "'.");
}

unordered_map<string, int> parameter_occurrences;
for (const ArgumentInfo &arg_info : feature.get_arguments()) {
if (arg_info.type == TypeRegistry::NO_TYPE) {
errors.push_back(
"Missing Plugin for type of argument '" + arg_info.key
+ "' of feature '" + key + "'.");
"Missing Plugin for type of parameter '" + arg_info.key
+ "' of feature '" + feature_key + "'.");
}
++parameter_occurrences[arg_info.key];
}
// Check that parameters are unique
for (const auto &pair : parameter_occurrences) {
const string &parameter = pair.first;
int parameter_occurrence = pair.second;
if (parameter_occurrence > 1) {
errors.push_back(
"The Parameter '" + parameter + "' in '" + feature_key + "' is defined " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before you merge, can you change "Parameter" to "parameter" in the message? (The existing messages can also be criticized for the same kind of grammatical mistake, but we can argue that we think of "Plugin" as a technical proper noun in this context. For "parameter" we can't.)

to_string(parameter_occurrence) + " times.");
}
}
}
Expand Down