I merged the branch into default. I assume David Epstein is located in the US
and therefore possibly didn't pull the latest version yet, wondering why the
problem still takes long for translating.

Merged. Thanks everyone, especially Jendrik for running the experiments and
creating the tag that I forgot.

@Guillem: the original reason for this change was that I noticed that "Assign"
is exported from the pddl module in pddl.__init__.py, while the base class is
not, so "FunctionAssignment" is treated a bit like an internal implementation
secret of the pddl module. (The old code lived inside the pddl module and could
use "FunctionAssignment" directly; the new code does not.)

Of course we could add "FunctionAssignment" to the list of exported classes. But
I think "Assign" is a better fit anyway because this iterates over the initial
state. In the initial state, you are not allowed to say "(increase ...)", only
"(= ...)" The "FunctionAssignment" base class is only really relevant in the
context of action effects.

I also had a look at the code; nothing to add.

I had a quick look and the code looks good to me as well. The only thing is that I see you changed a check 

isinstance(fact, FunctionAssignment)

by another

isinstance(element, pddl.Assign):

whose only effect, looking at the class hierarchy, is to leave out `Increase` objects.
I'm not familiar with the preprocessor, but I guess that those objects might have been compiled into semantically equivalent 
`Assign` objects at that point and that that is no oversight, right? If so, feel free to ignore tihs comment!

Many thanks! The "filtered" ones are based on the output.sas hash? Yes, these
tasks are all likely to be affected by the invariant synthesis timeout.

It looks like there are significant speedups of the "completing instantiation"
stage for some IPC 2018 domains, which is nice. In agricola-sat18-strips, we
save a total of 348 seconds, and in data-network-sat18-strips the total for this
step drops from 61 to 18 seconds, which is also nice.

From my perspective it looks like this can be merged, but I'll wait until
tomorrow to see if someone else wants to look at the diff first.

Here are the results:
https://ai.dmi.unibas.ch/_tmp_files/seipp/issue919-v1-issue919-base-issue919-v1-compare.html

And here are the runs where the output changed (I guess due to nondeterminism):
https://ai.dmi.unibas.ch/_tmp_files/seipp/issue919-v1-issue919-base-issue919-v1-compare-filtered.html

The code looks fine to me, but I'm not familiar with the inner workings of the
translator.

Awesome! :-)

> Also, can someone help run an experiment?

I'll run an experiment.

I have pushed a change to my bitbucket repository that seems to address the
issue. On dmi-skinny, the time translating the example task reduces from 4113.23
seconds to 5.03 seconds. Can someone have a look at the code? It is not a big diff.

Pull request here:

https://bitbucket.org/malte/downward/pull-requests/10/issue919-translator-action-instantiation/diff

Also, can someone help run an experiment? This would need a translator-only
experiment on all benchmarks (in all formulations) to verify that the translator
output doesn't change and to compare the translator runtime before-after. The
interesting parts are total runtime and the runtime for the "completing
instantiation" step.

In terms of runtime, this should only affect tasks that use parameterized action
costs (i.e. something like "(increase (total-cost) (some-function ?x ?y ?z))")
and that have a large initial state in terms of the number of entries in the
":init" list. I don't know any IPC domains that have these properties, so it
should have no noticeable impact on the IPC benchmarks. But I don't know the
more recent domains very well.

I initially thought that this might affect problems with large initial states in
general (whether or not they use parameterized costs), but that was based on a
wrong assumption about the type of a variable.

And here is the domain file.

The translator action instantiation ("Completing instantiation...") uses linear
searches to look up cost assignments in the initial state. These can be required
hundreds or thousands of times, or millions of times in extreme cases. So this
looks like a major performance bottleneck in problems with a large initial state.

See the "init_facts" arguments of the "instantiate" methods in:
pddl/actions.py
pddl/axioms.py
pddl/conditions.py
pddl/effects.py
pddl/f_expression.py

Most of these are probably OK. There is a problematic linear scan in
pddl/f_expression.py, but it may be a good idea to check the other files, too.

The init_facts are currently represented as a set. They should be represented as
a dict instead. Or we could separate the propositions from the cost assignments,
using a set for the former and a dict for the latter.

On the attached example problem, provided by David Epstein, the translator
requires 4113.23 seconds on dmi-skinny, of which 4109.66 seconds are for
"Completing instantiation".  The search component with astar(lmcut()) requires
0.009 seconds to solve the problem.

I compressed the problem file because it is large for the tracker (~9 MB
uncompressed).