[1072] make conjunctive landmarks usable for preferred operators#149
[1072] make conjunctive landmarks usable for preferred operators#149SimonDold wants to merge 8 commits intoaibasel:mainfrom
Conversation
| @@ -0,0 +1,107 @@ | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
Experiments should be removed before merging.
| 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.
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Remove those two newlines. Maybe (one of) the reason why the style checks fail?
| 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.
| 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_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. |
4 participants
No description provided.