[1072] make conjunctive landmarks usable for preferred operators #149
Conversation
| @@ -0,0 +1,107 @@ | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
Experiments should be removed before merging.
| @@ -30,6 +30,8 @@ namespace landmarks { | |||
| LandmarkCountHeuristic::LandmarkCountHeuristic(const plugins::Options &opts) | |||
| : Heuristic(opts), | |||
| use_preferred_operators(opts.get<bool>("pref")), | |||
| use_preferred_operators_conjunctive(opts.get<bool>("use_preferred_operators_conjunctive")), | |||
There was a problem hiding this comment.
I assume this option (and the one just below) is going to be removed before merging.
| Nodes nodes; | ||
|
|
||
| void remove_node_occurrences(LandmarkNode *node); | ||
| void remove_node_occurrences(LandmarkNode *node, bool erase_conjunctive); |
There was a problem hiding this comment.
Also here, I'm assuming that the options erase_conjunctive and store_conjunctive are removed and the code behaves as in the true case.
| lm_fact)->second); | ||
| auto it = std::find(conjunctive_landmarks_vector->begin(), conjunctive_landmarks_vector->end(), node); | ||
| if (it != conjunctive_landmarks_vector->end()) { | ||
| conjunctive_landmarks_vector->erase(it); |
There was a problem hiding this comment.
Maybe turn the if into an assert when we remove the erase_conjunctive option.
| @@ -177,4 +205,6 @@ void LandmarkGraph::set_landmark_ids() { | |||
| ++id; | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Remove those two newlines. Maybe (one of) the reason why the style checks fail?
| @@ -242,25 +245,52 @@ void LandmarkCountHeuristic::generate_preferred_operators( | |||
| FactProxy fact_proxy = effect.get_fact(); | |||
| LandmarkNode *lm_node = lgraph->get_node(fact_proxy.get_pair()); | |||
| if (lm_node && landmark_is_interesting( | |||
| state, reached, *lm_node, all_landmarks_reached)) { | |||
| state, reached, *lm_node, all_landmarks_reached)) { | |||
There was a problem hiding this comment.
I'm not sure if this change is correct, did/does uncrustify complain here?
There was a problem hiding this comment.
Indeed, the previous indentation should be restored.
| @@ -222,6 +224,7 @@ void LandmarkCountHeuristic::generate_preferred_operators( | |||
| assert(successor_generator); | |||
| vector<OperatorID> applicable_operators; | |||
| successor_generator->generate_applicable_ops(state, applicable_operators); | |||
| vector<OperatorID> preferred_operators_conjunctive; | |||
There was a problem hiding this comment.
When we remove the hierarchy, we can remove all three vector and just directly call set_preferred(operators[op_id])
| for (OperatorID op_id : preferred_operators_simple) { | ||
| set_preferred(operators[op_id]); | ||
| } | ||
| for (OperatorID op_id : preferred_operators_disjunctive) { | ||
| set_preferred(operators[op_id]); | ||
| } | ||
| } |
There was a problem hiding this comment.
If we pursue the variant without hierarchy (which I hope we can), then this two-stage approach is no longer necessary; preferred_operators_{conjunctive,simple,disjunctive}) should no longer exist, and set_preferred should instead be called directly.
But this is enough with a code change, together with the other changes that we should make to remove temporary options etc., that I think the experiments should be run again on the version we would like to merge.
# Conflicts: # src/search/landmarks/landmark_count_heuristic.cc # src/search/landmarks/landmark_graph.cc
|
Closing this because it is superseeded by #210. All relevant changes here have been transferred. |
No description provided.