This is part of issue727.

While the original scope of this issue was broader, the new goal is to use only 
"Evaluator" and not "Heuristic" in the option parser code. This means that all 
current "Heuristic" plugins need to return "Evaluator" pointers. Before we 
tackle 
this issue, we need to make sure that all public methods of the "Heuristic" 
class 
are also part of the "Evaluator" class. issue724 deals with the "notify_*" 
methods and the "get_involved_heuristics()" method. The last remaining public 
method that needs to be promoted to the "Evaluator" class is 
"get_description()".

This has been merged.

Looks good, please merge!

Once this is pushed, we should check that the documentation looks alright. I'm
not sure if the Doc/Heuristic wiki page will be deleted automatically, but I
think it will. We also need to change the existing links to Doc/Heuristic from
the manual wiki pages, but Salome is already working on that.

Here is e new PR for reviewing
https://bitbucket.org/cgeissmann/downward/pull-requests/7/issue718/diff

Update summary.

Just a quick note: issue724 is merged into master, we can have a look together at 
this one tomorrow, Manuel, and see if I can help with it.

This is currently at the top of the review queue, but we cannot continue with it
at the moment because it is blocked. I looked at issue750 instead and will
remove this from the review queue for now.

Update summary.

I'm happy with the pull request now :-)

Once both of you are happy with the pull request, please let me know and I'll
also have a final look.

Sounds good to me. Maybe it would be good to factor out the common casting code.

For EnforcedHillClimbingSearch there is no reason to do the casts.

For greedy and lazy searches, we have to cast the preferred operator heuristics
when they are collected to a set of heuristics. The set of heuristics consist of
pointers to heuristics because they provide the notify functions. During our
live discussions, we decided to leave the notify functions in Heuristic (not to
move them to Evaluator) and to change the subscription/notification mechanism
with issue724.

I will move the casts to the lines where they are required and change the type
of variables where possible, ok?

I added some comments on Bitbucket.

Why do we need all the dynamic_casts? Is it just to avoid changing the types of 
variables? I think that removing the casts and changing the types would yield 
clearer code.

I created a new branch issue718-backup out of branch issue718. Afterwards, I
closed branch issue718 and I created a new branch with the same name out of
default. To that new branch, I imported patches that contain the meaningful
changes from the old issue718 branch.

In the new branch the changes affect options only. The type of member variables
are not changed from Heuristic to Evaluator. Also the functions are not moved
from Heuristic to Evaluator. Instead, I chose to cast Evaluator to Heuristics if
necessary and call an InputError if the Evaluator that is passed through options
is not a Heuristic. For example the InputError is called if someone puts an
Evaluator as preferred operator heuristic because an Evaluator does not provide
preferred operators. 

After our live discussions I decided for these changes to the code, because they
seemed most minimalistic and safe to me.

A new pull request can be found here:
https://bitbucket.org/manuel_h/downward/pull-requests/4/issue718/diff

Here it is: https://bitbucket.org/manuel_h/downward/pull-requests/3/issue718/diff

Can you add a link to the pull request?

Regarding the five items, I suggest we discuss them live tomorrow.

Regarding 1), we should probably discuss with the others what we want the
command-line option and the wiki page to be called, and we should discuss the
question of whether to add sections or some other way of structuring wiki pages
with lots of entries, which we will get after merging the ScalarEvaluator and
Heuristic pages.

Regarding 2), I think we should look at where and how these descriptions are
used first. I think we currently have two kinds of descriptions; the shorter
ones that are used as "tags" in statistics and are created automatically from
the option parser, and longer ones that are used to describe to the user what is
going on. For the shorter ones, I think we can do better than we currently do
when we improve the option parser, and with the longer ones, we should probably
come up with a clearer plan of what we want to do with them. At the moment I
think it's a good idea to have proper descriptions for everything, but it would
be good to think about this feature more generally.

Regarding 4), forgetting to override this function in cases where it must be
overridden will lead to serious bugs, and in such cases our usual policy is not
to provide a default implementation because then people implementing derived
classes often don't think about whether they have to provide an implementation.
But, looking at its documentation, I think this function should probably go away
anyway in the medium term.

I put a pull request on bitbucket. Up to now, I did the minimal changes to
eliminate Heuristic from plugins. During the process of this change I stumbled
over following issues:

1. Change predefinition option --heuristic to --evaluator.

2. Enable description for all evaluators, not only for heuristics. The
description for FFHeuristic is "ff", which is the plugin key. At the moment
Evaluator provides the function get_description() (which was formerly a function
of Heuristic only). It returns "n.a." for classes that do not inherit from
Heuristic.

3. Get rid of dynamic_cast for checking if Evaluator is a Heuristic. The cast
ensures that caching and counting evaluations is done for Heuristic only. I
suggest to add following functions to Evaluator: bool
cache_evaluation_results(), bool count_evaluations().  

4. Classes that inherit from Evaluator are forced to implement the virtual
function get_involved_heuristics(). Consequently, each evaluator must define it.
I suggest to define the function in Evaluator with an empty body ({}). Then, the
classes are free to redefine the function.

5. ConstEvaluator can inherit from Evaluator, because the requirement that a
search needs at least one Heuristic does not hold anymore.

I am not sure if we want to cover some of these issues now, or add them to the
issue tracker.

I would describe it a bit differently: we want all "user code" to use the class
Evaluator, but the class Heuristic will still exist to be used as a common base
class of specific heuristics like FFHeuristic etc. It will keep responsibilities
such as keeping track of the heurisitic cache, for example.

After having renamed ScalarEvaluator to Evaluator in the process of merging
Heuristic and ScalarEvaluator to a new class (issue716), we merge Heuristic into
Evaluator. Eventually, the class Evaluator replaces the classes ScalarEvaluator
and Heuristic.