Issue720

Title Move task_utils to separate dir and create separate plugins and namespaces
Priority wish Status resolved
Superseder Nosy List danielk, florian, jendrik, malte
Assigned To danielk Keywords
Optional summary

Created on 2017-04-26.14:22:41 by danielk, last changed by jendrik.

Messages
msg6285 (view) Author: jendrik Date: 2017-04-28.21:18:45
Merged and pushed. Thanks, Daniel!
msg6283 (view) Author: danielk Date: 2017-04-28.21:04:12
I have merged default. The pipeline tests passed as well as the dependencies test.
msg6277 (view) Author: malte Date: 2017-04-28.18:47:44
I had a brief look, and it looks good to me, too. If it passes all the tests, I
think it's fine to merge it.
msg6256 (view) Author: jendrik Date: 2017-04-27.22:56:28
I reviewed the individual commits. Apart from the last round of minor comments, 
this looks mergeable to me :-) Should somebody else have a look, Malte?
msg6245 (view) Author: danielk Date: 2017-04-27.17:18:34
The pull request is now ready to be reviewed.
msg6235 (view) Author: malte Date: 2017-04-26.19:21:47
I'm generally not a fan of retyping the namespaces all the time in
implementation files. (Header files are a different story because they inject
dependencies etc.)
msg6234 (view) Author: jendrik Date: 2017-04-26.19:20:27
Link to pull request: 
https://bitbucket.org/danielkillenberger/fast-downward/pull-requests/5

I added some minor comments. Maybe we should add "using namespace 
domain_transition_graph" to cea_heuristic.cc and cg_heuristic.cc to avoid 
repeating the namespace so often. We usually explicitly type the namespaces, but 
maybe we should make an exception here since the files are tightly coupled. What 
do the others think about this?
msg6226 (view) Author: jendrik Date: 2017-04-26.14:30:15
We need to keep in mind Gabi's and Malte's comments:

> The domain transition graph implementation has been designed to meet the
> specific needs of the causal graph and context-enhanced additive
> heuristic and should in my opinion not be used for general purposes.
> So it should rather be moved to the heuristics directory.

That's true. It would be good to add a comment to
domain_transition_graph.h that explains that (at least in its current
form) the DTG implementation is only useful for CG and CEA.

I guess in terms of plug-ins, this means we need a separate
(dependency-only) plug-in for domain_transition_graph and make CG and
CEA depend on it.
msg6224 (view) Author: danielk Date: 2017-04-26.14:22:41
Move the following files from search/ to search/task_utils/ and give them plugins and 
namespaces:
     causal_graph
     domain_transition_graph
     sampling
     successor_generator
     task_tools -> task_properties
     variable_order_finder
History
Date User Action Args
2017-04-28 21:18:45jendriksetstatus: reviewing -> resolved
messages: + msg6285
2017-04-28 21:04:12danielksetmessages: + msg6283
2017-04-28 18:47:44maltesetmessages: + msg6277
2017-04-27 22:56:28jendriksetmessages: + msg6256
2017-04-27 17:18:34danielksetstatus: in-progress -> reviewing
messages: + msg6245
2017-04-26 19:21:47maltesetmessages: + msg6235
2017-04-26 19:20:27jendriksetmessages: + msg6234
2017-04-26 14:30:15jendriksetmessages: + msg6226
2017-04-26 14:22:41danielkcreate