Issue414

Title clean up overall architecture of planner runs
Priority feature Status resolved
Superseder Nosy List gabi, jendrik, malte, silvan
Assigned To jendrik Keywords
Optional summary

Created on 2014-02-06.21:06:42 by malte, last changed by malte.

Messages
msg3867 (view) Author: malte Date: 2014-10-23.17:20:26
Thanks! Keywords added.
msg3866 (view) Author: jendrik Date: 2014-10-23.16:53:35
I added issue493 for the time slice issue on Windows. The "driver" keyword is a 
good idea, maybe we should also add "windows" and "osx" keywords for people 
interested in support for these platforms?

I just sent the announcement to the mailing list, so I'll close this issue now.
msg3861 (view) Author: malte Date: 2014-10-23.12:26:39
Great! :-) So the last outstanding point is the email to the mailing list? We
should also fix the portfolio time slices on Windows soon, but let's make that a
separate issue. Jendrik, shall I send that email, or would you rather do it? If
there are no objections, I'd also like to add a new "driver" keyword to the
tracker, similarly to the "translator" keyword we already have. I already have a
sizable list of RFEs for the driver...
msg3858 (view) Author: jendrik Date: 2014-10-22.22:10:04
I have adapted the wiki documentation to the new driver script.
msg3845 (view) Author: malte Date: 2014-10-17.14:33:48
Great! Who is "we" in this case? ;-)
msg3844 (view) Author: jendrik Date: 2014-10-17.14:32:02
I merged and pushed branch. We still need to update the instructions in the wiki 
and send a mail to the FD mailing list.
msg3843 (view) Author: malte Date: 2014-10-17.11:06:27
Yes, it looks ready to merge.
msg3842 (view) Author: jendrik Date: 2014-10-17.11:06:03
Should I go ahead and merge this?
msg3837 (view) Author: malte Date: 2014-10-16.17:22:09
Looks good! :-)

I don't see a good reason why we would need to test this with a longer timeout.
msg3836 (view) Author: jendrik Date: 2014-10-16.16:42:33
I ran satisificing and optimal experiments with a 60s timeout. The results are at

http://ai.cs.unibas.ch/_tmp_files/seipp/issue414-base-issue414-compare-sat.html
http://ai.cs.unibas.ch/_tmp_files/seipp/issue414-base-issue414-compare-opt.html

Do you think we need experiments with a longer timeout?
msg3823 (view) Author: malte Date: 2014-10-14.14:31:42
Sounds good! I only added one more comment (which doesn't need another experiment).
msg3822 (view) Author: jendrik Date: 2014-10-14.13:37:21
I made your suggested changes and will run experiments now.
msg3813 (view) Author: malte Date: 2014-10-13.18:30:00
Let's go for "fast-downward.py" then. I added more comments on bitbucket.
msg3812 (view) Author: jendrik Date: 2014-10-13.18:03:57
3. is now done. Also, the portfolios now use the new --internal-plan-counter option. 

Since we now check that the plan costs decrease, I automatically add the "bound" parameter to 
the portfolio configurations.

The diff to your last commit is here: 
https://bitbucket.org/jendrikseipp/downward/branches/compare/bcc4128..1f46109#diff

So from my point of view this only leaves the question of file renamings.
msg3805 (view) Author: malte Date: 2014-10-13.13:34:16
No strong preferences either way.
msg3804 (view) Author: jendrik Date: 2014-10-13.12:30:41
Maybe it makes sense to rename "downward-release" to "search-release" for 
consistency while we're changing the planner interface. Opinions?
msg3803 (view) Author: jendrik Date: 2014-10-13.12:03:09
Regarding the TODOs in msg3798: 

1. I also like "fast-downward.py"

2. Done.

3. I like your proposed solution of cleaning up before starting the search. I would recommend issuing a warning before deleting the 
files but probably you already had this in mind. I will implement something along these lines.

Regarding the option names, I think using --preprocess , --preprocess-options, --run-all etc. is a nice solution.
msg3798 (view) Author: malte Date: 2014-10-11.22:30:28
@Jendrik: I've rewritten most of the plan-handling parts of the portfolio runner
to encapsulate the functionality more nicely and separate things into smaller
pieces. I think some further refactoring would be good -- we still have too many
distinct functionalities in one file, and it's a code smell that so many
functions take 7-9 positional arguments. But that doesn't have to concern us
now, I think. We can refactor this after merging this issue.

These are the things I think we still need to do before merging:

1. Rename ./plan.py to something else.

I'm fine with "fast-downward.py". Thoughts, opinions?

2. Fix the remaining issues with ./plan.py --help.

The lack of mention of input files vs. component options in the usage string is
the only one I can remember. I think I left a TODO in the code for this.

3. Address the issue mentioned in msg3786.

I noticed that we cannot leave this in the current state: when running as a
portfolio, the planner will find previously generated plans and think they are
fresh. This won't be a problem within lab when running everything from a fresh
repository, but we cannot leave it like this for our users.

4. Run experiments.


Anything I forgot?


Later on, I think it would be good to also use our PlanManager outside
portfolios and for optimal planning portfolios. (In that setting, they'd be used
only to detect and delete incomplete plans, not for plan counting.) But that
needs further code changes and doesn't need to be done within this issue.
msg3786 (view) Author: malte Date: 2014-10-11.19:44:25
A design question.

Background: one thing our driver file is supposed to do is to gracefully deal
with incomplete plan files and leave things in a clean state for the user. If
the search component is killed (e.g. due to a timeout) while writing a plan
file, we want to detect this and get rid of the incompletely written plan file.
Moreover, we want to make sure that for anytime configurations, plan files are
consecutively numbered. For example, if we run a portfolio consisting of several
anytime configurations and the first anytime configuration produces files
sas_plan.1, ..., sas_plan.4, of which the last one is incomplete, we want to
delete sas_plan.4 before running the next portfolio component and have that one
start its counter with number 4. This is important e.g. for the IPC where all
plans produced by an anytime planner are supposed to validate, except possibly
for the very last one if the planner was killed at an unfortunate time.

So far, so good. Here's the case which I'm not sure about how to handle best.
Let's say we run a planner configuration (no alias, no portfolio), and at the
end the driver script find the files sas_plan, sas_plan.1, sas_plan.2 and
sas_plan.3.

Normally, we would check the generated plans and see if they are complete,
deleting the incomplete ones. But in this case we don't know if the current
planner run produced sas_plan or if it produced the sas_plan.* files because the
driver doesn't know if the given configuration is anytime. Indeed, it's even
possible that *none* of these files were generated now because the current run
didn't succeed. It's also possible that, say, sas_plan.{1,2} are new and
sas_plan.3 stem from a previous run.

I don't really like this -- it would be good if the driver could tell which
plans were generated by *this* planner run, so that we can do something useful
with them (e.g. optionally run the validator). My suggestion is that *before*
running the search component, we always clean up existing plan files that fit
the given plan filename argument, and we do this both for the non-anytime
variant (default: sas_plan) and for the anytime variants (consecutively numbered
sas_plan.NUMBER files).

Thoughts?
msg3781 (view) Author: malte Date: 2014-10-11.17:59:06
OK, pushed this change and some others related to the help output. Also, the
alias for --alias is gone.

@Jendrik: passing over to you again; see the email I just sent.
msg3780 (view) Author: gabi Date: 2014-10-11.17:29:49
--run-all :-)
msg3779 (view) Author: malte Date: 2014-10-11.17:29:17
Me too. "--run-all" or "--all"?
msg3778 (view) Author: gabi Date: 2014-10-11.17:27:31
I also like --translate --preprocess and --search much better.
msg3777 (view) Author: malte Date: 2014-10-11.17:26:23
We decide to use "translate" instead of "translator" and "preprocess" instead of
"preprocessor" inside the options, but Jendrik didn't like the options
"--run-translate" and "--run-preprocess". How about renaming this set of options
to simply "--translate", "--preprocess", "--search"?
msg3776 (view) Author: malte Date: 2014-10-11.16:34:49
Gabi suggested changing the name of the driver script to something like
"fast-downward.py". Opinions?
msg3743 (view) Author: malte Date: 2014-10-09.12:05:36
Looking good! I suggest one more refactoring of the plan file handling in the
portfolio runner; everything else is just documentation and logging.
msg3692 (view) Author: jendrik Date: 2014-10-07.13:20:43
OK, I changed the config accordingly.
msg3685 (view) Author: silvan Date: 2014-10-06.22:36:56
As discussed with Malte in issue123, the unit-cost case of the lama
configuration should not use any specification of cost types, i.e.
we should remove lm_cost_type=plusone,cost_type=plusone from the
configuration.

    "--if-unit-cost",
    "--heuristic",
    "hlm,hff=lm_ff_syn(lm_rhw(reasonable_orders=true,"
    "                         lm_cost_type=plusone,cost_type=plusone))",

should become

    "--if-unit-cost",
    "--heuristic",
    "hlm,hff=lm_ff_syn(lm_rhw(reasonable_orders=true))",
msg3661 (view) Author: jendrik Date: 2014-10-05.22:34:02
I have prepared an experiment for testing that we didn't break anything, but I 
think it makes sense to do the code review first.

https://bitbucket.org/jendrikseipp/downward/pull-request/21/issue414-driver/diff
msg3628 (view) Author: malte Date: 2014-10-04.18:15:02
I'm not sure I remember the last stage of the discussion on bitbucket.
Should I review the current version there?
msg3522 (view) Author: malte Date: 2014-09-24.01:36:53
I guess plan.py would have the advantage that you could more easily run it on
Windows, where hash-bang lines don't have a meaning, but file extensions do.
msg3521 (view) Author: jendrik Date: 2014-09-24.01:29:28
I think we should just take the name we like best (plan or plan.py, I'm quite 
indifferent) and let lab figure out how to handle this later.
msg3520 (view) Author: malte Date: 2014-09-24.01:00:04
If we think of the new code as the default and the old one as the legacy
behaviour, I think it's generally better to test for the existence of something
that should be there (like the new plan script) than the absence of something
that shouldn't be there. (What if we decide to rename "downward-release" to
"downward" again now that this name is no longer used by the script?) But I have
no very strong preference.
msg3518 (view) Author: jendrik Date: 2014-09-23.22:13:36
I thought we could just test for the existence of src/search/downward. lab has 
never used the plan script anyway.
msg3509 (view) Author: malte Date: 2014-09-23.18:36:25
A note: once this is merged, I think we will need some changes in lab to fully
make use of the new features. However, it would be nice if these changes didn't
completely break previous versions of the code, i.e., ideally it should be
compatible with the old and new code.

Considering this, I was wondering if perhaps it might be a good idea to stick
with "plan.py" for the new driver script. This would give us a clean and simple
way to test if a given planner configuration is "new" (has plan.py) or "old".
msg3491 (view) Author: malte Date: 2014-09-21.16:50:00
I sent review comments on Rietveld. Looks like we could be there soon. :-)
msg3490 (view) Author: jendrik Date: 2014-09-21.15:38:19
I have tried to integrate portfolios into the new architecture and uploaded a 
patch that shows only my changes in the branch at 
https://codereview.appspot.com/146850044/. I also added some TODOs to the code 
for which I'd like your input before proceeding.
msg3375 (view) Author: gabi Date: 2014-08-29.12:22:01
Hui, having only two output files would be great! And with the integration of
the preprocessor and the search component we would be down at one. :-)
msg3370 (view) Author: malte Date: 2014-08-28.16:18:13
I think it would make sense to change the search code at the same time, so that
we no longer need plan_numbers_and_costs (as discussed before). Then we can get
rid of all the stupid temporary files with this issue, and only output.sas and
output would remain for now.
msg3369 (view) Author: jendrik Date: 2014-08-28.16:13:28
I'm happy to look into the portfolio part. Assigning this to me for now.
msg3368 (view) Author: malte Date: 2014-08-28.16:07:35
OK, the necessary parts of this to replace the old scripts are mostly done, with
the main missing feature the integration of the portfolio functionality. There
are some features we have discussed that are not in there yet, but these can
wait until later because the old scripts don't have them either. (I have a list
somewhere, but let's wait with this until later.)

Jendrik, if think we discussed offline a while ago that you could look into the
portfolio part. Do I recall correctly, and if yes, can you do take over for
this? (If not, I can do it too, but it might take a bit longer.)

The only other remaining TODOs are to remove some debugging code, make sure the
behaviour in case of planner errors makes sense, and improve the documentation.
But I think these are all best done last.

I've followed the suggestion of calling the script "plan" and have removed the
old "plan" script in the latest commit in the issue branch.
msg3266 (view) Author: malte Date: 2014-08-04.21:41:32
> I don't prefer either version, just wanted to mention it.

OK, in this case I think I'd prefer to go with KISS and YAGNI -- we might want
to generalize this later, but it's hard to get such an API right without a large
enough number of use cases, and in such as a case I think it's best to start
with something simpler and develop the more complex system once it's clearer
what it's going to be used for. issue448 is merged and documented on
http://www.fast-downward.org/OptionSyntax#Conditional_options.
msg3264 (view) Author: jendrik Date: 2014-08-04.10:43:25
Regarding 1: I like this solution.

Regarding 2: Moving this to the search code is a very good idea in my opinion. We 
might think about using generic --if and --if-not options that expect one argument 
to make the system more easily extendable. Then the config would become

--if unit-cost --heuristic "h1=ff()" --heuristic "h2=blind()"
--if-not unit-cost --heuristic "h1=cea()" --heuristic "h2=lmcut()"
--always --search "eager_greedy([h1, h2])"

and we could maybe replace "--always" with "--if true".

I don't prefer either version, just wanted to mention it.

Regarding 3: Good idea!

Regarding "--portfolio-file": If we ever want to allow passing portfolios on the 
command line, we could probably just pass multiple --search options with timeouts, 
therefore we don't have to reserve the name --portfolio for this use-case and I 
like the shorter version. BTW: your code still expects "--portfolio-file" in 
run_components.py, line 67.
msg3262 (view) Author: malte Date: 2014-08-03.19:09:13
> 2. Our mechanism for handling the unit-cost vs. general-cost case in the LAMA
> configuration is a special case that would be cleaner to implement if promoted
> to a general mechanism.

I've split this off to issue448 so that I can get started on an implementation.
(This doesn't mean we have to go with the idea if we end up disliking it.)
msg3260 (view) Author: malte Date: 2014-08-03.12:51:15
One more thing: I've renamed "--portfolio-file" to "--portfolio" for now because
"--portfolio-file" led to some really ugly line breaking in the --help output. I
used to prefer "--portfolio-file", but I'm somewhat indifferent about this point
now. If anyone has strong preferences, let me know.
msg3259 (view) Author: malte Date: 2014-08-03.12:48:41
I've worked on this again and now found a good solution for implementing the
separation of options for the driver script from options to the planner
components that doesn't require messing with argparse's usual mode of doing
things. (See https://bitbucket.org/malte/downward/commits/issue414 if interested.)

Actually working on the code showed some new aspects:

1. In implementation terms, it's much easier to *not* require a strict order
between translator, preprocessor and search options. So I've changed my mind on
this, and the currently implemented syntax works like the C++ access specifier
(public/private/protected) model. Component options go to the search component
by default, and this can be changed at any time by using --translate-options,
--preprocess-options or --search-options, which can be mixed freely and even
repeated.

2. Our mechanism for handling the unit-cost vs. general-cost case in the LAMA
configuration is a special case that would be cleaner to implement if promoted
to a general mechanism.

At the moment, the only way to use separate options for general vs. unit cost is
by implementing an alias in the driver script. This is a bit unfortunate; e.g.,
it complicates doing experiments with LAMA-like planners, and it means that such
a distinction could not be made in a portfolio. It also complicates the alias
implementation, at it means that we need an extra component that goes over the
input file to test whether it is unit cost, which takes time and requires
another piece of code that needs to be able to (at least partially) parse our
input files.

I propose to change this by moving this functionality into the search component.
The simplest clean mechanism I can think of is to support special arguments that
mean that the following arguments are only considered for certain inputs. For
example, this:

--if-unit-cost --heuristic "h1=ff()" --heuristic "h2=blind()"
--if-nonunit-cost --heuristic "h1=cea()" --heuristic "h2=lmcut()"
--always --search "eager_greedy([h1, h2])"

would do an eager greedy search with the FF and blind heuristic on unit cost
problems and with the cea and LM-cut heuristic on other problems. Would you be
OK with that change? Then LAMA could be a regular configuration like any other.

Also, please let me know if you have other suggestions for the names of these
options. I like "--if-unit-cost" (in particular the fact that it begins with
"--if"), and I also think we need an "if true" option, but I'm not sure if
"--always" is the best name for it. (Then again, I'm not sure "--if-true" would
be a good name either.)

3. It would be useful if the driver script could automatically detect if a given
input was a PDDL file, a translator output file, or a preprocessor output file.
This is of course possible to do now already, but it would be nice if we could
make it simpler the next time we change the output format. In particular, at the
moment, we may have to read many megabytes of input before we can distinguish a
translator output file from a preprocessor output file. I suggest we make a
change to the headers of translator and preprocessor files to make this
distinction easier. One simple possibility would be to augment the existing
begin_version block, e.g.:

begin_version
4
translator
end_version

What do you think?
msg3127 (view) Author: jendrik Date: 2014-04-14.15:31:17
Orders: I agree.

Naming: I also have no strong preference, but would probably still go for "plan".
msg3126 (view) Author: malte Date: 2014-04-14.15:17:55
> I don't see any problems with the syntax you propose in msg3114. While I
> think it makes sense to put the arguments in the proposed order, I don't
> really see why we should enforce this. Am I overseeing a source of ambiguity
> or is it because of argparse's prefix-matching?

We have to require "driver options before filenames before rest" to determine
the boundaries for these (string-matching is not enough; e.g. someone could use
"--portfolio-file --translator-options" to use a portfolio file with filename
"--translator-options".

We don't need to mandate an order for the other three parts, but I think if we
have to mandate the partial order "A < B < {C, D, E}", the mental model for the
user is simplest if we make the order strict altogether and only allow "A < B <
C < D < E". It's easier to explain and hence easier to remember.

> and additionally "plan" is also the canonical name for IPC 2011 and 2014
planner scripts

For me this is an argument against the name "plan", because our script uses a
different syntax than these planner scripts (and has to, because one has to
specify the search options at minimum). So if we *don't* call it plan (but
rather something else like "framboozle", then an IPC submission could write a
"plan" wrapper script that calls "framboozle". If we call our script plan, then
an IPC submission must first rename it before adding another plan script.

(However, this could be resolved by putting the IPC plan script in a different
directory, e.g. at the level above "src", and have "plan" call "src/plan".)

Does this change your mind?

(I don't want to convince; I'm quite indifferent between the two options. I
tab-complete anyway, so brevity of the name is not really an issue for me.)
msg3125 (view) Author: jendrik Date: 2014-04-14.13:21:40
I don't see any problems with the syntax you propose in msg3114. While I think it makes sense 
to put the arguments in the proposed order, I don't really see why we should enforce this. Am I 
overseeing a source of ambiguity or is it because of argparse's prefix-matching? 

Regarding your questions: I think it makes sense to rename "plan.py" to "plan" for backwards 
compatibility and brevity. Also, the extension might confuse some people and additionally 
"plan" is also the canonical name for IPC 2011 and 2014 planner scripts. I think domain-auto-
detection should be moved into the driver script.
msg3114 (view) Author: malte Date: 2014-04-13.01:07:04
I wrote some less technical documentation on the suggested command-line syntax
than in msg3111, since we'll have to be explain it to other people once the work
on this issue is done. (I mainly did this to focus my thoughts on the syntax;
the syntax is not yet set in stone if you don't like it).

I'm pasting it here since it may be easier to read and/or make more sense than
msg3111:

The command-line syntax of the new driver script is as follows (broken
up over multiple lines to avoid overly long lines):

$ src/plan.py [DRIVER_OPTION ...] [FILENAME ...]
   [--translate-options TRANSLATE_OPTION ...]
   [--preprocess-options PREPROCESS_OPTION ...]
   [[--search-options] SEARCH_OPTION ...]

Note that there is a fixed order in which the different arguments must
be specified:

1. options for driver script
2. input filename(s)
3. options for the translator
4. options for the preprocessor
5. options for the search

All parts are optional. However, a filename must be given in almost
all circumstances, since the planner needs an input to operate on.
(Filenames are only optional when no actual translation, preprocessing
or search should take place, for example when displaying help output.)

The --search-options marker may be omitted if at least one filename is
given and neither --translate-options nor --preprocess-options are
given. (Otherwise the marker is necessary to clarify where the options
that belong to the search component begin.)

To find out about the options of the driver script, run

$ src/plan.py --help


Supporting unusual filenames:

- To partition the arguments into the five groups, src/plan.py assumes
  that filenames don't begin with "-". To safely support arbitrary
  filenames, include "--" separators before and after the filenames as
  follows:

  $ src/plan.py [DRIVER_OPTION ...] -- FILENAME ... --
     [--translate-options TRANSLATE_OPTION ...]
     [--preprocess-options PREPROCESS_OPTION ...]
     [[--search-options] SEARCH_OPTION ...]

  This works with arbitrary filenames, even the filename "--". Note
  that a TRANSLATE_OPTION, PREPROCESS_OPTION or SEARCH_OPTION may
  never be "--", so src/plan.py will assume that the *last* occurrence
  of "--" is the one that separates the filenames from the following
  options.


For users of the old src/plan script:

- Note that the new syntax is compatible with the old one:

  $ src/plan FILENAME [FILENAME] [SEARCH_OPTION ...]

  The above rules guarantee that this syntax still works with
  src/plan.py, as long as the filenames don't begin with "-".
msg3113 (view) Author: malte Date: 2014-04-13.00:14:09
Question: once this is done, should we rename "plan.py" to "plan"?

Advantage: old users of "plan" don't need to switch to a new name.

Disadvantage: we use *.py for Python scripts everywhere else.
msg3112 (view) Author: malte Date: 2014-04-12.22:34:32
We also discussed that we want to support compression, i.e., be able to read
compressed inputs. Given the size of some pregrounded PDDL inputs, I think it
makes sense to support compression also for the PDDL inputs.

But this gives us another little headache. Currently, we have some logic in the
translator to auto-detect the domain file that goes with a problem file in most
cases. But this can't work when we use a compressed input, because then the
translator only sees a named pipe as its input file, without knowing the
original filename. I see two main solutions:

1) Only support auto-detection of domain names when using uncompressed inputs.

2) Move the functionality for auto-detecting domain names from the translator
into the driver script.

I think 2) makes sense conceptually because auto-detection of domain names is
exactly the kind of convenience functionality for which we have a wrapper script
in the first place.

What do you think?
msg3111 (view) Author: malte Date: 2014-04-12.22:25:58
We thought about the option-passing issue a little bit more, and the original
idea with domain.pddl:problem.pddl is probably a bad idea because it is
problematic on Windows (where ":" occurs in filenames with drive letters). There
are also some other issues that are a bit tricky to work around, e.g. the
argument prefix handling of argparse and the fact that we want a --help option
both for the driver script and for the components.

Remember, one main issue is that the driver script must decide which arguments
to pass on to whom, but should not need to know what the command-line syntax of
each component is to disambiguate which arguments go where. And we also don't
want to have to pass lots of explicit markers for every call, as in "./plan.py
--debug --search-options --search "astar(blind())" --inputs domain.pddl
problem.pddl".

My new suggestion would be to use the following command-line syntax, which
allows avoiding markers unless one wants to pass special translator or
preprocessor options:

$ ./plan.py [OPTIONS_FOR_DRIVER_SCRIPT] FILENAME ... [OPTIONS_FOR_COMPONENTS]

where the arguments have to occur in a certain order. Everything before the
filename(s) belongs to the driver script, everything after it to the planner
components. The driver script would parse the arguments sequentially until it
finds something that isn't an argument to one of its options and doesn't start
with "-". From there on, it assumes that the following arguments are FILENAMEs
followed by options for the components.

This also mimics how the plan script currently works, so the old syntax would
still work:

$ ./plan FILENAME [FILENAME] [OPTIONS_FOR_SEARCH_COMPONENT]

(This is just the special case with no options for the driver script and all
options going to the search component, which will be the default; see below.)

This also mimics how traditional Unix argument parsing works, and how it works
with GNU argument passing conventions if one inserted "--" before the first
FILENAME. Implementing this may be a bit tricky with argparse, unfortunately,
which is more lenient than traditional Unix, but I think there is no way to
avoid some argparse-related pain no matter which solution we choose.

To decide where the OPTIONS_FOR_COMPONENTS begin (i.e., how many FILENAMEs are
passed) we use the rule that arguments after the driver options are FILENAMEs
until we first find one that begins with "-", which starts the
OPTIONS_FOR_COMPONENTS. This makes it a bit difficult to use filenames that
start with "-" (they could be given as "./-XYZ", but this is painful in
scripts). To deal with this, we also support the traditional "--" separator: if
"--" occurs anywhere after the options for the driver script, then the last such
occurrence is used as a separator between FILENAMEs and
OPTIONS_FOR_SEARCH_COMPONENT. (I think this rule is the only one that works for
all filenames, even when one uses an input file called "--".)

Regarding OPTIONS_FOR_COMPONENTS, I think it's most sane if we only allow them
in the order translator, preprocessor, search, and if we require explicit
markers to say which options go to which component:

[--translate-options ...] [--preprocess-options ...] [--search-options ...]

To make the most common case nicer, one may also leave out the
"--search-options" marker if there are no translator options or preprocessor
options.

Does this make sense? Do you see any problems?
msg2949 (view) Author: malte Date: 2014-02-10.19:46:46
Outcome of meeting of Jendrik and Malte:

- integrate everything into one script

- support options --timeout and --memory

- rename --portfolio to --portfolio-file to later allow specifying simple
portfolios directly on the command line (but don't implement this immediately)

- portfolios to be executed with execfile; won't need import and the call that
is currently at the bottom since they won't drive the overall process; they only
need to define certain attributes

- To get rid of plan_numbers_and_cost, include a comment with the plan cost in
the generated sas_plan files. Commented lines start with "; " and are ignored by
VAL. We need to be able to test that the file was completely written. For this
purpose, we must have the "; cost = XXX" line at the *bottom* of the plan file
and end it with a newline. (Existence of the newline then shows that the cost
was also completely written.)

- The overall script always requires *one* positional argument (the first one)
that specifies the input. In "auto mode", if the input file looks like it might
be a preprocessor output file, only the search component is run. Otherwise, all
three components are run. We might also include manual specification of modes
such as --only-translate, --only-preprocess, --only-search to override the
default auto mode. For cases where the translator needs two input files, these
are specified as one argument of the form "domain.pddl:problem.pddl". The
rationale of this is that we don't have to make guesses how many of the
parameters we get are intended as input files rather than arguments to be passed
on. Note the confusing behaviour we currently get with something like:
    $ ./plan ../benchmarks/blocks/domain.pddl
../benchmarks/blocks/probBLOCKS-17-1.pddl --search 'astar(blind())'
(Hint: there is no blocksworld task 17-1). At least in my shell, filenames
tab-complete nicely after a ":", so that's not a usability issue.

- By default, unparsed parameters go to the search component. However, this can
be overridden by specifying options like
translate OPTION_FOR_TRANSLATOR ANOTHER_OPTION_FOR_TRANSLATOR preprocess
OPTION_FOR_PREPROCESSOR search OPTION_FOR_SEARCH ANOTHER_OPTION_FOR_SEARCH
(The absence of "--" in "translate", "preprocess", "search" is intended to mimic
hg-style subcommands.)
Think of this as working like C++'s public/protected/private, where the default
is "search".
msg2943 (view) Author: malte Date: 2014-02-10.01:12:32
The new downward.py script that makes "dispatch" and "unitcost" obsolete is
mostly done. The only part that is missing at the moment is the integration of
portfolios. Once this is done, the "plan.py" and "downward.py" scripts could be
integrated if desired.
msg2929 (view) Author: jendrik Date: 2014-02-07.11:50:03
That's true. Let's just forget about my first point.
msg2927 (view) Author: malte Date: 2014-02-07.11:30:24
I'm not sure I understand the first point. In option 1, it would be trivial to
have --translate, --preprocess, --search options (or combinations of them) if we
want. I only listed --search as an example. (In contrast, auto-detection can't
distinguish the case where it's run on a translator file or a preprocessor file
without potentially reading tens of MB of data, since they look the same until
after the operator description, so that couldn't be easily supported.)
msg2926 (view) Author: jendrik Date: 2014-02-07.11:06:05
I have no strong preference, but I like auto-detection a little more since in 
Option 1:
- --search wouldn't have a --preprocess counterpart
- there would be an additional "--search" in the SEARCH_OPTION
msg2925 (view) Author: malte Date: 2014-02-06.22:36:20
A design question: currently, there are two main modes for running the planner:
running all three major components in one go (via the plan script), and running
only the search component for an already preprocessed task (via the downward
script).

It makes sense to unify the functionality of the two scripts in one: this way,
we get rid of the need to communicate via intermediate files like elapsed.time.
But we still need to support both use cases (run whole planner; run only search)
for lab experiments where we don't care about translation/preprocessing, for
debugging, etc.

How should the script know in which mode it is run?

Option 1: via command-line arguments

$ ./plan problem.pddl SEARCH_OPTION ...
=> run full planner on input file "problem.pddl"

$ ./plan --search output SEARCH_OPTION ...
=> run search only on input file "output"

Option 2: auto-detection of the input file:

$ ./plan problem.pddl SEARCH_OPTION ...
=> run full planner because first argument doesn't look like a preprocessor
output file (for this we'd have to need the first few lines of the file)

$ ./plan output SEARCH_OPTION ...
=> run search only because first argument looks like a preprocessor output file 

Thoughts?
msg2924 (view) Author: malte Date: 2014-02-06.22:25:48
I've shared a repository in which I'm working on the branch for this issue. For
now, I've written a replacement for the plan script (minus some TODOs -- see code).

The downward script etc. still need to be integrated, so at the moment there's
no real benefit yet.
msg2923 (view) Author: malte Date: 2014-02-06.21:06:42
The name of the issue is poor, but I couldn't phrase this better.

This is about cleaning up the way that the different parts of the planner call
each other and what intermediate files are generated by the process. At the
moment, running the planner involves the following executables (omitting
standard stuff like "cat" etc. that we can expect to be installed in all OSes we
support):

1. plan (main script; Bash)
2. translate/translate.py (translator; Python)
3. preprocess/preprocess (preprocessor; C++)
4. search/downward (search component wrapper script; Bash)
5. search/downward-*.py (portfolio wrapper scripts; Python)
6. search/downward-{1,2,4} (search component executables; C++)
7. search/dispatch (script to determine required state_var_t size; gawk wrapped
into sh)
8. search/unitcost (script to determine whether general action costs are used;
gawk wrapped into sh)
9. gtime (OS X only; third-party executable)

Basically, 1. calls 2., 3., 4. and 9.
4. calls 5., 6., 7., 8. and 9.
5. calls 6.

Currently we use the following files (inputs and intermediate files):

A. PDDL domain file (input)
B. PDDL problem file (input)
C. output.sas (translator output)
D. output (preprocessor output)
E. sas_plan and sas_plan.* (plan(s) found)
F. downward.tmp.* (copy of "output")
G. elapsed.time (records time used by planner components other than the main
search component for allocating time slices in the portfolio)
H. plan_numbers_and_cost (records number and qualities of solutions found so
far, so that the portfolio can set the correct output file names and cost bounds)

This issue is about getting rid of executables 4., 5., 7., 8. and 9. and
intermediate files F., G., H.

The idea is to replace 1. with a Python script that includes the functionality
of 4. 5., 7., 8. and 9. directly.
History
Date User Action Args
2014-10-23 17:20:26maltesetmessages: + msg3867
2014-10-23 16:53:35jendriksetstatus: done-cbb -> resolved
messages: + msg3866
2014-10-23 12:26:39maltesetmessages: + msg3861
2014-10-22 22:10:04jendriksetmessages: + msg3858
2014-10-17 14:33:48maltesetmessages: + msg3845
2014-10-17 14:32:03jendriksetstatus: reviewing -> done-cbb
messages: + msg3844
2014-10-17 11:06:27maltesetmessages: + msg3843
2014-10-17 11:06:03jendriksetmessages: + msg3842
2014-10-16 17:22:09maltesetmessages: + msg3837
2014-10-16 16:42:33jendriksetmessages: + msg3836
2014-10-14 14:31:42maltesetmessages: + msg3823
2014-10-14 13:37:21jendriksetmessages: + msg3822
2014-10-13 18:30:00maltesetmessages: + msg3813
2014-10-13 18:03:57jendriksetmessages: + msg3812
2014-10-13 13:34:16maltesetmessages: + msg3805
2014-10-13 12:30:41jendriksetmessages: + msg3804
2014-10-13 12:03:09jendriksetmessages: + msg3803
2014-10-11 22:30:28maltesetmessages: + msg3798
2014-10-11 19:44:26maltesetmessages: + msg3786
2014-10-11 17:59:06maltesetmessages: + msg3781
2014-10-11 17:29:49gabisetmessages: + msg3780
2014-10-11 17:29:17maltesetmessages: + msg3779
2014-10-11 17:27:31gabisetmessages: + msg3778
2014-10-11 17:26:23maltesetmessages: + msg3777
2014-10-11 16:34:49maltesetmessages: + msg3776
2014-10-09 12:05:36maltesetmessages: + msg3743
2014-10-07 13:20:43jendriksetmessages: + msg3692
2014-10-06 22:36:56silvansetnosy: + silvan
messages: + msg3685
2014-10-05 22:34:03jendriksetstatus: chatting -> reviewing
messages: + msg3661
2014-10-04 18:15:02maltesetmessages: + msg3628
2014-09-24 01:36:53maltesetmessages: + msg3522
2014-09-24 01:29:28jendriksetmessages: + msg3521
2014-09-24 01:00:04maltesetmessages: + msg3520
2014-09-23 22:13:36jendriksetmessages: + msg3518
2014-09-23 18:36:25maltesetmessages: + msg3509
2014-09-21 16:50:00maltesetmessages: + msg3491
2014-09-21 15:38:19jendriksetmessages: + msg3490
2014-08-29 12:22:01gabisetmessages: + msg3375
2014-08-28 16:18:13maltesetmessages: + msg3370
2014-08-28 16:13:28jendriksetassignedto: malte -> jendrik
messages: + msg3369
2014-08-28 16:07:35maltesetmessages: + msg3368
2014-08-04 21:41:32maltesetmessages: + msg3266
2014-08-04 10:43:25jendriksetmessages: + msg3264
2014-08-03 19:09:13maltesetmessages: + msg3262
2014-08-03 12:51:15maltesetmessages: + msg3260
2014-08-03 12:48:41maltesetmessages: + msg3259
2014-04-14 15:31:17jendriksetmessages: + msg3127
2014-04-14 15:17:55maltesetmessages: + msg3126
2014-04-14 13:21:40jendriksetmessages: + msg3125
2014-04-13 01:07:04maltesetmessages: + msg3114
2014-04-13 00:14:09maltesetmessages: + msg3113
2014-04-12 22:34:32maltesetmessages: + msg3112
2014-04-12 22:25:58maltesetmessages: + msg3111
2014-03-04 14:02:59gabisetnosy: + gabi
2014-02-10 19:46:46maltesetmessages: + msg2949
2014-02-10 01:12:32maltesetmessages: + msg2943
2014-02-07 11:50:03jendriksetmessages: + msg2929
2014-02-07 11:30:24maltesetmessages: + msg2927
2014-02-07 11:06:05jendriksetmessages: + msg2926
2014-02-06 22:36:20maltesetmessages: + msg2925
2014-02-06 22:25:48maltesetstatus: unread -> chatting
messages: + msg2924
2014-02-06 21:20:50jendriksetnosy: + jendrik
2014-02-06 21:06:42maltecreate