Merged.

All looks good. Thanks a lot for the clean-up!

The experiment has completed and the translator memory usage stays the same while 
the times mostly decrease a little bit. Results are attached. 

Here is the diff: http://codereview.appspot.com/6223064/

Of course it does :)

Am 22.05.2012 15:23, schrieb Malte Helmert:
> Does it verify that the expected result is produced?

> The world just got a little better ;) I added a py.test test for this.

Does it verify that the expected result is produced?
IIRC, the test was of the kind "Let it run and let the human observer judge that
the output is indeed as it should be."

dummy text

ok, let's keep it.
>
> unary_actions was used by a one-off top-level script that apparently we deleted
> at some point. (It was used for some experiments, I forgot on what.) I think we
> can remove it.
>
> untyped_strips_preconditions and parse_typed should go.
all gone.
>
> popped_elements is meant for statistics and/or memory debugging. I'm not sure if
> we should remove it; it completes the interface of the class.
I fixed a bug in it (queue.queue -> self.queue) and will keep it.
>
> test_normalization is a test function whose use is currently commented out. It
> is useful for debugging this module and should stay. (In a better world, it
> would be a proper unit test -- maybe that can be added as a TODO.)
The world just got a little better ;) I added a py.test test for this.

The pretty printer was meant to be used for debugging output. I found it useful
to have around when I implemented the parser. I don't have strong objections to
removing it, but it might still be useful for testing.

unary_actions was used by a one-off top-level script that apparently we deleted
at some point. (It was used for some experiments, I forgot on what.) I think we
can remove it.

untyped_strips_preconditions and parse_typed should go.

popped_elements is meant for statistics and/or memory debugging. I'm not sure if
we should remove it; it completes the interface of the class.

test_normalization is a test function whose use is currently commented out. It
is useful for debugging this module and should stay. (In a better world, it
would be a proper unit test -- maybe that can be added as a TODO.)

You're right. Vulture is a static code analyzer, so dynamic attribute 
access is not discovered. The underscored functions are indeed required.

The other functions are not needed though and the pddl/pretty_print.py 
module is altogether unused. Shall I remove those functions and the file?

> I have written a simple tool that finds dead python code, do you want me to
> remove any of the unused functions it found?

I had a short look at the code, and I doubt these are all unused. Please try out
if the translator still works with them removed (in particular the _underscored
ones).

After having merged with default again (and therefore having a deterministic translator) these changes do not alter the SAS+ output 
(all first instances have the same files). Another search experiment is therefore unnecessary, but I will run a new translator 
experiment.

I have written a simple tool that finds dead python code, do you want me to remove any of the unused functions it found?

$ vulture .
build_model.py:300: Unused function 'popped_elements'
pddl/actions.py:76: Unused function 'unary_actions'
pddl/actions.py:111: Unused function 'untyped_strips_preconditions'
pddl/conditions.py:168: Unused function '_simplified'
pddl/conditions.py:252: Unused function '_untyped'
pddl/conditions.py:336: Unused function '_relaxed'
pddl/functions.py:13: Unused function 'parse_typed'
pddl/pretty_print.py:25: Unused function 'print_nested_list'
pddl_to_prolog.py:171: Unused function 'test_normalization'

Hmm, the LAMA coverage results are not good. In three domains we go from
"perfect" (all problems solved) to "not perfect any more"; this is a warning
sign. I'd like to know more about this first. I'm surprised that this can have
an impact in a domain like Satellite at all. Can you investigate what goes on
with the translator in those domains where we lose perfect coverage, and
specifically in Satellite? Is it due to a qualitative representation change, or
does the translator take more time now? (But I guess this is not measured by the
experiment?)

I'd be inclined not to merge the change if the performance difference cannot be
explained.

BTW: The one validate_error stems from a killed validator run. I think I fixed 
this in the scripts.

The experiments are finished. I ran the configs lama and yY. Do you think we can remove our permutations function?

http://www.informatik.uni-freiburg.de/~downward/exp-js-278-eval-abs-d.html (lama)
http://www.informatik.uni-freiburg.de/~downward/exp-js-278-eval-abs-p.html (lama)

http://www.informatik.uni-freiburg.de/~downward/exp-js-278-yY-eval-abs-d.html (yY)
http://www.informatik.uni-freiburg.de/~downward/exp-js-278-yY-eval-abs-p.html (yY)

I already removed some dead code and unnecessary imports in 
bitbucket/digitaldump/downward branch issue278 and removed our implementation of 
product since we're already depending on python 2.6 anyway. Currently I'm testing 
the impact of using python's built-in itertools.permutations.