All done. I removed the last few references to the option caveat page from the
automatic documentation in the default branch.

I handled Malte's additional comment and a few added by Jendrik afterwards.
Looks like all comments are handled now. I'll merge and remove the infamous
option caveat.

Strangely, you sometimes have to update the pull request manually by clicking the 
bubble that sometimes appears on the top right of the pull request.

Commits just showed up, left one more comment.

Can do, but they don't seem to be on bitbucket yet.

Done with addressing the comments. Somebody wants to check the three commits?

I'm done with the comments on bitbucket. Looks ready to merge once the comments
are addressed. (I can be swayed regarding the portfolio config.)

The run_dir is included in the default attributes of our Lab issue experiments.
(Sorry for being late to save time..)

D'oh! run_dir is included. I think I missed it because I looked at the summary
rather than the table of contents. Could have saved me a ton of time...

Thanks, the path was exactly right! I'll have a closer look at home.

BTW, it would help a lot if the "run_dir" attribute were included in the table.
I think this is a very useful attribute in general, and I would encourage
everyone to include it in all their reports. Have already used it many times in
this sprint.

The experiment should be under
/infai/pommeren/projects/downward/issues/issue776/experiments/issue776/data/issue776-v2

(This is from memory without grid access, so if it's not there I probably made
an obvious mistake. It should all be readable so you should be able to find it
from there.)

I think LAMA's behaviour is known to be non-stable, so the differences are not
necessarily a sign of a problem. Can you tell us where we can find the run.log
files for this? They may be useful for deciding whether things work as expected.

And here is the one for lama (didn't have a chance to look at it yet):
https://ai.dmi.unibas.ch/_tmp_files/pommeren/issue776-v2-lama-issue776-v2-base-issue776-v2-compare.html

Reports for bjolp and lama-first are done:
https://ai.dmi.unibas.ch/_tmp_files/pommeren/issue776-v2-issue776-v2-base-issue776-v2-compare.html

bjolp looks fine but lama-first has a few changes in the number of evaluations.
I won't have time to look into this tonight.

After a longer discussion among Malte, Florian, and myself, we think that we can
remove the lm_cost_type option of LandmarkCountHeuristic. By doing so, the
problem with not knowing which task to use for a cost transformation vanishes,
because the landmark heuristic will use the same (potentially cost adapted) task
to pass to the landmark factory and for everything else it does. Florian started
experiments to verify this.

http://ai.cs.unibas.ch/_tmp_files/sieverss/issue776-v1-lama-second-issue776-base-issue776-v1-compare.html

You are right, the documentation on the old options is correct but doesn't help
us here. I'm not sure how to proceed with this issue, since the patch does not
fix anything if using task transformations with landmark heuristics "is not
allowed" in the first place.

PS: The experiment only needs to look at the domains that are not unit cost. In
particular, we don't need any of the before-2008 domains.

> Costs change in elevators and nomystery (3 tasks). Regarding our discussion
> whether the current code would perform a cost adaptation twice if using plusone
> for both the heuristic and the landmark factory, I think this needs further
> investigation.

The anytime behaviour of lama inteferes with this experiment, so I suggest
running a "lama-second" configuration, i.e., extracting the second configuration
used by LAMA 2011 in the non-unit-cost case. Something like:

[
"--heuristic",
"hlm2=lama_synergy(lm_rhw(reasonable_orders=true,lm_cost_type=plusone),transform=adapt_costs(plusone))",
"--heuristic",
"hff2=ff_synergy(hlm2)",
"--search",
"lazy_greedy([hff2,hlm2],preferred=[hff2,hlm2],reopen_closed=false)"
]

> From the code it looks like it could be the case, but the
> documentation at http://www.fast-downward.org/OptionCaveats reads
> that only one of the cost options is used in any case.

Not sure I understand. We saw that the old code ignored the "task" argument
here, so indeed only one of the options was used in the old code. But we change
the code here, so the documentation for the old code can't tell us how the new
code behaves.

It's not urgent, I already integrated the fix in my code.

Here are the requested results:
http://ai.cs.unibas.ch/_tmp_files/sieverss/issue776-v1-issue776-base-issue776-v1-compare.html

Costs change in elevators and nomystery (3 tasks). Regarding our discussion
whether the current code would perform a cost adaptation twice if using plusone
for both the heuristic and the landmark factory, I think this needs further
investigation. From the code it looks like it could be the case, but the
documentation at http://www.fast-downward.org/OptionCaveats reads that only one
of the cost options is used in any case.

If the patch solves Silvan's problem, I'm fine with merging it, although I would
like to see some experiments, e.g. with lama-first, lama and seq-opt-bjolp. 

However, the new code still looks broken to me in the way it attempts/pretends
to handle task transformations. This needs further discussion, and probably a
further (bug) issue.

Good catch! I left some comments on Bitbucket but am happy with the general fix.

The patch looks fine to me. Using g_root_task is left over from our change to
the task interface, it shouldn't be hard-coded. After the patch, it is still
hard-coded on a higher level. This should also eventually go away (as part of
issue509) but it will take some more work to do that.

The plugin adapt_cost now creates a cost-adapted task of the root task. I
documented this, but I'm not sure if this is the right intention. I saw that at
the transform option of Heuristic, we say that there are currently two
transformations: no_transform and adapt_costs. However, we have many more tasks,
but none of them supports being created via plugin. This issue probably has to
do with many of our past discussions on where and how to create shared pointer
objects and how to pass them around.

The current solution works fine in my eyes and I'd be very happy to have a quick
review, which should take only a few minutes, because this solves my problem of
computing suboptimal solutions with the landmark heuristic in my IPC submissions.

https://bitbucket.org/SilvanS/fd-dev/pull-requests/34/issue776/diff

There is a bug in CostAdaptedTask: it passed g_root_task to its parent class
DelegatingTask, which is a very nasty bug.