-
Notifications
You must be signed in to change notification settings - Fork 96
Check config key #1792
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
base: develop
Are you sure you want to change the base?
Check config key #1792
Conversation
2ea21ad
to
ec6eda5
Compare
ec6eda5
to
91c9732
Compare
6e141fe
to
db95be4
Compare
Suggestion: use the pnode to keep track of which keys of a map were accessed. If not all keys were accessed, then there are some invalid keys. |
Implementation example: if (auto& obj = config.get("max_iters")) by std::set<std::string> allowed_keys;
auto get_config_value = [](const char* key) {
allowed_keys.insert(key);
return config.get(key);
};
// ...
if (auto& obj = get_config_value("max_iters")) The main goal is to specify every key name string only once, so we avoid accidental typos. |
db95be4
to
fabb554
Compare
It merge the allowed keys preparation with trying getting nodes now |
fabb554
to
2234682
Compare
This PR adds the check to verify the keys in config are allowed by predefined keys set.
typo in config is quite easy in the file because we do not have auto-complete for that
It avoids typo or misusage of parameters from user such that the configuring solver is not what users expect without notice.
Isome thoughts will be nice.
allowed_keys
preparation: predefine it as early as possible or insert the key into the set close to the conditionI am predefining it as early as possible. but I think the second way might makes sense when the class has long list of keys like multigrid. It makes the manual type together and reduce the chance that missing deleting one of condition and allowed key. The first one is somehow easy to set with constructor not many lines of
set.insert("key")