Issue599

Title use namespaces for "easy" modules
Priority wish Status resolved
Superseder Nosy List florian, jendrik, malte
Assigned To florian Keywords
Optional summary
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

Created on 2015-11-20.20:01:05 by florian, last changed by florian.

Summary
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
Messages
msg4880 (view) Author: florian Date: 2015-12-07.03:04:54
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.
msg4879 (view) Author: malte Date: 2015-12-06.20:22:08
No need for expriments if we just change the filenames and namespaces.
msg4878 (view) Author: florian Date: 2015-12-06.20:02:07
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?
msg4835 (view) Author: florian Date: 2015-11-24.19:20:50
Sure. I would like to merge this before our next sprint, if possible, but apart
from that, I don't think this is urgent.
msg4834 (view) Author: malte Date: 2015-11-24.19:14:03
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.)
msg4833 (view) Author: malte Date: 2015-11-24.18:55:56
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.
msg4832 (view) Author: florian Date: 2015-11-24.18:53:14
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.
msg4831 (view) Author: malte Date: 2015-11-24.18:51:06
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")
msg4830 (view) Author: florian Date: 2015-11-24.11:46:24
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.
msg4829 (view) Author: jendrik Date: 2015-11-23.17:46:01
I'm done with my review. Except for some minor things, everything looks good.
msg4828 (view) Author: jendrik Date: 2015-11-23.16:16:10
I'll review the code.
msg4827 (view) Author: florian Date: 2015-11-23.15:59:59
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.)
msg4820 (view) Author: florian Date: 2015-11-20.22:32:32
Excluded open lists, which will be moved to a namespace as part of issue481.
msg4818 (view) Author: florian Date: 2015-11-20.20:01:05
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.
History
Date User Action Args
2015-12-07 03:04:54floriansetstatus: chatting -> resolved
messages: + msg4880
2015-12-06 20:22:08maltesetmessages: + msg4879
2015-12-06 20:02:07floriansetmessages: + msg4878
2015-11-24 19:20:50floriansetmessages: + msg4835
2015-11-24 19:14:03maltesetmessages: + msg4834
2015-11-24 18:55:56maltesetmessages: + msg4833
2015-11-24 18:53:14floriansetmessages: + msg4832
2015-11-24 18:51:06maltesetmessages: + msg4831
2015-11-24 11:46:25floriansetassignedto: florian
messages: + msg4830
2015-11-23 17:46:01jendriksetmessages: + msg4829
2015-11-23 16:16:10jendriksetmessages: + msg4828
2015-11-23 15:59:59floriansetmessages: + msg4827
2015-11-20 22:32:32floriansetstatus: unread -> chatting
messages: + msg4820
summary: This issue is part of issue64 and handles all modules that are more or 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 depedency. open_lists/ (OpenLists*) alternation_open_list.cc bucket_open_list.cc epsilon_greedy_open_list.cc open_list.cc pareto_open_list.cc standard_scalar_open_list.cc tiebreaking_open_list.cc type_based_open_list.cc 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 -> 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
2015-11-20 20:01:05floriancreate