Issue669

Title Move algorithm utilities to their own subdirectory algorithms
Priority feature Status resolved
Superseder Nosy List danielk, florian, jendrik, malte, silvan
Assigned To danielk Keywords
Optional summary
Collect generic algorithm classes in their own subdirectory called
algorithms.
Suggested algorithms to be moved into the mentioned folders are:
- dynamic_bitset
- int_packer
- max_cliques
- priority_queue
- segmented_vector
- equivalence_relation

Created on 2016-09-04.17:38:08 by silvan, last changed by jendrik.

Summary
Collect generic algorithm classes in their own subdirectory called
algorithms.
Suggested algorithms to be moved into the mentioned folders are:
- dynamic_bitset
- int_packer
- max_cliques
- priority_queue
- segmented_vector
- equivalence_relation
Messages
msg6200 (view) Author: jendrik Date: 2017-04-21.10:22:30
Perfect. I merged and pushed the branch. Thanks for your work!
msg6199 (view) Author: danielk Date: 2017-04-21.10:01:37
Yes I have implemented the suggested changes and tested the dependencies.
msg6198 (view) Author: jendrik Date: 2017-04-21.09:49:19
What's the status here? Are you finished with Florian's comments, Daniel? If so, I 
will merge the branch.
msg6194 (view) Author: florian Date: 2017-04-18.14:28:30
Thanks. I left a few comments on Bitbucket but nothing major. I found some of
the old forward declarations but I'm not sure if I got them all. Maybe a search
for "class IntPacker;" would be good (and for the other moved classes as well).
msg6193 (view) Author: danielk Date: 2017-04-13.17:19:37
@Florian
I have prepared the branch as you have suggested: 
https://bitbucket.org/danielkillenberger/fast-
downward/branch/issue669_default_comparison

Note that the changes are backwards as I have replaced the new files in branch 
'issue669' with the original files from 'default' in the algorithms folder. I have 
renamed priority_queue.h to priority_queues.h to make a direct comparison on 
bitbucket possible.
msg6192 (view) Author: florian Date: 2017-04-13.13:11:37
I can have a look after the Easter break.Could you try to make the diff easier
to review with a temporary branch? Bitbucket unfortunately is bad at recognizing
moved files.

One thing that could work is to create a temporary branch based on the same
master revision where the issue branch splits off. In the temporary branch, copy
(with "hg copy") the old files to their new location. In Bitbucket, we would
then look at the diff between the temporary branch and the issue branch. But
that might not work, so maybe do a small trial run with one file first.

If that doesn't work, then you could branch the temporary branch off of the
issue branch tip, copy ("hg copy") the new files to their old locations, commit,
do a regular file copy from the original files from the master over the files in
the repo, and commit again. The diff between the issue branch and the temporary
branch should again show the actual file changes this way.

In both cases, please remember not to push the temporary branch into the main
repo during the merge.
msg6190 (view) Author: jendrik Date: 2017-04-12.15:11:29
Daniel has moved all files listed in the summary and the code looks good to me. 
 Another pair of eyes probably won't hurt though. Silvan or Florian, could you have 
a look at the pull request, please?
msg6188 (view) Author: jendrik Date: 2017-04-07.09:39:42
I like your idea and I think it fits our conventions. We have 
cegar/subtask_generators.h, merge_and_shrink/types.h, pdbs/types.h, 
utils/collections.hm globals.h, options/registries.h, options/predefinitions.h, 
options/errors.h, etc.
msg6187 (view) Author: malte Date: 2017-04-07.00:53:49
Would calling the namespace and file "priority_queues" (plural) be consistent
with our conventions? Unlike other source files like dynamic_bitset, it contains
multiple related priority queue classes if I recall correctly, so it might fit.
msg6186 (view) Author: jendrik Date: 2017-04-06.22:20:08
We are putting each of the algorithms into its own CMake plugin and namespace. For 
priority_queue.h this is not straightforward since the "priority_queue" name 
collides with std::priority_queue and we usually do "using namespace std;". Do you 
have a good idea how to solve this?
msg6184 (view) Author: malte Date: 2017-03-29.19:11:16
EquivalenceRelation fits the bill.

BTW, looking at the code for EquivalenceRelation, the header file contains an
unrelated and unused structure DoubleEpsilonEquality, which should be removed.

(Addendum: I've just removed the dead code.)
msg6183 (view) Author: jendrik Date: 2017-03-29.15:48:32
@Malte: what about EquivalenceRelation?
msg6182 (view) Author: danielk Date: 2017-03-29.15:41:34
@silvan Thanks!
I think Jendrik was able to give access to the entire AI Group.
I set the status of the issue back to 'in-progress' as only a part of the issue has 
been resolved.

Collected the suggested algorithms that are to be stored in the algorithms/ dir in the 
issue summary.
msg6181 (view) Author: malte Date: 2017-03-29.12:11:32
Yes, the dynamic bitset would also fit well.
msg6180 (view) Author: jendrik Date: 2017-03-29.12:10:36
Should we also move "utils/dynamic_bitset.h" into the "algorithms" directory?

The aibasel group should now have access to Daniel's repo.
msg6179 (view) Author: malte Date: 2017-03-29.11:01:37
I think priority_queue.h would fit well into the algorithms directory.
I'd also suggest segmented_vector.h.

Can you collect these suggestions, including the earlier ones, in the "optional
summary" field? Then we won't need to scan over the old comments later on.
msg6178 (view) Author: jendrik Date: 2017-03-29.09:48:47
@Daniel: It's probably easiest if you make me an administrator. Then I can give 
access to the aibasel group.

@Silvan: We're trying not to do too many steps at once for the first issue. Once 
everybody is happy with the changes to IntPacker, we'll move other algorithms.

@All: Are there other algorithms we should move to the "algorithms" directory? 
Maybe priority_queue.h?
msg6177 (view) Author: silvan Date: 2017-03-29.09:39:45
Welcome on the issue tracker, Daniel!

I cannot access your repository. Can you give access to SilvanS (and/or others
on this issue, e.g. jendrikseipp or florianpommerening)? Furthermore, the change
you suggest would only be a part of this issue, i.e. if you wanted to apply a
patch for this issue, it should contain all the changes mentioned in the comment
below (msg5591).
msg6176 (view) Author: danielk Date: 2017-03-29.09:29:02
Move intPacker to algorithms/ and give it its own namespace

https://bitbucket.org/danielkillenberger/fast-downward/pull-requests/2/issue669/diff
msg5591 (view) Author: silvan Date: 2016-09-04.17:38:08
In issue667, we will add a class to compute maximal strongly connected
components of a graph, given in a generic "list of neighbors" form. We would
like to collect such generic algorithm classes in their own subdirectory called
algorithms. For the moment, at least the maximal cliques computation from the
pdbs subdir and the Intpacker class could also belong to that set of algorithm
classes.
History
Date User Action Args
2017-04-21 10:22:30jendriksetstatus: reviewing -> resolved
messages: + msg6200
2017-04-21 10:01:37danielksetmessages: + msg6199
2017-04-21 09:49:19jendriksetmessages: + msg6198
2017-04-18 14:28:30floriansetmessages: + msg6194
2017-04-13 17:19:37danielksetmessages: + msg6193
2017-04-13 15:34:26danielksetstatus: in-progress -> reviewing
2017-04-13 13:11:37floriansetmessages: + msg6192
2017-04-12 15:11:29jendriksetmessages: + msg6190
2017-04-07 09:39:42jendriksetmessages: + msg6188
2017-04-07 00:53:49maltesetmessages: + msg6187
2017-04-06 22:20:09jendriksetmessages: + msg6186
2017-03-29 19:11:34maltesetsummary: Collect generic algorithm classes in their own subdirectory called algorithms. Suggested algorithms to be moved into the mentioned folders are: - dynamic_bitset - int_packer - max_cliques - priority_queue - segmented_vector -> Collect generic algorithm classes in their own subdirectory called algorithms. Suggested algorithms to be moved into the mentioned folders are: - dynamic_bitset - int_packer - max_cliques - priority_queue - segmented_vector - equivalence_relation
2017-03-29 19:11:16maltesetmessages: + msg6184
2017-03-29 15:48:32jendriksetmessages: + msg6183
2017-03-29 15:41:34danielksetstatus: reviewing -> in-progress
messages: + msg6182
summary: Collect generic algorithm classes in their own subdirectory called algorithms. Suggested algorithms to be moved into the mentioned folders are: - dynamic_bitset - int_packer - max_cliques - priority_queue - segmented_vector
2017-03-29 12:11:32maltesetmessages: + msg6181
2017-03-29 12:10:36jendriksetmessages: + msg6180
2017-03-29 11:01:37maltesetmessages: + msg6179
2017-03-29 09:48:47jendriksetmessages: + msg6178
2017-03-29 09:39:45silvansetmessages: + msg6177
2017-03-29 09:29:03danielksetstatus: chatting -> reviewing
assignedto: danielk
messages: + msg6176
nosy: + danielk
2016-09-04 19:35:43jendriksetnosy: + jendrik
2016-09-04 17:38:08silvancreate