Issue1130

[ClemensBuechner]

No description provided.

[jendrikseipp]

LGTM.

[jendrikseipp]

Suggested change

	"when the option `pass_bound` is set to true in which case the "
	"when the option `pass_bound` is set to true, in which case the "

[jendrikseipp]

Suggested change

	"The definition of our interfaces for search algorithms allow to "
	"Our interfaces for search algorithms allow to "

[maltehelmert]

This construction with "allow to" is apparently a typical English language mistake that native speakers of German do. Chris Beck pointed this out in the context of Florian's thesis.

[maltehelmert]

I guess I'm also susceptible to the mistake, so I'm not 100% sure about the following. But I think the error is that "allow" requires saying who is allowed something, i.e., you need a direct object like "allow the users to ...". The better solution is almost always to use a different phrasing.

[maltehelmert]

"anyways" is not really a world (in formal writing); it's always "anyway".

But also I think this discussion is much longer than I think it needs to be. I would not discuss what is or isn't reasonable and discuss interactions with "pass_bound" at length. I would limit this for a discussion of what the behaviour is, and I think it's enough to make this part of the discussion of "pass_bound" rather than a separate note.

I struggle a bit to formulate this because I think the text for "pass_bound" is already bad. What is the "bound from previous search"? We're not using a bound from a previous search, we use the best solution cost so far as a bound.

I would not add a new note and instead rewrite the documentation of "pass_bound", for example along those lines: "use the cost of the best solution found as a bound for component algorithms, unless these already have a lower bound set".

[ClemensBuechner]

I think it's a good idea to integrate the document note into the documentation of the pass_bound option. I also like the suggested phrasing, except that the passed bound does not have to be based on the best solution so far, it could also just be the bound set for the iterated search itself. So how about this instead:

"use the cost of the best solution found (or bound of iterated search if none found so far) as a bound for component algorithms, unless these already have a lower bound set".

[FlorianPommerening]

How about this as a replacement for the current pass_bound option and instead of the extra document note?

        add_option<bool>(
            "pass_bound",
            "use the real cost of the plan (regardless of the cost_type "
            "parameter) found in one iteration as a bound for the next "
            "iteration. If that search for that iteration already has a "
            "bound, the lower of the two bounds is used.",
            "true");

[FlorianPommerening]

Sorry, my comment overlapped with yours and wasn't meant as a response. Do we want to keep the part about the cost type and real cost?

[ClemensBuechner]

I guess the "real cost (regardless of cost_type)" is relevant information, so combining the suggestions by Florian and me, I ended up with the following draft:

"use the bound of iterated search as a bound for its component search algorithms, unless these already have a lower bound set. The iterated search bound is tightened whenever a component finds a cheaper plan (using real costs regardless of the cost_type parameter)."

What are your opinions?

[FlorianPommerening]

sounds good to me

[maltehelmert]

I prefer Clemens's phrasing over Florian's suggestion because it is more precise without being convoluted. ("found in one iteration as a bound for the next iteration" is not quite right because it is used for all following iterations, not just the next one).

The discussion of cost_type invites the question what cost_type for iterated does then, and the answer is: it does nothing and shouldn't be there in the first place.

It would be good to fix this sooner rather than later. Its existence is a consequence of the currently wrong inheritance hierarchy for search algorithms, where too much stuff is present in the base class.

Personally, I have a mild preference for not including a discussion of cost_type in the bound documentation because I think it doesn't resolve the confusion that people might have about cost_type, and it's not unlikely that once we fix the hierarchy, we will forget to update the documentation of this argument. But if you prefer to include it, I won't object.

[ClemensBuechner]

Florian and I support removing the cost_type discussion now. I have removed the comment in parentheses.
Fixing the inheritance hierarchy should be done in a separate issue, though.