This issue is part of issue64 and handles all modules that are more or less
decided already. For those, we plan to use the following file structure.
Namespace names are in parentheses and are marked with a star, if the CMake
module is only used as a dependency.

evaluators/
    const_evaluator.cc (ConstEvaluator)
    g_evaluator.cc (GEvaluator)
    combining_evaluator.cc (CombiningEvaluator)
    max_evaluator.cc (MaxEvaluator)
    pref_evaluator.cc (PrefEvaluator)
    weighted_evaluator.cc (WeightedEvaluator)
    sum_evaluator.cc (SumEvaluator)

heuristics/
    relaxation_heuristic.cc (RelaxationHeuristic*)
    ipc_max_heuristic.cc (IPCMaxHeuristic)
    additive_heuristic.cc (AdditiveHeuristic)
    blind_search_heuristic.cc (BlindSearchHeuristic)
    cea_heuristic.cc (CEAHeuristic)
    cg_heuristic.cc (CGHeuristic)
    cg_cache.cc  (CGHeuristic)
    ff_heuristic.cc (FFHeuristic)
    goal_count_heuristic.cc (GoalCountHeuristic)
    hm_heuristic.cc (HMHeuristic)
    lm_cut_heuristic.cc (LMcutHeuristic)
    lm_cut_landmarks.cc (LMcutHeuristic)
    max_heuristic.cc (MaxHeuristic)

lp/ (LP*)
    lp_internals.cc
    lp_solver.cc

merge_and_shrink/ (MergeAndShrink)
    distances.cc
    factored_transition_system.cc
    fts_factory.cc
    heuristic_representation.cc
    label_equivalence_relation.cc
    labels.cc
    merge_and_shrink_heuristic.cc
    merge_dfp.cc
    merge_linear.cc
    merge_strategy.cc
    shrink_bisimulation.cc
    shrink_bucket_based.cc
    shrink_fh.cc
    shrink_random.cc
    shrink_strategy.cc
    transition_system.cc

landmarks/ (Landmarks)
    exploration.cc
    h_m_landmarks.cc
    lama_ff_synergy.cc
    landmark_cost_assignment.cc
    landmark_count_heuristic.cc
    landmark_factory.cc
    landmark_factory_rpg_exhaust.cc
    landmark_factory_rpg_sasp.cc
    landmark_factory_zhu_givan.cc
    landmark_graph.cc
    landmark_graph_merged.cc
    landmark_status_manager.cc
    util.cc

operator_counting/ (OperatorCounting)
    constraint_generator.cc
    lm_cut_constraints.cc
    operator_counting_heuristic.cc
    pho_constraints.cc
    state_equation_constraints.cc

pdbs/ (PDBs)
    canonical_pdbs_heuristic.cc
    dominance_pruner.cc
    match_tree.cc
    max_cliques.cc
    pattern_database.cc
    pattern_generation_edelkamp.cc
    pattern_generation_haslum.cc
    pattern_generation_systematic.cc
    pdb_heuristic.cc
    util.cc
    zero_one_pdbs_heuristic.cc

potentials/ (Potentials)
    diverse_potential_heuristics.cc
    potential_function.cc
    potential_heuristic.cc
    potential_max_heuristic.cc
    potential_optimizer.cc
    sample_based_potential_heuristics.cc
    single_potential_heuristics.cc
    util.cc

Thanks, I merged and pushed.

Strangely, I had to run uncrustify and change some lines of code
(http://hg.fast-downward.org/rev/8a179ce10e02). This looks like what you wrote
in your email, Malte. I'm surprised that the buildbot still complained about
this. After all, the code was already merged before merging this issue.

No need for expriments if we just change the filenames and namespaces.

I merged in the changes from issue481 and issue585 and updated the pull request: 

https://bitbucket.org/flogo/downward-issues/pull-requests/5

Should we run experiments for this, or can we merge it without?

Sure. I would like to merge this before our next sprint, if possible, but apart
from that, I don't think this is urgent.

Hmmm, can you hold off on merging this for a week until issue481 is merged? The
changes to the search algorithms will cause merge conflicts, and I think it may
be easier to first merge issue481 and then this one because this one is much
less hairy in terms of understanding that we don't mess anything up.

I can promise to be done with the "necessary" parts of issue481 and start an
experiment by Sunday. (Or if I don't, I'll swallow the pill and deal with the
merge conflicts.)

Ah, sorry for the misunderstanding.

Perhaps we can count this as another argument for calling the heuristic classes
"Heuristic" instead of "XyzHeuristic", but if we want to make this a convention,
we would need to discuss it with the others.

Proposition is not a nested class, the compiler just thinks it is, because it
interprets the name of the namespace as the name of the (base) class.

I have no strong opinion, but for what it's worth, perhaps Proposition doesn't
need to defined within the class to start with. The main reason why we had it
inside the class was to avoid polluting the global namespace, which isn't an
issue any more with the new namespaces.

I always find nested classes a bit awkward because of all the typing etc. (see
python -c "import this" | sed "7q;d")

One thing that came up in the review was whether we want to name namespaces
identical to classes. There are recommendations against this in other languages
(Java, C#), but I don't know if that’s a problem in C++.

In one case this made a difference in our code:

  class AdditiveHeuristic : public RelaxationHeuristic::RelaxationHeuristic {
     RelaxationHeuristic::Proposition p;
  }

This interprets RelaxationHeuristic as the class
RelaxationHeuristic::RelaxationHeuristic and complains about the missing
definition of
RelaxationHeuristic::RelaxationHeuristic::Proposition instead of using the
definition from the namespace. I fixed this by using a typedef:

  using Proposition = RelaxationHeuristic::Proposition;
  class AdditiveHeuristic : public RelaxationHeuristic::RelaxationHeuristic {
     Proposition p;
  }

This solution seems fine to me, I just wanted to ask, if you have a different
opinion.

I'm done with my review. Except for some minor things, everything looks good.

I'll review the code.

I started a pull request for this. Can one of you do a review?

https://bitbucket.org/flogo/downward-issues/pull-requests/5
(The pull request is against a dummy branch where I only did the file movements,
so the diff for moved files shows up nicer.)

Excluded open lists, which will be moved to a namespace as part of issue481.

We want to introduce namespaces, CMake modules and directories for potentials,
pdbs, operator counting, landmarks, merge and shrink, the lp solver, individual
heuristics, individual evaluators, and open lists.