Everybody was happy with the latest iteration of the patch on bitbucket, so I 
merged and pushed the branch.

Hi everyone, as discussed offline, I'll stay off the bitbucket discussion for
the time being, but if you want, I can have a look at a later iteration of the
code (if there will be one).

I agree with all comments by Silvan: use the timer for hill-climbing only, and
for the setting of 0 don't run hill-climbing at all, but use the initial
patterns. I also like the suggested help text.

I think hill-climbing is not the only thing that can take quite a bit of time,
so I think it would also make sense to later check what the total preprocessing
time is for certain values of the hill-climbing timneout. Then we know if
there's anything time-critical in the post-processing phase. (My recollection
from the PhO paper was that this *can* be time-critical, but I'm not sure if
this happened with iPDB or only with the systematic patterns.)

I copy excerpts of my comment from the pull request at bitbucket in order to
have it also in the discussion here:

I think we should clarify what exactly we want to limit here: is it the time the
whole pattern generation method is allowed? This is what is currently done with
the timer instancer of the class, and this is not what I would expect from the
description of this option, which says "time limit for finding patterns".
Otherwise, if the time limit is meant to affect the hill-climbing procedure,
then we should only compare against the local timer of the hill-climbing method.

I would strongly vote in favor of the second variant: compare against the
hill-climbing timer. I don't think that we would ever want to limit the total
time of the ipdb approach, because we want to at least have all the pdbs for the
goal variables. Then, setting the max_time to 0 would also make sense, because
we would perform no hill-climbing at all and end up with the goal variable pdbs.
Otherwise, setting the parameter to 0 would imply that no abstraction heuristic
would be built at all. Furthermore, we currently do not compare against the
timer during the computation of the initial pattern collection, so we always
build the pdbs for all goal variables (which is good!).

If we settle for this alternative, I would also change the description for this
option to "maximum time in seconds for improving the initial pattern collection
via hill climbing. If set to 0, no hill climbing is performed at all". We could
then also *not* call hill_climbing at all if the max_time parameter is set to 0.

Now that issue410 and issue411 have been merged, you should definitely integrate
the newest default branch into this issue's branch.

Also the problem mentioned in msg2905 should be taken care of; issue411 split up
the method hill_climbing in several more manageable chunks in order to avoid
such a problem from happening again.

(There will though be a lot of conflicts when merging, I suppose, sorry about that.)

I am not done implementing your comments. Will do so when time allows.

What is the status here? Is this waiting for my review? Or is this waiting for
other things to be merged first?

The problem mentioned in issue411 actually only occurs when using this branch.

The problem is caused by moving

const vector<int> &best_pattern = candidate_pdbs[best_pdb_index]->get_pattern();
if (improvement >= min_improvement) {
    // add the best pattern to the CanonicalPDBsHeuristic
    cout << "found a better pattern with improvement " << improvement << endl;
    cout << "pattern: " << best_pattern << endl;
    current_heuristic->add_pattern(best_pattern);
}

before the check for ending the hill climbing search. I fixed this by moving it
*after* the check again (I anyway do not see any reason why having this piece of
code before the termination check).

I added a few comments there.

I opened a pull request in the IPC14 repo and hope that you have access there, 
Malte.

I have pushed a branch with the simple timeout implementation.

I have not yet implemented this, so please go ahead.

Added Gabi to the nosy list (see msg2853).

As discussed offline ipdb needs a "max_time" parameter. Gabi, have you already 
implemented this? If not, I will apply a simple patch from Florian in the 
respective issue branch. It should be enough for the IPC at least.